tree: Don't open nvme devices until it's absolutely required

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>

diff --git a/src/libnvme.map b/src/libnvme.map
index 82387d44d711407585efd17bc5eae9612f65a629..83dd25ac8e546bce5f399453235454b20fb39fb1 100644 (file)
--- a/src/libnvme.map
+++ b/src/libnvme.map
@@ -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:

diff --git a/src/nvme/private.h b/src/nvme/private.h
index 809b3bb744395ffdda9ca7673891ee728c8e8cff..f4992a48a41e1d6c7a8238c107c4f23296248051 100644 (file)
--- a/src/nvme/private.h
+++ b/src/nvme/private.h
@@ -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 */

diff --git a/src/nvme/tree.c b/src/nvme/tree.c
index 805eff994a1e8ccdd9ccac25102e74445840948e..3b02fc64746328fdfbae39f0cedb7e62e01570b6 100644 (file)
--- a/src/nvme/tree.c
+++ b/src/nvme/tree.c
@@ -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);

diff --git a/src/nvme/tree.h b/src/nvme/tree.h
index bcf3636f3c333f5da507fdbe0a4f8b9282f251c6..a82019e042bf0867a7e811c9c97754d79b4659c2 100644 (file)
--- a/src/nvme/tree.h
+++ b/src/nvme/tree.h
@@ -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