Re: [PATCH 1/2] Add for_each_module iterator function v2

[Andi Kleen]

On Wed, Sep 22, 2010 at 10:14:32AM -0400, Mathieu Desnoyers wrote:
> * Andi Kleen (andi@xxxxxxxxxxxxxx) wrote:
> > From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> > 
> > This is a generic function to iterate over all modules.
> > To be used in the next patch.
> > 
> > v2: Use module_mutex instead of preempt disabling
> > 
> > Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> > ---
> >  include/linux/module.h |    1 +
> >  kernel/module.c        |   10 ++++++++++
> >  2 files changed, 11 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 403ac26..809b6db 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -410,6 +410,7 @@ struct module *__module_address(unsigned long addr);
> >  bool is_module_address(unsigned long addr);
> >  bool is_module_percpu_address(unsigned long addr);
> >  bool is_module_text_address(unsigned long addr);
> > +void for_each_module(void (*op)(struct module *, void *arg), void *arg);
> >  
> >  static inline int within_module_core(unsigned long addr, struct module *mod)
> >  {
> > diff --git a/kernel/module.c b/kernel/module.c
> > index eba1341..9e2a561 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -384,6 +384,16 @@ struct module *find_module(const char *name)
> >  }
> >  EXPORT_SYMBOL_GPL(find_module);
> >  
> > +void for_each_module(void (*op)(struct module *, void *arg), void *arg)
> > +{
> > +	struct module *mod;
> > +
> > +	mutex_lock(&module_mutex);
> 
> I'm concerned about the following mutex dependency chain:
> 
> module_mutex -> jump_label_mutex (on module load)
> jump_label_mutex -> module_mutex (on jump_label_update)

There are actually more problems in this version with using
module_mutex().  Looking at a fix.

> 
> Actually, I think your module notifier is missing in your version. This
> is why you don't see this dependency problem.

The module notifier is not needed anymore, but there's another 
callback through the trace point code on module load.

Unload notifier is not needed anymore because there is no
state to tear down.

-Andi
--
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/