Add initcall_blacklist kernel parameter [v4]

PATCH In review Prarit Bhargava 3 participants 6 messages Apr 1, 2014 – Apr 15, 2014 Documentation

Share & tools

Prarit Bhargava

author

2014-04-01 16:08 UTC #0 lore raw

PATCH 1/1 Add initcall_blacklist kernel parameter [v4]

When a module is built into the kernel the module_init() function becomes an initcall. Sometimes debugging through dynamic debug can help, however, debugging built in kernel modules is typically done by changing the .config, recompiling, and booting the new kernel in an effort to determine exactly which module caused a problem.

This patchset can be useful stand-alone or combined with initcall_debug. There are cases where some initcalls can hang the machine before the console can be flushed, which can make initcall_debug output inaccurate. Having the ability to skip initcalls can help further debugging of these scenarios.

Usage: initcall_blacklist=<list of comma separated initcalls>

ex) added "initcall_blacklist=sgi_uv_sysfs_init" as a kernel parameter and the log contains:

	blacklisted initcall sgi_uv_sysfs_init
	...
	...
	function sgi_uv_sysfs_init returning without executing

ex) added "initcall_blacklist=foo_bar,sgi_uv_sysfs_init" as a kernel parameter and the log contains:

initcall blacklisted foo_bar
initcall blacklisted sgi_uv_sysfs_init
...
...
function sgi_uv_sysfs_init returning without executing

Cc Richard Weinberger <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Josh Boyer <[email protected]>

Cc Rob Landley <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc [email protected]

Signed-off-by Prarit Bhargava <[email protected]>

[v2]: A few people recommended that I add code to allow for a list of initcalls as there maybe dependencies between initcalls such that one cannot be made without the other. I also updated the patch description. [v3]: Andi Kleen - don't worry about free'ing the small amount of memory. [v4]: Richard Weinberger -- add CONFIG_KALLSYMS dependency.

---
 Documentation/kernel-parameters.txt |    4 +++
 init/main.c                         |   50 +++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Documentation/kernel-parameters.txt +4 -0

index 67755ea..ebeb8a5 100644
@@ -1275,6 +1275,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			for working out where the kernel is dying during
 			startup.
 
+	initcall_blacklist=  [KNL] Do not execute a comma-separated list of
+			initcall functions.  Useful for debugging built-in
+			modules and initcalls.
+
 	initrd=		[BOOT] Specify the location of the initial ramdisk
 
 	inport.irq=	[HW] Inport (ATI XL and Microsoft) busmouse driver

init/main.c +50 -0

68 lines not on this page · raw message

Signature · 1 line

1.7.9.3

Andrew Morton

2014-04-14 21:29 UTC #1 lore raw

Quoted 22 lines · On Tue, 1 Apr 2014 11:19:05 -0400 Prarit Bhargava <[email protected]> wrote:

When a module is built into the kernel the module_init() function becomes an initcall. Sometimes debugging through dynamic debug can help, however, debugging built in kernel modules is typically done by changing the .config, recompiling, and booting the new kernel in an effort to determine exactly which module caused a problem.
This patchset can be useful stand-alone or combined with initcall_debug. There are cases where some initcalls can hang the machine before the console can be flushed, which can make initcall_debug output inaccurate. Having the ability to skip initcalls can help further debugging of these scenarios.
Usage: initcall_blacklist=<list of comma separated initcalls>
ex) added "initcall_blacklist=sgi_uv_sysfs_init" as a kernel parameter and the log contains:
	blacklisted initcall sgi_uv_sysfs_init
	...
	...
	function sgi_uv_sysfs_init returning without executing

Quoted 8 lines

ex) added "initcall_blacklist=foo_bar,sgi_uv_sysfs_init" as a kernel parameter and the log contains:
initcall blacklisted foo_bar
initcall blacklisted sgi_uv_sysfs_init
...
...
function sgi_uv_sysfs_init returning without executing

I guess the idea is reasonable and can be implemented very cheaply.

Quoted 27 lines

Documentation/kernel-parameters.txt +3 -0

@@ -1275,6 +1275,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			for working out where the kernel is dying during
 			startup.
 
+	initcall_blacklist=  [KNL] Do not execute a comma-separated list of
+			initcall functions.  Useful for debugging built-in
+			modules and initcalls.
 
 
 	initrd=		[BOOT] Specify the location of the initial ramdisk
 
 	inport.irq=	[HW] Inport (ATI XL and Microsoft) busmouse driver

