sysfs: minimize heap allocations of sysfs paths

11a0918a9972 ("nvme: allow to overwrite base sysfs path")
added support for changing the sysfs path via an environment variable.
Unfortunately, it added a heap allocation
every time a sysfs path was requested.

Modify the callers to not free the paths, which allows a string constant
to be returned if the path isn't overridden, avoiding an allocation.
Cache the path in a static variable so that if it is overridden,
the heap-allocated string only needs to be constructed once
and afterwards can be reused.

Create a file sysfs.c to consolidate this logic
instead of spreading them across 3 files.
Also introduce a helper to factor out the duplicated code.

Fixes: 11a0918a9972 ("nvme: allow to overwrite base sysfs path")
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>

diff --git a/src/meson.build b/src/meson.build
index 43d641a520b5c8c164e8621065e5acfc720f0507..001c3b922bf2b8a496e83a4f1497644ca8198a13 100644 (file)
--- a/src/meson.build
+++ b/src/meson.build
@@ -12,6 +12,7 @@ sources = [
     'nvme/ioctl.c',
     'nvme/linux.c',
     'nvme/log.c',
+    'nvme/sysfs.c',
     'nvme/tree.c',
     'nvme/util.c',
     'nvme/base64.c',

diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c
index ac639bdfd3b9581de253384f51664df9defb63ff..6738e9dc42ec2d5ef58aa1974a3188fa8a42ff06 100644 (file)
--- a/src/nvme/fabrics.c
+++ b/src/nvme/fabrics.c
@@ -1186,29 +1186,12 @@ struct nvmf_discovery_log *nvmf_get_discovery_wargs(struct nvme_get_discovery_ar
        return log;
 }
 
-#define PATH_UUID_IBM  "/proc/device-tree/ibm,partition-uuid"
-
-static char *uuid_ibm_filename(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_UUID_IBM);
-
-       if (!asprintf(&str, "%s" PATH_UUID_IBM, basepath))
-               return NULL;
-
-       return str;
-}
-
 static int uuid_from_device_tree(char *system_uuid)
 {
-       _cleanup_free_ char *filename = uuid_ibm_filename();
        _cleanup_fd_ int f = -1;
        ssize_t len;
 
-       f = open(filename, O_RDONLY);
+       f = open(nvme_uuid_ibm_filename(), O_RDONLY);
        if (f < 0)
                return -ENXIO;
 
@@ -1220,22 +1203,6 @@ static int uuid_from_device_tree(char *system_uuid)
        return strlen(system_uuid) ? 0 : -ENXIO;
 }
 
-#define PATH_DMI_ENTRIES       "/sys/firmware/dmi/entries"
-
-static char *dmi_entries_dir(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_DMI_ENTRIES);
-
-       if (!asprintf(&str, "%s" PATH_DMI_ENTRIES, basepath))
-               return NULL;
-
-       return str;
-}
-
 /*
  * See System Management BIOS (SMBIOS) Reference Specification
  * https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.2.0.pdf
@@ -1264,7 +1231,7 @@ static bool is_dmi_uuid_valid(const char *buf, size_t len)
 static int uuid_from_dmi_entries(char *system_uuid)
 {
        _cleanup_dir_ DIR *d = NULL;
-       _cleanup_free_ char *entries_dir = dmi_entries_dir();
+       const char *entries_dir = nvme_dmi_entries_dir();
        int f;
        struct dirent *de;
        char buf[512] = {0};

diff --git a/src/nvme/filters.c b/src/nvme/filters.c
index 312b8f6c896766e048552d451d2606da675cd787..ceaba68f9d1000da5668351602beff150af9cca2 100644 (file)
--- a/src/nvme/filters.c
+++ b/src/nvme/filters.c
@@ -6,68 +6,12 @@
  * Authors: Keith Busch <keith.busch@wdc.com>
  *         Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
  */
-#include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <dirent.h>
-#include <libgen.h>
-
-#include <sys/param.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <fcntl.h>
-#include <unistd.h>
 
 #include "filters.h"
