lkml.org 
[lkml]   [2008]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH] call sysrq_timer_list_show from a workqueue
    Date
    On Tuesday 08 January 2008 20:17:42 Ingo Molnar wrote:
    > (Rusty Cc:-ed)
    >
    > * Kyle McMartin <kyle@mcmartin.ca> wrote:
    > > handle_sysrq can be called from interrupt context.
    > > sysrq_timer_list_show eventually starts poking at module symbols which
    > > take the module mutex.
    > >
    > > so instead, let's just kick off a workqueue.
    > >
    > > [ doesn't happen on my laptop with the keyboard, but does when
    > > triggered from /proc/sysrq-trigger ]
    >
    > hmmm. This really shouldnt be happening - how come we hit the module
    > mutex on simple things like symbol lookups?

    We did start demutexing paths to try to get more info out for oopsen, but
    lookup_module_symbol_name was not among them.

    Hmm, the module symbol lookup interfaces are out of control, but this
    should do it:

    De-mutex more symbol lookup paths in the module code

    Kyle McMartin reports sysrq_timer_list_show() can hit the module
    mutex; these paths don't need to though, since we long ago changed all
    the module list manipulation to occur via stop_machine().

    Disabling preemption is enough.

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

    diff -r f52abc2cbb4e kernel/module.c
    --- a/kernel/module.c Tue Jan 08 17:39:23 2008 +1100
    +++ b/kernel/module.c Tue Jan 08 22:27:01 2008 +1100
    @@ -2238,29 +2238,34 @@ static const char *get_ksymbol(struct mo
    /* For kallsyms to ask for address resolution. NULL means not found.
    We don't lock, as this is used for oops resolution and races are a
    lesser concern. */
    +/* FIXME: Risky: returns a pointer into a module w/o lock */
    const char *module_address_lookup(unsigned long addr,
    unsigned long *size,
    unsigned long *offset,
    char **modname)
    {
    struct module *mod;
    + const char *ret = NULL;

    + preempt_disable();
    list_for_each_entry(mod, &modules, list) {
    if (within(addr, mod->module_init, mod->init_size)
    || within(addr, mod->module_core, mod->core_size)) {
    if (modname)
    *modname = mod->name;
    - return get_ksymbol(mod, addr, size, offset);
    + ret = get_ksymbol(mod, addr, size, offset);
    + break;
    }
    }
    - return NULL;
    + preempt_enable();
    + return ret;
    }

    int lookup_module_symbol_name(unsigned long addr, char *symname)
    {
    struct module *mod;

    - mutex_lock(&module_mutex);
    + preempt_disable();
    list_for_each_entry(mod, &modules, list) {
    if (within(addr, mod->module_init, mod->init_size) ||
    within(addr, mod->module_core, mod->core_size)) {
    @@ -2270,12 +2275,12 @@ int lookup_module_symbol_name(unsigned l
    if (!sym)
    goto out;
    strlcpy(symname, sym, KSYM_NAME_LEN);
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return 0;
    }
    }
    out:
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return -ERANGE;
    }

    @@ -2284,7 +2289,7 @@ int lookup_module_symbol_attrs(unsigned
    {
    struct module *mod;

    - mutex_lock(&module_mutex);
    + preempt_disable();
    list_for_each_entry(mod, &modules, list) {
    if (within(addr, mod->module_init, mod->init_size) ||
    within(addr, mod->module_core, mod->core_size)) {
    @@ -2297,12 +2302,12 @@ int lookup_module_symbol_attrs(unsigned
    strlcpy(modname, mod->name, MODULE_NAME_LEN);
    if (name)
    strlcpy(name, sym, KSYM_NAME_LEN);
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return 0;
    }
    }
    out:
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return -ERANGE;
    }

    @@ -2311,7 +2316,7 @@ int module_get_kallsym(unsigned int symn
    {
    struct module *mod;

    - mutex_lock(&module_mutex);
    + preempt_disable();
    list_for_each_entry(mod, &modules, list) {
    if (symnum < mod->num_symtab) {
    *value = mod->symtab[symnum].st_value;
    @@ -2320,12 +2325,12 @@ int module_get_kallsym(unsigned int symn
    KSYM_NAME_LEN);
    strlcpy(module_name, mod->name, MODULE_NAME_LEN);
    *exported = is_exported(name, mod);
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return 0;
    }
    symnum -= mod->num_symtab;
    }
    - mutex_unlock(&module_mutex);
    + preempt_enable();
    return -ERANGE;
    }

    @@ -2348,6 +2353,7 @@ unsigned long module_kallsyms_lookup_nam
    unsigned long ret = 0;

    /* Don't lock: we're in enough trouble already. */
    + preempt_disable();
    if ((colon = strchr(name, ':')) != NULL) {
    *colon = '\0';
    if ((mod = find_module(name)) != NULL)
    @@ -2358,6 +2364,7 @@ unsigned long module_kallsyms_lookup_nam
    if ((ret = mod_find_symname(mod, name)) != 0)
    break;
    }
    + preempt_enable();
    return ret;
    }
    #endif /* CONFIG_KALLSYMS */

    \
     
     \ /
      Last update: 2008-01-08 12:31    [W:0.026 / U:76.700 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site