init/main.c +1 -0

index 9c7fd4c..e050c24 100644
@@ -77,6 +77,7 @@
 #include <linux/sched_clock.h>
 #include <linux/context_tracking.h>
 #include <linux/random.h>
+#include <linux/list.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>

init/main.c static void __init do_ctors(void) · line 675

Quoted 9 lines

init/main.c +6 -0

@@ -666,6 +667,42 @@ static void __init do_ctors(void)
 bool initcall_debug;
 core_param(initcall_debug, initcall_debug, bool, 0644);
 
+struct blacklist_entry {
+	struct list_head next;
+	char *buf;
+};
+
+static __initdata_or_module LIST_HEAD(blacklisted_initcalls);

I suggest this be moved inside #ifdef CONFIG_KALLSYMS, then add

#ifdef CONFIG_KALLSYMS
static bool initcall_blacklisted(const char *id)

{
	...
}
#else
static bool initcall_blacklisted(const char *id)
{
	return false;
}

#endif

than call that from do_one_initcall().

init/main.c static void __init do_ctors(void) · line 689

Quoted 13 lines

init/main.c +13 -0

+#ifdef CONFIG_KALLSYMS
+static int __init initcall_blacklist(char *str)
+{
+	char *str_entry;
+	struct blacklist_entry *entry;
+
+	/* str argument is a comma-separated list of functions */
+	do {
+		str_entry = strsep(&str, ",");
+		if (str_entry) {
+			pr_debug("initcall blacklisted %s \n", str_entry);
+			entry = alloc_bootmem(sizeof(*entry));
+			entry->buf = alloc_bootmem(strlen(str_entry));

This needs to be strlen()+1.

init/main.c static void __init do_ctors(void) · line 690

Quoted 1 line

init/main.c +1 -0

+			strncpy(entry->buf, str_entry, strlen(str_entry));

and strcpy().

Or add a new strdup_bootmem() if it looks like there are (or will be) other callers.

init/main.c static void __init do_ctors(void) · line 700

Quoted 10 lines

init/main.c +10 -0

+			list_add(&entry->next, &blacklisted_initcalls);
+		}
+	} while (str_entry);
+
+	return 0;
+}
+#else
+static int __init initcall_blacklist(char *str)
+{
+	pr_warning("initcall_blacklist requires CONFIG_KALLSYMS to be enabled.\n");

"initcall_blacklist requires CONFIG_KALLSYMS" is sufficient.

Quoted 9 lines

init/main.c +5 -0

+	return 0;
+}
+#endif
+__setup("initcall_blacklist=", initcall_blacklist);
+
 static int __init_or_module do_one_initcall_debug(initcall_t fn)
 {
 	ktime_t calltime, delta, rettime;

init/main.c int __init_or_module do_one_initcall(initcall_t fn) · line 733

Quoted 8 lines

init/main.c +5 -0

@@ -689,6 +726,19 @@ int __init_or_module do_one_initcall(initcall_t fn)
 	int count = preempt_count();
 	int ret;
 	char msgbuf[64];
+	char fn_name[128] = "\0";
+	struct list_head *tmp;
+	struct blacklist_entry *entry;
+
+	snprintf(fn_name, 128, "%pf", fn);

Remove fn_name[], use kasprintf(), remember to free it.

init/main.c int __init_or_module do_one_initcall(initcall_t fn) · line 737

Quoted 4 lines

init/main.c +4 -0

+	list_for_each(tmp, &blacklisted_initcalls) {
+		entry = list_entry(tmp, struct blacklist_entry, next);
+		if (!strcmp(fn_name, entry->buf)) {
+			pr_debug("initcall %pf returning without executing\n",

"foo returning without executing" is contradictory: how can it return if it didn't execute? I suggest something like "skipping blacklisted initcall %pf".

init/main.c int __init_or_module do_one_initcall(initcall_t fn) · line 741

Quoted 4 lines

init/main.c +4 -0

+				 fn);
+			return -EPERM;
+		}
+	}

Let's not leak all those blacklist entries when we're finished?

Andi Kleen

2014-04-14 22:40 UTC #2 lore raw

Quoted 1 line

Let's not leak all those blacklist entries when we're finished?

It's difficult, because you cannot free bootmem after bootmem is finished.

For the rare debug case some leaking should be acceptable.

-Andi

Signature · 1 line

[email protected] -- Speaking for myself only.

Prarit Bhargava

author

2014-04-15 14:09 UTC #3 lore raw

Quoted 7 lines · Andi Kleen

Quoted 1 line
Let's not leak all those blacklist entries when we're finished?
It's difficult, because you cannot free bootmem after bootmem is finished.
For the rare debug case some leaking should be acceptable.

Andrew, FWIW I agree with Andi on this. It is a minor leak in a debug situation. I'd also like to know in a kdump situation what was specified as blacklisted so keeping this memory around is IMO a good idea.

Quoted 2 lines

-Andi

Prarit Bhargava

author

2014-04-15 15:17 UTC #4 lore raw

PATCH 1/1 Add initcall_blacklist kernel parameter [v5]

In reply to Andrew Morton

Usage: initcall_blacklist=<list of comma separated initcalls>

ex) added "initcall_blacklist=sgi_uv_sysfs_init" as a kernel parameter and the log contains:

	blacklisted initcall sgi_uv_sysfs_init
	...
	...
	function sgi_uv_sysfs_init returning without executing

