tree: Don't open nvme devices until it's absolutely required
authorMartin Belanger <martin.belanger@dell.com>
Mon, 3 Jul 2023 18:41:14 +0000 (14:41 -0400)
committerDaniel Wagner <wagi@monom.org>
Mon, 10 Jul 2023 07:46:32 +0000 (09:46 +0200)
Don't open nvme devices while scanning the tree. Only open devices
when we actually need to write commands to them.

This patch also provides functions to close fds when a user no
longer needs them to be open.

Signed-off-by: Martin Belanger <martin.belanger@dell.com>
src/libnvme.map
src/nvme/private.h
src/nvme/tree.c
src/nvme/tree.h

index 82387d44d711407585efd17bc5eae9612f65a629..83dd25ac8e546bce5f399453235454b20fb39fb1 100644 (file)
@@ -1,4 +1,12 @@
 # SPDX-License-Identifier: LGPL-2.1-or-later
+LIBNVME_1_6 {
+       global:
+               nvme_ctrl_release_fd;
+               nvme_host_release_fds;
+               nvme_ns_release_fd;
+               nvme_root_release_fds;
+               nvme_subsystem_release_fds;
+};
 
 LIBNVME_1_5 {
        global:
index 809b3bb744395ffdda9ca7673891ee728c8e8cff..f4992a48a41e1d6c7a8238c107c4f23296248051 100644 (file)
@@ -197,6 +197,11 @@ __nvme_msg(nvme_root_t r, int lvl, const char *func, const char *format, ...);
                                   format, ##__VA_ARGS__);              \
        } while (0)
 
+#define root_from_ctrl(c) ((c)->s && (c)->s->h ? (c)->s->h->r : NULL)
+#define root_from_ns(n) ((n)->s && (n)->s->h ? (n)->s->h->r : \
+                        (n)->c && (n)->c->s && (n)->c->s->h ? (n)->c->s->h->r : \
+                        NULL)
+
 /* mi internal headers */
 
 /* internal transport API */
index 805eff994a1e8ccdd9ccac25102e74445840948e..3b02fc64746328fdfbae39f0cedb7e62e01570b6 100644 (file)
@@ -341,6 +341,14 @@ void nvme_free_tree(nvme_root_t r)
        free(r);
 }
 
+void nvme_root_release_fds(nvme_root_t r)
+{
+       struct nvme_host *h, *_h;
+
+       nvme_for_each_host_safe(r, h, _h)
+               nvme_host_release_fds(h);
+}
+
 const char *nvme_subsystem_get_nqn(nvme_subsystem_t s)
 {
        return s->subsysnqn;
@@ -412,7 +420,7 @@ nvme_path_t nvme_namespace_next_path(nvme_ns_t ns, nvme_path_t p)
 static void __nvme_free_ns(struct nvme_ns *n)
 {
        list_del_init(&n->entry);
-       close(n->fd);
+       nvme_ns_release_fd(n);
        free(n->generic_name);
        free(n->name);
        free(n->sysfs_dir);
@@ -454,6 +462,18 @@ static void __nvme_free_subsystem(struct nvme_subsystem *s)
        free(s);
 }
 
+void nvme_subsystem_release_fds(struct nvme_subsystem *s)
+{
+       struct nvme_ctrl *c, *_c;
+       struct nvme_ns *n, *_n;
+
+       nvme_subsystem_for_each_ctrl_safe(s, c, _c)
+               nvme_ctrl_release_fd(c);
+
+       nvme_subsystem_for_each_ns_safe(s, n, _n)
+               nvme_ns_release_fd(n);
+}
+
 /*
  * Stub for SWIG
  */
@@ -524,6 +544,14 @@ static void __nvme_free_host(struct nvme_host *h)
        free(h);
 }
 
