2003-03-18 15:49:00

by John Levon

Subject: [PATCH] module load notification try 2

Rusty, any happier with this one ?

The below implements a module load notification for oprofile's benefit.
The oprofile part is not attached here, but seems to work OK.

regards
john


diff -Naur -X dontdiff linux-linus/include/linux/module.h up/include/linux/module.h
--- linux-linus/include/linux/module.h 2003-03-17 21:44:38.000000000 +0000
+++ up/include/linux/module.h 2003-03-18 14:34:07.000000000 +0000
@@ -142,6 +142,7 @@
const struct exception_table_entry *entry;
};

+struct notifier_block;

#ifdef CONFIG_MODULES

@@ -352,6 +353,9 @@
/* For extable.c to search modules' exception tables. */
const struct exception_table_entry *search_module_extables(unsigned long addr);

+int register_module_notifier(struct notifier_block * nb);
+int unregister_module_notifier(struct notifier_block * nb);
+
#else /* !CONFIG_MODULES... */
#define EXPORT_SYMBOL(sym)
#define EXPORT_SYMBOL_GPL(sym)
@@ -396,6 +400,17 @@
{
return NULL;
}
+
+static inline int register_module_notifier(struct notifier_block * nb)
+{
+ return -ENOSYS;
+}
+
+static inline int unregister_module_notifier(struct notifier_block * nb)
+{
+ return -ENOSYS;
+}
+
#endif /* CONFIG_MODULES */

#ifdef MODULE
diff -Naur -X dontdiff linux-linus/kernel/module.c up/kernel/module.c
--- linux-linus/kernel/module.c 2003-03-17 21:44:21.000000000 +0000
+++ up/kernel/module.c 2003-03-18 15:42:56.000000000 +0000
@@ -31,6 +31,7 @@
#include <linux/errno.h>
#include <linux/err.h>
#include <linux/vermagic.h>
+#include <linux/notifier.h>
#include <asm/uaccess.h>
#include <asm/semaphore.h>
#include <asm/pgalloc.h>
@@ -61,6 +62,29 @@
static LIST_HEAD(symbols);
static LIST_HEAD(extables);

+static DECLARE_MUTEX(notify_mutex);
+static struct notifier_block * module_notify_list;
+
+int register_module_notifier(struct notifier_block * nb)
+{
+ int err;
+ down(&notify_mutex);
+ err = notifier_chain_register(&module_notify_list, nb);
+ up(&notify_mutex);
+ return err;
+}
+EXPORT_SYMBOL(register_module_notifier);
+
+int unregister_module_notifier(struct notifier_block * nb)
+{
+ int err;
+ down(&notify_mutex);
+ err = notifier_chain_unregister(&module_notify_list, nb);
+ up(&notify_mutex);
+ return err;
+}
+EXPORT_SYMBOL(unregister_module_notifier);
+
/* We require a truly strong try_module_get() */
static inline int strong_try_module_get(struct module *mod)
{
@@ -1368,6 +1392,10 @@
/* Drop lock so they can recurse */
up(&module_mutex);

+ down(&notify_mutex);
+ notifier_call_chain(&module_notify_list, MODULE_STATE_COMING, mod);
+ up(&notify_mutex);
+
/* Start the module */
ret = mod->init();
if (ret < 0) {

2003-03-25 00:56:29

by Rusty Russell

Subject: Re: [PATCH] module load notification try 2

In message <[email protected]> you write:
>
> Rusty, any happier with this one ?

Yep, much simpler. Couple of comments:

> +static inline int register_module_notifier(struct notifier_block * nb)
> +{
> + return -ENOSYS;
> +}
> +
> +static inline int unregister_module_notifier(struct notifier_block * nb)
> +{
> + return -ENOSYS;
> +}
> +
> #endif /* CONFIG_MODULES */

Shouldn't fail just because !CONFIG_MODULES: should just return 0.
Otherwise there's no way to sanely use them without wrapping in #ifdef
CONFIG_MODULES (in which case why have the non-module definition at
all?).

> +static DECLARE_MUTEX(notify_mutex);

Hmm, yes, you need to use your own protection around
notifier_chain_register and notifier_call_chain. Wierd, because
notifier.c does its own locking for register and unregister, but not
for calling, which AFAICT makes it useless...

Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2003-03-25 01:04:19

by John Levon

Subject: Re: [PATCH] module load notification try 2

On Sun, Mar 23, 2003 at 03:36:26PM +1100, Rusty Russell wrote:

> > +static inline int register_module_notifier(struct notifier_block * nb)
> > +{
> > + return -ENOSYS;
> > +}
>
> Shouldn't fail just because !CONFIG_MODULES: should just return 0.

Ho-de-hum, flipped a coin, lost the toss.

> Otherwise there's no way to sanely use them without wrapping in #ifdef

Well, "if (err && err != -ENOSYS)". The relative sanity of that is
debatable. I don't care either way, hence the coin toss.

I Cc:ed Zwane who was advocating -ENOSYS on IRC ...

> > +static DECLARE_MUTEX(notify_mutex);
>
> Hmm, yes, you need to use your own protection around
> notifier_chain_register and notifier_call_chain. Wierd, because
> notifier.c does its own locking for register and unregister, but not
> for calling, which AFAICT makes it useless...

I mentioned about this some time ago on lkml to massive indifference.
Later on someone from OSDL reworked all the notifier stuff, dunno what
happened to the patch.

regards,
john

2003-03-25 06:22:11

by Rusty Russell

Subject: Re: [PATCH] module load notification try 2

In message <[email protected]> you write:
> > Otherwise there's no way to sanely use them without wrapping in #ifdef
>
> Well, "if (err && err != -ENOSYS)". The relative sanity of that is
> debatable. I don't care either way, hence the coin toss.

OK, well, I have an opinion and it's the other one. 8)

> > Hmm, yes, you need to use your own protection around
> > notifier_chain_register and notifier_call_chain. Wierd, because
> > notifier.c does its own locking for register and unregister, but not
> > for calling, which AFAICT makes it useless...
>
> I mentioned about this some time ago on lkml to massive indifference.

Yeah, it's a PITA.

Cheers,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.