[PATCH RFC 2/2] netconsole: Plug to dynamic configfs item

Breno Leitao posted 2 patches 2 months, 1 week ago
[PATCH RFC 2/2] netconsole: Plug to dynamic configfs item
Posted by Breno Leitao 2 months, 1 week ago
Convert netconsole to use the new configfs kernel-space item
registration API for command-line configured targets.

Previously, netconsole created boot-time targets by searching for them
when userspace tried to create an item with the a special name. This
approach was fragile and hard for users to deal with.

This change refactors netconsole to:
  - Call configfs_register_item() directly from populate_configfs_item()
    to properly register boot/module parameter targets.

  - Remove the find_cmdline_target() logic and the special handling in
    make_netconsole_target() that intercepted userspace operations (aka
    the ugly workaround)

This makes the management of netconsole easier, simplifies the code
(removing ~40 lines) and properly separates kernel-created items from
userspace-created items, making the code more maintainable and robust.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netconsole.c | 63 +++++++++++++++++++++------------------------------------------
 1 file changed, 21 insertions(+), 42 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index bb6e03a92956..d3c720a2f9ef 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -225,8 +225,8 @@ static void netconsole_target_put(struct netconsole_target *nt)
 {
 }
 
-static void populate_configfs_item(struct netconsole_target *nt,
-				   int cmdline_count)
+static int populate_configfs_item(struct netconsole_target *nt,
+				  int cmdline_count)
 {
 }
 #endif	/* CONFIG_NETCONSOLE_DYNAMIC */
@@ -1256,23 +1256,6 @@ static void init_target_config_group(struct netconsole_target *nt,
 	configfs_add_default_group(&nt->userdata_group, &nt->group);
 }
 
-static struct netconsole_target *find_cmdline_target(const char *name)
-{
-	struct netconsole_target *nt, *ret = NULL;
-	unsigned long flags;
-
-	spin_lock_irqsave(&target_list_lock, flags);
-	list_for_each_entry(nt, &target_list, list) {
-		if (!strcmp(nt->group.cg_item.ci_name, name)) {
-			ret = nt;
-			break;
-		}
-	}
-	spin_unlock_irqrestore(&target_list_lock, flags);
-
-	return ret;
-}
-
 /*
  * Group operations and type for netconsole_subsys.
  */
@@ -1283,19 +1266,6 @@ static struct config_group *make_netconsole_target(struct config_group *group,
 	struct netconsole_target *nt;
 	unsigned long flags;
 
-	/* Checking if a target by this name was created at boot time.  If so,
-	 * attach a configfs entry to that target.  This enables dynamic
-	 * control.
-	 */
-	if (!strncmp(name, NETCONSOLE_PARAM_TARGET_PREFIX,
-		     strlen(NETCONSOLE_PARAM_TARGET_PREFIX))) {
-		nt = find_cmdline_target(name);
-		if (nt) {
-			init_target_config_group(nt, name);
-			return &nt->group;
-		}
-	}
-
 	nt = alloc_and_init();
 	if (!nt)
 		return ERR_PTR(-ENOMEM);
@@ -1351,14 +1321,20 @@ static struct configfs_subsystem netconsole_subsys = {
 	},
 };
 
-static void populate_configfs_item(struct netconsole_target *nt,
-				   int cmdline_count)
+static int populate_configfs_item(struct netconsole_target *nt,
+				  int cmdline_count)
 {
 	char target_name[16];
+	int ret;
 
 	snprintf(target_name, sizeof(target_name), "%s%d",
 		 NETCONSOLE_PARAM_TARGET_PREFIX, cmdline_count);
+
 	init_target_config_group(nt, target_name);
+
+	ret = configfs_register_item(&netconsole_subsys.su_group,
+				     &nt->group.cg_item);
+	return ret;
 }
 
 static int sysdata_append_cpu_nr(struct netconsole_target *nt, int offset)
@@ -1899,7 +1875,9 @@ static struct netconsole_target *alloc_param_target(char *target_config,
 	} else {
 		nt->enabled = true;
 	}
-	populate_configfs_item(nt, cmdline_count);
+	err = populate_configfs_item(nt, cmdline_count);
+	if (err)
+		goto fail;
 
 	return nt;
 
@@ -1911,6 +1889,8 @@ static struct netconsole_target *alloc_param_target(char *target_config,
 /* Cleanup netpoll for given target (from boot/module param) and free it */
 static void free_param_target(struct netconsole_target *nt)
 {
+	if (nt->group.cg_item.ci_dentry)
+		configfs_unregister_item(&nt->group.cg_item);
 	netpoll_cleanup(&nt->np);
 	kfree(nt);
 }
@@ -1937,6 +1917,10 @@ static int __init init_netconsole(void)
 	char *target_config;
 	char *input = config;
 
+	err = dynamic_netconsole_init();
+	if (err)
+		goto exit;
+
 	if (strnlen(input, MAX_PARAM_LENGTH)) {
 		while ((target_config = strsep(&input, ";"))) {
 			nt = alloc_param_target(target_config, count);
@@ -1966,10 +1950,6 @@ static int __init init_netconsole(void)
 	if (err)
 		goto fail;
 
-	err = dynamic_netconsole_init();
-	if (err)
-		goto undonotifier;
-
 	if (console_type_needed & CONS_EXTENDED)
 		register_console(&netconsole_ext);
 	if (console_type_needed & CONS_BASIC)
@@ -1978,10 +1958,8 @@ static int __init init_netconsole(void)
 
 	return err;
 
-undonotifier:
-	unregister_netdevice_notifier(&netconsole_netdev_notifier);
-
 fail:
+	dynamic_netconsole_exit();
 	pr_err("cleaning up\n");
 
 	/*
@@ -1993,6 +1971,7 @@ static int __init init_netconsole(void)
 		list_del(&nt->list);
 		free_param_target(nt);
 	}
+exit:
 
 	return err;
 }

-- 
2.47.3