+void nvme_host_release_fds(struct nvme_host *h)
+{
+       struct nvme_subsystem *s, *_s;
+
+       nvme_for_each_subsystem_safe(h, s, _s)
+               nvme_subsystem_release_fds(s);
+}
+
 /* Stub for SWIG */
 void nvme_free_host(struct nvme_host *h)
 {
@@ -787,18 +815,25 @@ free_path:
 
 int nvme_ctrl_get_fd(nvme_ctrl_t c)
 {
-       nvme_root_t r = c->s && c->s->h ? c->s->h->r : NULL;
-
        if (c->fd < 0) {
                c->fd = nvme_open(c->name);
                if (c->fd < 0)
-                       nvme_msg(r, LOG_ERR,
+                       nvme_msg(root_from_ctrl(c), LOG_ERR,
                                 "Failed to open ctrl %s, errno %d\n",
                                 c->name, errno);
        }
        return c->fd;
 }
 
+void nvme_ctrl_release_fd(nvme_ctrl_t c)
+{
+       if (c->fd < 0)
+               return;
+
+       close(c->fd);
+       c->fd = -1;
+}
+
 nvme_subsystem_t nvme_ctrl_get_subsystem(nvme_ctrl_t c)
 {
        return c->s;
@@ -998,10 +1033,7 @@ nvme_path_t nvme_ctrl_next_path(nvme_ctrl_t c, nvme_path_t p)
        do { if (a) { free(a); (a) = NULL; } } while (0)
 void nvme_deconfigure_ctrl(nvme_ctrl_t c)
 {
-       if (c->fd >= 0) {
-               close(c->fd);
-               c->fd = -1;
-       }
+       nvme_ctrl_release_fd(c);
        FREE_CTRL_ATTR(c->name);
        FREE_CTRL_ATTR(c->sysfs_dir);
        FREE_CTRL_ATTR(c->firmware);
@@ -1289,7 +1321,8 @@ static char *nvme_ctrl_lookup_phy_slot(nvme_root_t r, const char *address)
                if (entry->d_type == DT_DIR &&
                    strncmp(entry->d_name, ".", 1) != 0 &&
                    strncmp(entry->d_name, "..", 2) != 0) {
-                       ret = asprintf(&path, "/sys/bus/pci/slots/%s", entry->d_name);
+                       ret = asprintf(&path, "%s/%s",
+                                      nvme_slots_sysfs_dir, entry->d_name);
                        if (ret < 0) {
                                errno = ENOMEM;
                                free(target_addr);
@@ -1625,9 +1658,26 @@ static int nvme_bytes_to_lba(nvme_ns_t n, off_t offset, size_t count,
 
 int nvme_ns_get_fd(nvme_ns_t n)
 {
+       if (n->fd < 0) {
+               n->fd = nvme_open(n->name);
+               if (n->fd < 0)
+                       nvme_msg(root_from_ns(n), LOG_ERR,
+                                "Failed to open ns %s, errno %d\n",
+                                n->name, errno);
+       }
+
        return n->fd;
 }
 
+void nvme_ns_release_fd(nvme_ns_t n)
+{
+       if (n->fd < 0)
+               return;
+
+       close(n->fd);
+       n->fd = -1;
+}
+
 nvme_subsystem_t nvme_ns_get_subsystem(nvme_ns_t n)
 {
        return n->s;
@@ -1962,6 +2012,7 @@ static void nvme_ns_set_generic_name(struct nvme_ns *n, const char *name)
 static nvme_ns_t nvme_ns_open(const char *name)
 {
        struct nvme_ns *n;
+       int fd;
 
        n = calloc(1, sizeof(*n));
        if (!n) {
@@ -1969,27 +2020,29 @@ static nvme_ns_t nvme_ns_open(const char *name)
                return NULL;
        }
 
+       n->fd = -1;
        n->name = strdup(name);
-       n->fd = nvme_open(n->name);
-       if (n->fd < 0)
+
+       fd = nvme_ns_get_fd(n);
+       if (fd < 0)
                goto free_ns;
 
        nvme_ns_set_generic_name(n, name);
 
-       if (nvme_get_nsid(n->fd, &n->nsid) < 0)
-               goto close_fd;
+       if (nvme_get_nsid(fd, &n->nsid) < 0)
+               goto free_ns;
 
        if (nvme_ns_init(n) != 0)
-               goto close_fd;
+               goto free_ns;
 
        list_head_init(&n->paths);
        list_node_init(&n->entry);
 
+       nvme_ns_release_fd(n); /* Do not leak fds */
        return n;
 
-close_fd:
-       close(n->fd);
 free_ns:
+       nvme_ns_release_fd(n);
        free(n->generic_name);
        free(n->name);
        free(n);
index bcf3636f3c333f5da507fdbe0a4f8b9282f251c6..a82019e042bf0867a7e811c9c97754d79b4659c2 100644 (file)
@@ -61,6 +61,17 @@ void nvme_root_set_application(nvme_root_t r, const char *a);
  */
 const char *nvme_root_get_application(nvme_root_t r);
 
+/**
+ * nvme_root_release_fds - Close all opened file descriptors in the tree
+ * @r: &nvme_root_t object
+ *
+ * Controller and Namespace objects cache the file descriptors
+ * of opened nvme devices. This API can be used to close and
+ * clear all cached fds in the tree.
+ *
+ */
+void nvme_root_release_fds(nvme_root_t r);
+
 /**
  * nvme_free_tree() - Free root object
  * @r: &nvme_root_t object
@@ -484,10 +495,24 @@ nvme_ns_t nvme_subsystem_next_ns(nvme_subsystem_t s, nvme_ns_t n);
  * nvme_ns_get_fd() - Get associated file descriptor
  * @n: Namespace instance
  *
+ * libnvme will open() the file (if not already opened) and keep
+ * an internal copy of the file descriptor. Following calls to
+ * this API retrieve the internal cached copy of the file
+ * descriptor. The file will remain opened and the fd will
+ * remain cached until the ns object is deleted or
+ * nvme_ns_release_fd() is called.
+ *
  * Return: File descriptor associated with @n or -1
  */
 int nvme_ns_get_fd(nvme_ns_t n);
 
+/**
+ * nvme_ns_release_fd() - Close fd and clear fd from ns object
+ * @n: Namespace instance
+ *
+ */
+void nvme_ns_release_fd(nvme_ns_t n);
+
 /**
  * nvme_ns_get_nsid() - NSID of a namespace
  * @n: Namespace instance
@@ -772,10 +797,24 @@ nvme_ns_t nvme_path_get_ns(nvme_path_t p);
  * nvme_ctrl_get_fd() - Get associated file descriptor
  * @c: Controller instance
  *
+ * libnvme will open() the file (if not already opened) and keep
+ * an internal copy of the file descriptor. Following calls to
+ * this API retrieve the internal cached copy of the file
+ * descriptor. The file will remain opened and the fd will
+ * remain cached until the controller object is deleted or
+ * nvme_ctrl_release_fd() is called.
+ *
  * Return: File descriptor associated with @c or -1
  */
 int nvme_ctrl_get_fd(nvme_ctrl_t c);
 
+/**
+ * nvme_ctrl_release_fd() - Close fd and clear fd from controller object
+ * @c: Controller instance
+ *
+ */
+void nvme_ctrl_release_fd(nvme_ctrl_t c);
+
 /**
  * nvme_ctrl_get_name() - sysfs name of a controller
  * @c: Controller instance
@@ -1176,6 +1215,16 @@ const char *nvme_host_get_hostnqn(nvme_host_t h);
  */
 const char *nvme_host_get_hostid(nvme_host_t h);
 
+/**
+ * nvme_host_release_fds() - Close all opened file descriptors under host
+ * @h: nvme_host_t object
+ *
+ * Controller and Namespace objects cache the file descriptors
+ * of opened nvme devices. This API can be used to close and
+ * clear all cached fds under this host.
+ */
+void nvme_host_release_fds(struct nvme_host *h);
+
 /**
  * nvme_free_host() - Free nvme_host_t object
  * @h: nvme_host_t object
@@ -1292,6 +1341,18 @@ char *nvme_get_ns_attr(nvme_ns_t n, const char *attr);
 nvme_ns_t nvme_subsystem_lookup_namespace(struct nvme_subsystem *s,
                                          __u32 nsid);
 
+/**
+ * nvme_subsystem_release_fds() - Close all opened fds under subsystem
+ * @s:         nvme_subsystem_t object
+ *
+ * Controller and Namespace objects cache the file descriptors
+ * of opened nvme devices. This API can be used to close and
+ * clear all cached fds under this subsystem.
+ *
+ */
+void nvme_subsystem_release_fds(struct nvme_subsystem *s);
+
+
 /**
  * nvme_get_path_attr() - Read path sysfs attribute
  * @p:         nvme_path_t object