Messages in this thread |  | | | Date | Wed, 22 Sep 2010 16:50:13 +0200 | | From | Frederic Weisbecker <> | | Subject | Re: [PATCH 1/2] Add for_each_module iterator function v2 |
| |
On Wed, Sep 22, 2010 at 04:35:15PM +0200, Andi Kleen wrote: > On Wed, Sep 22, 2010 at 10:14:32AM -0400, Mathieu Desnoyers wrote: > > * Andi Kleen (andi@firstfloor.org) wrote: > > > From: Andi Kleen <ak@linux.intel.com> > > > > > > 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@linux.intel.com> > > > --- > > > 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
If possible, could you please also join a git url with your next version, it becomes hard to review as it's delta on top of others patches.
Thanks.
|  |