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

[Richard Weinberger]

On Tue, Apr 1, 2014 at 3:58 PM, Prarit Bhargava <prarit@xxxxxxxxxx> 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
>
> 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: Andi Kleen <andi@xxxxxxxxxxxxxx>
> Cc: Josh Boyer <jwboyer@xxxxxxxxxxxxxxxxx>
> Cc: Rob Landley <rob@xxxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: linux-doc@xxxxxxxxxxxxxxx
> Signed-off-by: Prarit Bhargava <prarit@xxxxxxxxxx>
>
> [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]: Drop memory cleanup, suggested by Andi Kleen.
> ---
>  Documentation/kernel-parameters.txt |    4 ++++
>  init/main.c                         |   42 +++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+)
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 67755ea..ebeb8a5 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -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
> diff --git a/init/main.c b/init/main.c
> index 9c7fd4c..64a3b8b 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -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>
> @@ -666,6 +667,34 @@ 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);
> +
> +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));
> +                       strncpy(entry->buf, str_entry, strlen(str_entry));
> +                       list_add(&entry->next, &blacklisted_initcalls);
> +               }
> +       } while (str_entry);
> +
> +       return 0;
> +}
> +__setup("initcall_blacklist=", initcall_blacklist);

Wouldn't it make sense to make this feature depend on CONFIG_KALLSYMS?
I.e. Making it a nop plus a warning if one uses initcall_blacklist and
has CONFIG_KALLSYMS=n.

>  static int __init_or_module do_one_initcall_debug(initcall_t fn)
>  {
>         ktime_t calltime, delta, rettime;
> @@ -689,6 +718,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);
> +       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",
> +                                fn);
> +                       return -EPERM;
> +               }
> +       }
>
>         if (initcall_debug)
>                 ret = do_one_initcall_debug(fn);
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Thanks,
//richard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/