Skip to content

Commit

Permalink
sysfs: minimize heap allocations of sysfs paths
Browse files Browse the repository at this point in the history
11a0918 ("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: 11a0918 ("nvme: allow to overwrite base sysfs path")
Signed-off-by: Caleb Sander Mateos <[email protected]>
  • Loading branch information
calebsander authored and igaw committed Mar 20, 2024
1 parent c2f23b3 commit eb4f794
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 126 deletions.
1 change: 1 addition & 0 deletions src/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
37 changes: 2 additions & 35 deletions src/nvme/fabrics.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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};
Expand Down
62 changes: 3 additions & 59 deletions src/nvme/filters.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,68 +6,12 @@
* Authors: Keith Busch <[email protected]>
* Chaitanya Kulkarni <[email protected]>
*/
#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)
{
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
9 changes: 6 additions & 3 deletions src/nvme/private.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
86 changes: 86 additions & 0 deletions src/nvme/sysfs.c
Original file line number Diff line number Diff line change
@@ -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);
}
36 changes: 7 additions & 29 deletions src/nvme/tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit eb4f794

Please sign in to comment.