kstat: allow multi-level module names

include/os/linux/spl/sys/kstat.h

@@ -21,6 +21,10 @@
  *  You should have received a copy of the GNU General Public License along
  *  with the SPL.  If not, see <http://www.gnu.org/licenses/>.
  */
+/*
+ * Copyright (c) 2024-2025, Klara, Inc.
+ * Copyright (c) 2024-2025, Syneto
+ */
 
 #ifndef _SPL_KSTAT_H
 #define	_SPL_KSTAT_H
@@ -90,6 +94,8 @@ typedef struct kstat_module {
 	struct list_head ksm_module_list;	/* module linkage */
 	struct list_head ksm_kstat_list;	/* list of kstat entries */
 	struct proc_dir_entry *ksm_proc;	/* proc entry */
+	struct kstat_module *ksm_parent;	/* parent module in hierarchy */
+	uint_t ksm_nchildren;			/* number of child modules */
 } kstat_module_t;
 
 typedef struct kstat_raw_ops {

module/os/freebsd/spl/spl_kstat.c

@@ -28,6 +28,10 @@
  * [1] https://illumos.org/man/1M/kstat
  * [2] https://illumos.org/man/9f/kstat_create
  */
+/*
+ * Copyright (c) 2024-2025, Klara, Inc.
+ * Copyright (c) 2024-2025, Syneto
+ */
 
 #include <sys/types.h>
 #include <sys/param.h>
@@ -288,7 +292,7 @@ __kstat_create(const char *module, int instance, const char *name,
 	char buf[KSTAT_STRLEN];
 	struct sysctl_oid *root;
 	kstat_t *ksp;
-	char *pool;
+	char *p, *frag;
 
 	KASSERT(instance == 0, ("instance=%d", instance));
 	if ((ks_type == KSTAT_TYPE_INTR) || (ks_type == KSTAT_TYPE_IO))
@@ -346,74 +350,54 @@ __kstat_create(const char *module, int instance, const char *name,
 	else
 		ksp->ks_data = kmem_zalloc(ksp->ks_data_size, KM_SLEEP);
 
-	/*
-	 * Some kstats use a module name like "zfs/poolname" to distinguish a
-	 * set of kstats belonging to a specific pool.  Split on '/' to add an
-	 * extra node for the pool name if needed.
-	 */
+	sysctl_ctx_init(&ksp->ks_sysctl_ctx);
+
 	(void) strlcpy(buf, module, KSTAT_STRLEN);
-	module = buf;
-	pool = strchr(module, '/');
-	if (pool != NULL)
-		*pool++ = '\0';
 
 	/*
-	 * Create sysctl tree for those statistics:
-	 *
-	 *	kstat.<module>[.<pool>].<class>.<name>
+	 * Walk over the module name, splitting on '/', and create the
+	 * intermediate nodes.
 	 */
-	sysctl_ctx_init(&ksp->ks_sysctl_ctx);
-	root = SYSCTL_ADD_NODE(&ksp->ks_sysctl_ctx,
-	    SYSCTL_STATIC_CHILDREN(_kstat), OID_AUTO, module, CTLFLAG_RW, 0,
-	    "");
-	if (root == NULL) {
-		printf("%s: Cannot create kstat.%s tree!\n", __func__, module);
-		sysctl_ctx_free(&ksp->ks_sysctl_ctx);
-		free(ksp, M_KSTAT);
-		return (NULL);
-	}
-	if (pool != NULL) {
-		root = SYSCTL_ADD_NODE(&ksp->ks_sysctl_ctx,
-		    SYSCTL_CHILDREN(root), OID_AUTO, pool, CTLFLAG_RW, 0, "");
+	root = NULL;
+	p = buf;
+	while ((frag = strsep(&p, "/")) != NULL) {
+		root = SYSCTL_ADD_NODE(&ksp->ks_sysctl_ctx, root ?
+		    SYSCTL_CHILDREN(root) : SYSCTL_STATIC_CHILDREN(_kstat),
+		    OID_AUTO, frag, CTLFLAG_RW, 0, "");
 		if (root == NULL) {
-			printf("%s: Cannot create kstat.%s.%s tree!\n",
-			    __func__, module, pool);
+			printf("%s: Cannot create kstat.%s tree!\n",
+			    __func__, buf);
 			sysctl_ctx_free(&ksp->ks_sysctl_ctx);
 			free(ksp, M_KSTAT);
 			return (NULL);
 		}
+		if (p != NULL)
+			p[-1] = '.';
 	}
+
 	root = SYSCTL_ADD_NODE(&ksp->ks_sysctl_ctx, SYSCTL_CHILDREN(root),
 	    OID_AUTO, class, CTLFLAG_RW, 0, "");
 	if (root == NULL) {
-		if (pool != NULL)
-			printf("%s: Cannot create kstat.%s.%s.%s tree!\n",
-			    __func__, module, pool, class);
-		else
-			printf("%s: Cannot create kstat.%s.%s tree!\n",
-			    __func__, module, class);
+		printf("%s: Cannot create kstat.%s.%s tree!\n",
+		    __func__, buf, class);
 		sysctl_ctx_free(&ksp->ks_sysctl_ctx);
 		free(ksp, M_KSTAT);
 		return (NULL);
 	}
+
 	if (ksp->ks_type == KSTAT_TYPE_NAMED) {
 		root = SYSCTL_ADD_NODE(&ksp->ks_sysctl_ctx,
 		    SYSCTL_CHILDREN(root),
 		    OID_AUTO, name, CTLFLAG_RW, 0, "");
 		if (root == NULL) {
-			if (pool != NULL)
-				printf("%s: Cannot create kstat.%s.%s.%s.%s "
-				    "tree!\n", __func__, module, pool, class,
-				    name);
-			else
-				printf("%s: Cannot create kstat.%s.%s.%s "
-				    "tree!\n", __func__, module, class, name);
+			printf("%s: Cannot create kstat.%s.%s.%s tree!\n",
+			    __func__, buf, class, name);
 			sysctl_ctx_free(&ksp->ks_sysctl_ctx);
 			free(ksp, M_KSTAT);
 			return (NULL);
 		}
-
 	}
+
 	ksp->ks_sysctl_root = root;
 
 	return (ksp);
@@ -437,7 +421,26 @@ kstat_install_named(kstat_t *ksp)
 		if (ksent->data_type != 0) {
 			typelast = ksent->data_type;
 			namelast = ksent->name;
+
+			/*
+			 * If a sysctl with this name already exists on this on
+			 * this root, first remove it by deleting it from its
+			 * old context, and then destroying it.
+			 */
+			struct sysctl_oid *oid = NULL;
+			SYSCTL_FOREACH(oid,
+			    SYSCTL_CHILDREN(ksp->ks_sysctl_root)) {
+				if (strcmp(oid->oid_name, namelast) == 0) {
+					kstat_t *oldksp =
+					    (kstat_t *)oid->oid_arg1;
+					sysctl_ctx_entry_del(
+					    &oldksp->ks_sysctl_ctx, oid);
+					sysctl_remove_oid(oid, 1, 0);
+					break;
+				}
+			}
 		}
+
 		switch (typelast) {
 		case KSTAT_DATA_CHAR:
 			/* Not Implemented */

module/os/linux/spl/spl-kstat.c

@@ -27,6 +27,10 @@
  *  [1] https://illumos.org/man/1M/kstat
  *  [2] https://illumos.org/man/9f/kstat_create
  */
+/*
+ * Copyright (c) 2024-2025, Klara, Inc.
+ * Copyright (c) 2024-2025, Syneto
+ */
 
 #include <linux/seq_file.h>
 #include <sys/kstat.h>
@@ -370,6 +374,8 @@ static const struct seq_operations kstat_seq_ops = {
 static kstat_module_t *
 kstat_find_module(char *name)
 {
+	ASSERT(MUTEX_HELD(&kstat_module_lock));
+
 	kstat_module_t *module = NULL;
 
 	list_for_each_entry(module, &kstat_module_list, ksm_module_list) {
@@ -380,33 +386,75 @@ kstat_find_module(char *name)
 	return (NULL);
 }
 
-static kstat_module_t *
-kstat_create_module(char *name)
+static void
+kstat_delete_module(kstat_module_t *module)
 {
-	kstat_module_t *module;
-	struct proc_dir_entry *pde;
+	ASSERT(MUTEX_HELD(&kstat_module_lock));
+	ASSERT(list_empty(&module->ksm_kstat_list));
+	ASSERT0(module->ksm_nchildren);
 
-	pde = proc_mkdir(name, proc_spl_kstat);
-	if (pde == NULL)
-		return (NULL);
+	kstat_module_t *parent = module->ksm_parent;
 
-	module = kmem_alloc(sizeof (kstat_module_t), KM_SLEEP);
-	module->ksm_proc = pde;
-	strlcpy(module->ksm_name, name, KSTAT_STRLEN);
-	INIT_LIST_HEAD(&module->ksm_kstat_list);
-	list_add_tail(&module->ksm_module_list, &kstat_module_list);
+	char *p = module->ksm_name, *frag;
+	while (p != NULL && (frag = strsep(&p, "/"))) {}
 
-	return (module);
+	remove_proc_entry(frag, parent ? parent->ksm_proc : proc_spl_kstat);
+	list_del(&module->ksm_module_list);
+	kmem_free(module, sizeof (kstat_module_t));
 
+	if (parent) {
+		parent->ksm_nchildren--;
+		if (parent->ksm_nchildren == 0 &&
+		    list_empty(&parent->ksm_kstat_list))
+			kstat_delete_module(parent);
+	}
 }
 
-static void
-kstat_delete_module(kstat_module_t *module)
+static kstat_module_t *
+kstat_create_module(char *name)
 {
-	ASSERT(list_empty(&module->ksm_kstat_list));
-	remove_proc_entry(module->ksm_name, proc_spl_kstat);
-	list_del(&module->ksm_module_list);
-	kmem_free(module, sizeof (kstat_module_t));
+	ASSERT(MUTEX_HELD(&kstat_module_lock));
+
+	char buf[KSTAT_STRLEN];
+	kstat_module_t *module, *parent;
+
+	(void) strlcpy(buf, name, KSTAT_STRLEN);
+
+	parent = NULL;
+	char *p = buf, *frag;
+	while ((frag = strsep(&p, "/")) != NULL) {
+		module = kstat_find_module(buf);
+		if (module == NULL) {
+			struct proc_dir_entry *pde = proc_mkdir(frag,
+			    parent ? parent->ksm_proc : proc_spl_kstat);
+			if (pde == NULL) {
+				cmn_err(CE_WARN, "kstat_create('%s'): "
+				    "module dir create failed", buf);
+				if (parent)
+					kstat_delete_module(parent);
+				return (NULL);
+			}
+
+			module = kmem_zalloc(sizeof (kstat_module_t), KM_SLEEP);
+			module->ksm_proc = pde;
+			strlcpy(module->ksm_name, buf, KSTAT_STRLEN);
+			INIT_LIST_HEAD(&module->ksm_kstat_list);
+			list_add_tail(&module->ksm_module_list,
+			    &kstat_module_list);
+
+			if (parent != NULL) {
+				module->ksm_parent = parent;
+				parent->ksm_nchildren++;
+			}
+		}
+
+		parent = module;
+		if (p != NULL)
+			p[-1] = '/';
+	}
+
+	return (module);
+
 }
 
 static int
@@ -625,12 +673,20 @@ kstat_proc_entry_install(kstat_proc_entry_t *kpep, mode_t mode,
 	}
 
 	/*
-	 * Only one entry by this name per-module, on failure the module
-	 * shouldn't be deleted because we know it has at least one entry.
+	 * We can only have one entry of this name per module. If one already
+	 * exists, replace it by first removing the proc entry, then removing
+	 * it from the list. The kstat itself lives on; it just can't be
+	 * inspected through the filesystem.
 	 */
 	list_for_each_entry(tmp, &module->ksm_kstat_list, kpe_list) {
-		if (strncmp(tmp->kpe_name, kpep->kpe_name, KSTAT_STRLEN) == 0)
-			goto out;
+		if (tmp->kpe_proc != NULL &&
+		    strncmp(tmp->kpe_name, kpep->kpe_name, KSTAT_STRLEN) == 0) {
+			ASSERT3P(tmp->kpe_owner, ==, module);
+			remove_proc_entry(tmp->kpe_name, module->ksm_proc);
+			tmp->kpe_proc = NULL;
+			list_del_init(&tmp->kpe_list);
+			break;
+		}
 	}
 
 	list_add_tail(&kpep->kpe_list, &module->ksm_kstat_list);