-#include "types.h"
-#include "util.h"
-#include "cleanup.h"
-
-#define PATH_SYSFS_NVME                        "/sys/class/nvme"
-#define PATH_SYSFS_NVME_SUBSYSTEM      "/sys/class/nvme-subsystem"
-#define PATH_SYSFS_BLOCK               "/sys/block"
-
-char *nvme_ctrl_sysfs_dir(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_SYSFS_NVME);
-
-       if (!asprintf(&str, "%s" PATH_SYSFS_NVME, basepath))
-               return NULL;
-
-       return str;
-}
-
-char *nvme_ns_sysfs_dir(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_SYSFS_BLOCK);
-
-       if (!asprintf(&str, "%s" PATH_SYSFS_BLOCK, basepath))
-               return NULL;
-
-       return str;
-}
-
-char *nvme_subsys_sysfs_dir(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_SYSFS_NVME_SUBSYSTEM);
-
-       if (!asprintf(&str, "%s" PATH_SYSFS_NVME_SUBSYSTEM, basepath))
-               return NULL;
-
-       return str;
-}
+#include "private.h"
 
 int nvme_namespace_filter(const struct dirent *d)
 {
@@ -132,7 +76,7 @@ int nvme_subsys_filter(const struct dirent *d)
 
 int nvme_scan_subsystems(struct dirent ***subsys)
 {
-       _cleanup_free_ char *dir = nvme_subsys_sysfs_dir();
+       const char *dir = nvme_subsys_sysfs_dir();
 
        return scandir(dir, subsys, nvme_subsys_filter, alphasort);
 }
@@ -145,7 +89,7 @@ int nvme_scan_subsystem_namespaces(nvme_subsystem_t s, struct dirent ***ns)
 
 int nvme_scan_ctrls(struct dirent ***ctrls)
 {
-       _cleanup_free_ char *dir = nvme_ctrl_sysfs_dir();
+       const char *dir = nvme_ctrl_sysfs_dir();
 
        return scandir(dir, ctrls, nvme_ctrls_filter, alphasort);
 }

diff --git a/src/nvme/private.h b/src/nvme/private.h
index 35058020b33d39b4163347a27f972e8448c8f44b..11744c2575265c2fc14031ff5b929cb2e9431311 100644 (file)
--- a/src/nvme/private.h
+++ b/src/nvme/private.h
@@ -17,9 +17,12 @@
 #include "mi.h"
 
 
-char *nvme_ctrl_sysfs_dir(void);
-char *nvme_subsys_sysfs_dir(void);
-char *nvme_ns_sysfs_dir(void);
+const char *nvme_subsys_sysfs_dir(void);
+const char *nvme_ctrl_sysfs_dir(void);
+const char *nvme_ns_sysfs_dir(void);
+const char *nvme_slots_sysfs_dir(void);
+const char *nvme_uuid_ibm_filename(void);
+const char *nvme_dmi_entries_dir(void);
 
 struct nvme_path {
        struct list_node entry;

diff --git a/src/nvme/sysfs.c b/src/nvme/sysfs.c
new file mode 100644 (file)
index 0000000..ea4f0e2
--- /dev/null
+++ b/src/nvme/sysfs.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "private.h"
+
+#define PATH_UUID_IBM                  "/proc/device-tree/ibm,partition-uuid"
+#define PATH_SYSFS_BLOCK               "/sys/block"
+#define PATH_SYSFS_SLOTS               "/sys/bus/pci/slots"
+#define PATH_SYSFS_NVME_SUBSYSTEM      "/sys/class/nvme-subsystem"
+#define PATH_SYSFS_NVME                        "/sys/class/nvme"
+#define PATH_DMI_ENTRIES               "/sys/firmware/dmi/entries"
+
+static const char *make_sysfs_dir(const char *path)
+{
+       char *basepath = getenv("LIBNVME_SYSFS_PATH");
+       char *str;
+
+       if (!basepath)
+               return path;
+
+       if (asprintf(&str, "%s%s", basepath, path) < 0)
+               return NULL;
+
+       return str;
+}
+
+const char *nvme_subsys_sysfs_dir(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_SYSFS_NVME_SUBSYSTEM);
+}
+
+const char *nvme_ctrl_sysfs_dir(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_SYSFS_NVME);
+}
+
+const char *nvme_ns_sysfs_dir(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_SYSFS_BLOCK);
+}
+
+const char *nvme_slots_sysfs_dir(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_SYSFS_SLOTS);
+}
+
+const char *nvme_uuid_ibm_filename(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_UUID_IBM);
+}
+
+const char *nvme_dmi_entries_dir(void)
+{
+       static const char *str;
+
+       if (str)
+               return str;
+
+       return str = make_sysfs_dir(PATH_DMI_ENTRIES);
+}

diff --git a/src/nvme/tree.c b/src/nvme/tree.c
index 50e40917c4e08cf5832219e4d6270f2163086475..de6ce8799ad7435cb25166f950be9291bdc6af5b 100644 (file)
--- a/src/nvme/tree.c
+++ b/src/nvme/tree.c
@@ -61,22 +61,6 @@ struct candidate_args {
 };
 typedef bool (*ctrl_match_t)(struct nvme_ctrl *c, struct candidate_args *candidate);
 
-#define PATH_SYSFS_SLOTS "/sys/bus/pci/slots"
-
-static char *nvme_slots_sysfs_dir(void)
-{
-       char *basepath = getenv("LIBNVME_SYSFS_PATH");
-       char *str;
-
-       if (!basepath)
-               return strdup(PATH_SYSFS_SLOTS);
-
-       if (!asprintf(&str, "%s" PATH_SYSFS_SLOTS, basepath))
-               return NULL;
-
-       return str;
-}
-
 static struct nvme_host *default_host;
 
 static void __nvme_free_host(nvme_host_t h);
