From: Hannes Reinecke <hare@suse.de>
Date: Wed, 23 Jun 2021 09:51:10 +0000 (+0200)
Subject: tree: rework topology deallocation
X-Git-Tag: v1.0-rc0~123^2
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=5c1eb92dcb4db259bec42d2d2369e87dfc68535f;p=users%2Fsagi%2Flibnvme.git

tree: rework topology deallocation

The original idea of implementing a simple refcount doesn't really
work out, as refcounting is everything but.
And especially when used with the SWIG bindings it's not easy to
figure if and when a refcount needs to be released.
So kill the entire refcounting and free the internal structure
only when calling nvme_free_tree(); the object-dependent functions
like nvme_free_subsystem() etc now merely become stubs for
symmetry and to keep SWIG happy.
That removes the need for refcounting at all as the tree
and its contents will always be available, and a call to
nvme_free_tree() will clear the entire structure.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---

diff --git a/src/nvme/private.h b/src/nvme/private.h
index b6bdfa25..a500f2d9 100644
--- a/src/nvme/private.h
+++ b/src/nvme/private.h
@@ -59,7 +59,6 @@ struct nvme_ctrl {
 	struct list_head paths;
 	struct list_head namespaces;
 	struct nvme_subsystem *s;
-	int refcount;
 
 	int fd;
 	char *name;
@@ -90,7 +89,6 @@ struct nvme_subsystem {
 	struct list_head ctrls;
 	struct list_head namespaces;
 	struct nvme_host *h;
-	int refcount;
 
 	char *name;
 	char *sysfs_dir;
@@ -104,7 +102,6 @@ struct nvme_host {
 	struct list_node entry;
 	struct list_head subsystems;
 	struct nvme_root *r;
-	int refcount;
 
 	char *hostnqn;
 	char *hostid;
@@ -113,7 +110,6 @@ struct nvme_host {
 struct nvme_root {
 	char *config_file;
 	struct list_head hosts;
-	int refcount;
 	bool modified;
 };
 
diff --git a/src/nvme/tree.c b/src/nvme/tree.c
index be710b6e..7fa4f262 100644
--- a/src/nvme/tree.c
+++ b/src/nvme/tree.c
@@ -32,6 +32,8 @@
 
 static struct nvme_host *default_host;
 
+static void __nvme_free_host(nvme_host_t h);
+static void __nvme_free_ctrl(nvme_ctrl_t c);
 static int nvme_subsystem_scan_namespace(struct nvme_subsystem *s, char *name);
 static int nvme_scan_subsystem(struct nvme_root *r, char *name,
 			       nvme_scan_filter_t f);
@@ -90,7 +92,6 @@ nvme_root_t nvme_scan_filter(nvme_scan_filter_t f)
 	}
 
 	list_head_init(&r->hosts);
-	r->refcount = 1;
 	nvme_scan_topology(r, f);
 	return r;
 }
@@ -153,7 +154,7 @@ void nvme_refresh_topology(nvme_root_t r)
 	struct nvme_host *h, *_h;
 
 	nvme_for_each_host_safe(r, h, _h)
-		nvme_free_host(h);
+		__nvme_free_host(h);
 	nvme_scan_topology(r, NULL);
 }
 
@@ -162,7 +163,7 @@ void nvme_reset_topology(nvme_root_t r)
 	struct nvme_host *h, *_h;
 
 	nvme_for_each_host_safe(r, h, _h)
-		nvme_free_host(h);
+		__nvme_free_host(h);
 	nvme_scan_topology(r, NULL);
 }
 
@@ -171,7 +172,7 @@ void nvme_free_tree(nvme_root_t r)
 	struct nvme_host *h, *_h;
 
 	nvme_for_each_host_safe(r, h, _h)
-		nvme_free_host(h);
+		__nvme_free_host(h);
 	if (r->config_file)
 		free(r->config_file);
 	free(r);
@@ -217,7 +218,7 @@ nvme_ns_t nvme_subsystem_next_ns(nvme_subsystem_t s, nvme_ns_t n)
 	return n ? list_next(&s->namespaces, n, entry) : NULL;
 }
 
-void nvme_free_ns(struct nvme_ns *n)
+static void __nvme_free_ns(struct nvme_ns *n)
 {
 	list_del_init(&n->entry);
 	close(n->fd);
@@ -226,20 +227,22 @@ void nvme_free_ns(struct nvme_ns *n)
 	free(n);
 }
 
-void nvme_free_subsystem(struct nvme_subsystem *s)
+/* Stub for SWIG */
+void nvme_free_ns(struct nvme_ns *n)
+{
+}
+
+static void __nvme_free_subsystem(struct nvme_subsystem *s)
 {
 	struct nvme_ctrl *c, *_c;
 	struct nvme_ns *n, *_n;
 
-	if (--s->refcount > 0)
-		return;
-
 	list_del_init(&s->entry);
 	nvme_subsystem_for_each_ctrl_safe(s, c, _c)
-		nvme_free_ctrl(c);
+		__nvme_free_ctrl(c);
 
 	nvme_subsystem_for_each_ns_safe(s, n, _n)
-		nvme_free_ns(n);
+		__nvme_free_ns(n);
 
 	free(s->name);
 	free(s->sysfs_dir);
@@ -253,6 +256,13 @@ void nvme_free_subsystem(struct nvme_subsystem *s)
 	free(s);
 }
 