ex) added "initcall_blacklist=foo_bar,sgi_uv_sysfs_init" as a kernel parameter and the log contains:

initcall blacklisted foo_bar
initcall blacklisted sgi_uv_sysfs_init
...
...
function sgi_uv_sysfs_init returning without executing

Cc Richard Weinberger <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Josh Boyer <[email protected]>

Cc Rob Landley <[email protected]>

Cc Andrew Morton <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc [email protected]

Signed-off-by Prarit Bhargava <[email protected]>

---
 Documentation/kernel-parameters.txt |    4 +++
 init/main.c                         |   68 +++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)

Documentation/kernel-parameters.txt +4 -0

index 03e50b4..7e1ee0c 100644
@@ -1294,6 +1294,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			for working out where the kernel is dying during
 			startup.
 
+	initcall_blacklist=  [KNL] Do not execute a comma-separated list of
+			initcall functions.  Useful for debugging built-in
+			modules and initcalls.
+
 	initrd=		[BOOT] Specify the location of the initial ramdisk
 
 	inport.irq=	[HW] Inport (ATI XL and Microsoft) busmouse driver

init/main.c +68 -0

86 lines not on this page · raw message

Signature · 1 line

1.7.9.3

Andrew Morton

2014-04-15 20:04 UTC #5 lore raw

Re: [PATCH] Add initcall_blacklist kernel parameter [v5]

Quoted 12 lines · On Tue, 15 Apr 2014 11:15:58 -0400 Prarit Bhargava <[email protected]> wrote:

When a module is built into the kernel the module_init() function becomes an initcall. Sometimes debugging through dynamic debug can help, however, debugging built in kernel modules is typically done by changing the .config, recompiling, and booting the new kernel in an effort to determine exactly which module caused a problem.
This patchset can be useful stand-alone or combined with initcall_debug. There are cases where some initcalls can hang the machine before the console can be flushed, which can make initcall_debug output inaccurate. Having the ability to skip initcalls can help further debugging of these scenarios.

Quoted 6 lines

Usage: initcall_blacklist=<list of comma separated initcalls>
ex) added "initcall_blacklist=sgi_uv_sysfs_init" as a kernel parameter and the log contains:
blacklisted initcall sgi_uv_sysfs_init

I think "blacklisting initcall sgi_uv_sysfs_init" makes more sense here.

Quoted 3 lines

	...
	...
	function sgi_uv_sysfs_init returning without executing

And this should be "blacklisted sgi_uv_sysfs_init initcall" or "initcall sgi_uv_sysfs_init blacklisted" (isn't English weird).

I did the below and fixed up the changelog to suit:

init/main.c renamed from init/main.c~init-mainc-add-initcall_blacklist-kernel-parameter-fix +1 -1

@@ -684,7 +684,7 @@ static int __init initcall_blacklist(cha
 	do {
 		str_entry = strsep(&str, ",");
 		if (str_entry) {
-			pr_debug("initcall blacklisted %s\n", str_entry);
+			pr_debug("blacklisting initcall %s\n", str_entry);
 			entry = alloc_bootmem(sizeof(*entry));
 			entry->buf = alloc_bootmem(strlen(str_entry) + 1);
 			strcpy(entry->buf, str_entry);