From cdf6d00696865ae1c46750059fd7d248323712f9 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 7 Mar 2019 16:27:37 -0800 Subject: dynamic_debug: don't duplicate modname in ddebug_add_module For built-in modules, we're already reusing the passed-in string via kstrdup_const(). But for actual modules (i.e. when we're called from dynamic_debug_setup in module.c), the passed-in string (which points at the name[] array inside struct module) is also guaranteed to live at least as long as the struct ddebug_table, since free_module() calls ddebug_remove_module(). Link: http://lkml.kernel.org/r/20190212214150.4807-6-linux@rasmusvillemoes.dk Signed-off-by: Rasmus Villemoes Acked-by: Jason Baron Cc: David Sterba Cc: Greg Kroah-Hartman Cc: Ingo Molnar Cc: Petr Mladek Cc: "Rafael J . Wysocki" Cc: Steven Rostedt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/dynamic_debug.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'lib/dynamic_debug.c') diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index dbf2b457e47e..8274c4ea75e0 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -847,17 +847,17 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n, const char *name) { struct ddebug_table *dt; - const char *new_name; dt = kzalloc(sizeof(*dt), GFP_KERNEL); if (dt == NULL) return -ENOMEM; - new_name = kstrdup_const(name, GFP_KERNEL); - if (new_name == NULL) { - kfree(dt); - return -ENOMEM; - } - dt->mod_name = new_name; + /* + * For built-in modules, name lives in .rodata and is + * immortal. For loaded modules, name points at the name[] + * member of struct module, which lives at least as long as + * this struct ddebug_table. + */ + dt->mod_name = name; dt->num_ddebugs = n; dt->ddebugs = tab; @@ -913,7 +913,6 @@ int ddebug_dyndbg_module_param_cb(char *param, char *val, const char *module) static void ddebug_table_free(struct ddebug_table *dt) { list_del_init(&dt->link); - kfree_const(dt->mod_name); kfree(dt); } -- cgit From 4573fe15437c909d5a06a01750125e2a06829370 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 7 Mar 2019 16:27:41 -0800 Subject: dynamic_debug: use pointer comparison in ddebug_remove_module Now that we store the passed-in string directly in ddebug_add_module, we can use pointer equality instead of strcmp. This is a little more efficient, but more importantly, this also makes the code somewhat more correct: Currently, if one loads and then unloads a module whose name happens to match the KBUILD_MODNAME of some built-in functionality (which need not even be modular at all), all of their dynamic debug entries vanish along with those of the actual module. For example, loading and unloading a core.ko hides all pr_debugs from drivers/base/core.c and other built-in files called core.c (incidentally, there is an in-tree module whose name is core, but I just tested this with an out-of-tree trivial one). Link: http://lkml.kernel.org/r/20190212214150.4807-7-linux@rasmusvillemoes.dk Signed-off-by: Rasmus Villemoes Acked-by: Jason Baron Cc: David Sterba Cc: Greg Kroah-Hartman Cc: Ingo Molnar Cc: Petr Mladek Cc: "Rafael J . Wysocki" Cc: Steven Rostedt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/dynamic_debug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'lib/dynamic_debug.c') diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 8274c4ea75e0..214828c65625 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -929,9 +929,10 @@ int ddebug_remove_module(const char *mod_name) mutex_lock(&ddebug_lock); list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) { - if (!strcmp(dt->mod_name, mod_name)) { + if (dt->mod_name == mod_name) { ddebug_table_free(dt); ret = 0; + break; } } mutex_unlock(&ddebug_lock); -- cgit From f008043bd3b5ca7f2c65dbdad8ea6df0a6f134f3 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 7 Mar 2019 16:27:45 -0800 Subject: dynamic_debug: remove unused EXPORT_SYMBOLs The only caller of ddebug_{add,remove}_module outside dynamic_debug.c is kernel/module.c, which is obviously not itself modular (though it would be an interesting exercise to make that happen...). I also fail to see how these interfaces can be used by modules, in-tree or not. Link: http://lkml.kernel.org/r/20190212214150.4807-8-linux@rasmusvillemoes.dk Signed-off-by: Rasmus Villemoes Acked-by: Jason Baron Cc: David Sterba Cc: Greg Kroah-Hartman Cc: Ingo Molnar Cc: Petr Mladek Cc: "Rafael J . Wysocki" Cc: Steven Rostedt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/dynamic_debug.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'lib/dynamic_debug.c') diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 214828c65625..7b76f43edaef 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -868,7 +868,6 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n, vpr_info("%u debug prints in module %s\n", n, dt->mod_name); return 0; } -EXPORT_SYMBOL_GPL(ddebug_add_module); /* helper for ddebug_dyndbg_(boot|module)_param_cb */ static int ddebug_dyndbg_param_cb(char *param, char *val, @@ -938,7 +937,6 @@ int ddebug_remove_module(const char *mod_name) mutex_unlock(&ddebug_lock); return ret; } -EXPORT_SYMBOL_GPL(ddebug_remove_module); static void ddebug_remove_all_tables(void) { -- cgit From 513770f54edba8b19c2175a151e02f1dfc911d87 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Thu, 7 Mar 2019 16:27:48 -0800 Subject: dynamic_debug: move pr_err from module.c to ddebug_add_module This serves two purposes: First, we get a diagnostic if (though extremely unlikely), any of the calls of ddebug_add_module for built-in code fails, effectively disabling dynamic_debug. Second, I want to make struct _ddebug opaque, and avoid accessing any of its members outside dynamic_debug.[ch]. Link: http://lkml.kernel.org/r/20190212214150.4807-9-linux@rasmusvillemoes.dk Signed-off-by: Rasmus Villemoes Acked-by: Jason Baron Cc: David Sterba Cc: Greg Kroah-Hartman Cc: Ingo Molnar Cc: Petr Mladek Cc: "Rafael J . Wysocki" Cc: Steven Rostedt Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- lib/dynamic_debug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'lib/dynamic_debug.c') diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 7b76f43edaef..7bdf98c37e91 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -849,8 +849,10 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n, struct ddebug_table *dt; dt = kzalloc(sizeof(*dt), GFP_KERNEL); - if (dt == NULL) + if (dt == NULL) { + pr_err("error adding module: %s\n", name); return -ENOMEM; + } /* * For built-in modules, name lives in .rodata and is * immortal. For loaded modules, name points at the name[] -- cgit