+/*
+ * Stub for SWIG
+ */
+void nvme_free_subsystem(nvme_subsystem_t s)
+{
+}
+
 struct nvme_subsystem *nvme_lookup_subsystem(struct nvme_host *h,
 					     const char *name,
 					     const char *subsysnqn)
@@ -265,7 +275,6 @@ struct nvme_subsystem *nvme_lookup_subsystem(struct nvme_host *h,
 		if (name && s->name &&
 		    strcmp(s->name, name))
 			continue;
-		s->refcount++;
 		return s;
 	}
 	s = calloc(1, sizeof(*s));
@@ -274,7 +283,6 @@ struct nvme_subsystem *nvme_lookup_subsystem(struct nvme_host *h,
 
 	s->h = h;
 	s->subsysnqn = strdup(subsysnqn);
-	s->refcount = 1;
 	list_head_init(&s->ctrls);
 	list_head_init(&s->namespaces);
 	list_node_init(&s->entry);
@@ -283,15 +291,13 @@ struct nvme_subsystem *nvme_lookup_subsystem(struct nvme_host *h,
 	return s;
 }
 
-void nvme_free_host(struct nvme_host *h)
+static void __nvme_free_host(struct nvme_host *h)
 {
 	struct nvme_subsystem *s, *_s;
 
-	if (--h->refcount > 0)
-		return;
 	list_del_init(&h->entry);
 	nvme_for_each_subsystem_safe(h, s, _s)
-		nvme_free_subsystem(s);
+		__nvme_free_subsystem(s);
 	free(h->hostnqn);
 	if (h->hostid)
 		free(h->hostid);
@@ -299,6 +305,11 @@ void nvme_free_host(struct nvme_host *h)
 	free(h);
 }
 
+/* Stub for SWIG */
+void nvme_free_host(struct nvme_host *h)
+{
+}
+
 struct nvme_host *nvme_lookup_host(nvme_root_t r, const char *hostnqn,
 				   const char *hostid)
 {
@@ -312,7 +323,6 @@ struct nvme_host *nvme_lookup_host(nvme_root_t r, const char *hostnqn,
 		if (hostid &&
 		    strcmp(h->hostid, hostid))
 			continue;
-		h->refcount++;
 		return h;
 	}
 	h = calloc(1,sizeof(*h));
@@ -324,7 +334,6 @@ struct nvme_host *nvme_lookup_host(nvme_root_t r, const char *hostnqn,
 	list_head_init(&h->subsystems);
 	list_node_init(&h->entry);
 	h->r = r;
-	h->refcount = 1;
 	list_add(&r->hosts, &h->entry);
 	r->modified = true;
 
@@ -428,7 +437,7 @@ static int nvme_scan_subsystem(struct nvme_root *r, char *name,
 	nvme_subsystem_scan_ctrls(s);
 
 	if (f && !f(s)) {
-		nvme_free_subsystem(s);
+		__nvme_free_subsystem(s);
 		return -1;
 	}
 
@@ -741,21 +750,18 @@ void nvme_unlink_ctrl(nvme_ctrl_t c)
 	c->s = NULL;
 }
 
-void nvme_free_ctrl(nvme_ctrl_t c)
+static void __nvme_free_ctrl(nvme_ctrl_t c)
 {
 	struct nvme_path *p, *_p;
 	struct nvme_ns *n, *_n;
 
-	if (--c->refcount > 0)
-		return;
-
 	nvme_unlink_ctrl(c);
 
 	nvme_ctrl_for_each_path_safe(c, p, _p)
 		nvme_free_path(p);
 
 	nvme_ctrl_for_each_ns_safe(c, n, _n)
-		nvme_free_ns(n);
+		__nvme_free_ns(n);
 
 	if (c->fd >= 0)
 		close(c->fd);
@@ -784,6 +790,11 @@ void nvme_free_ctrl(nvme_ctrl_t c)
 	free(c);
 }
 
+/* Stub for SWIG */
+void nvme_free_ctrl(nvme_ctrl_t c)
+{
+}
+
 #define ____stringify(x...) #x
 #define __stringify(x...) ____stringify(x)
 
@@ -902,7 +913,7 @@ struct nvme_ctrl *nvme_create_ctrl(const char *subsysnqn, const char *transport,
 		 !strncmp(transport, "tcp", 3)) {
 		nvme_msg(LOG_ERR, "No trsvcid specified for '%s'\n",
 			 transport);
-		nvme_free_ctrl(c);
+		__nvme_free_ctrl(c);
 		c = NULL;
 	}
 
@@ -932,14 +943,12 @@ struct nvme_ctrl *nvme_lookup_ctrl(struct nvme_subsystem *s, const char *transpo
 		if (trsvcid && c->trsvcid &&
 		    strcmp(c->trsvcid, trsvcid))
 			continue;
-		c->refcount++;
 		return c;
 	}
 	c = nvme_create_ctrl(s->subsysnqn, transport, traddr,
 			     host_traddr, host_iface, trsvcid);
 	if (c) {
 		c->s = s;
-		c->refcount = 1;
 		list_add(&s->ctrls, &c->entry);
 		s->h->r->modified = true;
 	}