@@ -672,10 +656,9 @@ static int nvme_subsystem_scan_namespaces(nvme_root_t r, nvme_subsystem_t s,
 
 static int nvme_init_subsystem(nvme_subsystem_t s, const char *name)
 {
-       _cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
        char *path;
 
-       if (asprintf(&path, "%s/%s", subsys_dir, name) < 0)
+       if (asprintf(&path, "%s/%s", nvme_subsys_sysfs_dir(), name) < 0)
                return -1;
 
        s->model = nvme_get_attr(path, "model");
@@ -716,12 +699,11 @@ static int nvme_scan_subsystem(struct nvme_root *r, const char *name,
 {
        struct nvme_subsystem *s = NULL, *_s;
        _cleanup_free_ char *path = NULL, *subsysnqn = NULL;
-       _cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
        nvme_host_t h = NULL;
        int ret;
 
        nvme_msg(r, LOG_DEBUG, "scan subsystem %s\n", name);
-       ret = asprintf(&path, "%s/%s", subsys_dir, name);
+       ret = asprintf(&path, "%s/%s", nvme_subsys_sysfs_dir(), name);
        if (ret < 0)
                return ret;
 
@@ -1703,7 +1685,7 @@ static int nvme_ctrl_scan_namespaces(nvme_root_t r, struct nvme_ctrl *c)
 static char *nvme_ctrl_lookup_subsystem_name(nvme_root_t r,
                                             const char *ctrl_name)
 {
-       _cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
+       const char *subsys_dir = nvme_subsys_sysfs_dir();
        _cleanup_dirents_ struct dirents subsys = {};
        int i;
 
@@ -1730,7 +1712,7 @@ static char *nvme_ctrl_lookup_subsystem_name(nvme_root_t r,
 
 static char *nvme_ctrl_lookup_phy_slot(nvme_root_t r, const char *address)
 {
-       _cleanup_free_ char *slots_sysfs_dir = nvme_slots_sysfs_dir();
+       const char *slots_sysfs_dir = nvme_slots_sysfs_dir();
        _cleanup_free_ char *target_addr = NULL;
        _cleanup_dir_ DIR *slots_dir = NULL;
        int ret;
@@ -1827,7 +1809,6 @@ static int nvme_configure_ctrl(nvme_root_t r, nvme_ctrl_t c, const char *path,
 
 int nvme_init_ctrl(nvme_host_t h, nvme_ctrl_t c, int instance)
 {
-       _cleanup_free_ char *ctrl_dir = nvme_ctrl_sysfs_dir();
        _cleanup_free_ char *subsys_name = NULL;
        _cleanup_free_ char *name = NULL;
        nvme_subsystem_t s;
@@ -1839,7 +1820,7 @@ int nvme_init_ctrl(nvme_host_t h, nvme_ctrl_t c, int instance)
                errno = ENOMEM;
                return -1;
        }
-       ret = asprintf(&path, "%s/nvme%d", ctrl_dir, instance);
+       ret = asprintf(&path, "%s/%s", nvme_ctrl_sysfs_dir(), name);
        if (ret < 0) {
                errno = ENOMEM;
                return ret;
@@ -1983,11 +1964,10 @@ nvme_ctrl_t nvme_scan_ctrl(nvme_root_t r, const char *name)
        _cleanup_free_ char *path = NULL;
        _cleanup_free_ char *hostnqn = NULL, *hostid = NULL;
        _cleanup_free_ char *subsysnqn = NULL, *subsysname = NULL;
-       _cleanup_free_ char *ctrl_dir = nvme_ctrl_sysfs_dir();
        int ret;
 
        nvme_msg(r, LOG_DEBUG, "scan controller %s\n", name);
-       ret = asprintf(&path, "%s/%s", ctrl_dir, name);
+       ret = asprintf(&path, "%s/%s", nvme_ctrl_sysfs_dir(), name);
        if (ret < 0) {
                errno = ENOMEM;
                return NULL;
@@ -2615,9 +2595,7 @@ static struct nvme_ns *__nvme_scan_namespace(const char *sysfs_dir, const char *
 
 nvme_ns_t nvme_scan_namespace(const char *name)
 {
-       _cleanup_free_ char *ns_dir = nvme_ns_sysfs_dir();
-
-       return __nvme_scan_namespace(ns_dir, name);
+       return __nvme_scan_namespace(nvme_ns_sysfs_dir(), name);
 }
 
 static int nvme_ctrl_scan_namespace(nvme_root_t r, struct nvme_ctrl *c,