[PATCH] module load notification try 2

[PATCH] module load notification try 2

[John Levon]

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) {

Re: [PATCH] module load notification try 2

[Rusty Russell]

In message <20030318155953.GA97463@compsoc.man.ac.uk> 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.

Re: [PATCH] module load notification try 2

[John Levon]

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

Re: [PATCH] module load notification try 2

[Rusty Russell]

In message <20030325011525.GA92370@compsoc.man.ac.uk> 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.