[RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Peter Chen]

Currently, if module's refcount is not zero during the unload,
it waits there until the user decreases that refcount. Assume
we have two modules (A & B), there are no symbol relationship
between each other. module B is module A's user, if the end
user tries to unload module A first wrongly, it will stop there
until there is another user process to unload B, and this
process can't be killed.
One use case is: the QA engineers do error test, they unload
module wrongly on purpose, after that, they find the console
is stopped there, and they can't do any thing go on.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 include/linux/module.h |    4 +-
 kernel/module.c        |   61 ++++++++++++++++++++++++++---------------------
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 05f2447..12edf07 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -367,8 +367,8 @@ struct module
 	/* What modules do I depend on? */
 	struct list_head target_list;
 
-	/* Who is waiting for us to be unloaded */
-	struct task_struct *waiter;
+	/* The work for waiting refcount to zero */
+	struct work_struct wait_refcount_work;
 
 	/* Destruction function. */
 	void (*exit)(void);
diff --git a/kernel/module.c b/kernel/module.c
index dc58274..94abc7e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -61,6 +61,7 @@
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
 #include <linux/fips.h>
+#include <linux/delay.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -644,8 +645,6 @@ static int module_unload_init(struct module *mod)
 
 	/* Hold reference count during initialization. */
 	__this_cpu_write(mod->refptr->incs, 1);
-	/* Backwards compatibility macros put refcount during init. */
-	mod->waiter = current;
 
 	return 0;
 }
@@ -813,19 +812,38 @@ EXPORT_SYMBOL(module_refcount);
 /* This exists whether we can unload or not */
 static void free_module(struct module *mod);
 
+/* Final destruction now no one is using it. */
+static void module_final_free(struct module *mod)
+{
+	if (mod->exit != NULL)
+		mod->exit();
+	blocking_notifier_call_chain(&module_notify_list,
+				     MODULE_STATE_GOING, mod);
+	async_synchronize_full();
+
+	/* Store the name of the last unloaded module for diagnostic purposes */
+	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
+
+	free_module(mod);
+}
+
 static void wait_for_zero_refcount(struct module *mod)
 {
-	/* Since we might sleep for some time, release the mutex first */
-	mutex_unlock(&module_mutex);
 	for (;;) {
 		pr_debug("Looking at refcount...\n");
-		set_current_state(TASK_UNINTERRUPTIBLE);
 		if (module_refcount(mod) == 0)
 			break;
-		schedule();
+		msleep(1000);
 	}
-	current->state = TASK_RUNNING;
-	mutex_lock(&module_mutex);
+	module_final_free(mod);
+}
+
+static void wait_module_refcount(struct work_struct *work)
+{
+	struct module *mod = container_of(work,
+		struct module, wait_refcount_work);
+
+	wait_for_zero_refcount(mod);
 }
 
 SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
@@ -859,8 +877,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 
 	/* Doing init or already dying? */
 	if (mod->state != MODULE_STATE_LIVE) {
-		/* FIXME: if (force), slam module count and wake up
-                   waiter --RR */
+		/* FIXME: if (force), slam module count */
 		pr_debug("%s already dying\n", mod->name);
 		ret = -EBUSY;
 		goto out;
@@ -876,30 +893,23 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		}
 	}
 
-	/* Set this up before setting mod->state */
-	mod->waiter = current;
-
 	/* Stop the machine so refcounts can't move and disable module. */
 	ret = try_stop_module(mod, flags, &forced);
 	if (ret != 0)
 		goto out;
 
 	/* Never wait if forced. */
-	if (!forced && module_refcount(mod) != 0)
-		wait_for_zero_refcount(mod);
+	if (!forced && module_refcount(mod) != 0) {
+		INIT_WORK(&mod->wait_refcount_work, wait_module_refcount);
+		schedule_work(&mod->wait_refcount_work);
+		ret = -EBUSY;
+		goto out;
+	}
 
 	mutex_unlock(&module_mutex);
-	/* Final destruction now no one is using it. */
-	if (mod->exit != NULL)
-		mod->exit();
-	blocking_notifier_call_chain(&module_notify_list,
-				     MODULE_STATE_GOING, mod);
-	async_synchronize_full();
 
-	/* Store the name of the last unloaded module for diagnostic purposes */
-	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
+	module_final_free(mod);
 
-	free_module(mod);
 	return 0;
 out:
 	mutex_unlock(&module_mutex);
@@ -1005,9 +1015,6 @@ void module_put(struct module *module)
 		__this_cpu_inc(module->refptr->decs);
 
 		trace_module_put(module, _RET_IP_);
-		/* Maybe they're waiting for us to drop reference? */
-		if (unlikely(!module_is_live(module)))
-			wake_up_process(module->waiter);
 		preempt_enable();
 	}
 }
-- 
1.7.1

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Rusty Russell]

Peter Chen <peter.chen@freescale.com> writes:
> Currently, if module's refcount is not zero during the unload,
> it waits there until the user decreases that refcount.

Hi Peter,

        In practice userspace uses O_NONBLOCK.  In fact, I've been
thinking of removing the blocking case altogether, since it's not really
what people want.

That would solve your problem and make the code simpler.  Thoughts?

Cheers,
Rusty.

> Assume
> we have two modules (A & B), there are no symbol relationship
> between each other. module B is module A's user, if the end
> user tries to unload module A first wrongly, it will stop there
> until there is another user process to unload B, and this
> process can't be killed.
> One use case is: the QA engineers do error test, they unload
> module wrongly on purpose, after that, they find the console
> is stopped there, and they can't do any thing go on.
>
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  include/linux/module.h |    4 +-
>  kernel/module.c        |   61 ++++++++++++++++++++++++++---------------------
>  2 files changed, 36 insertions(+), 29 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 05f2447..12edf07 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -367,8 +367,8 @@ struct module
>  	/* What modules do I depend on? */
>  	struct list_head target_list;
>  
> -	/* Who is waiting for us to be unloaded */
> -	struct task_struct *waiter;
> +	/* The work for waiting refcount to zero */
> +	struct work_struct wait_refcount_work;
>  
>  	/* Destruction function. */
>  	void (*exit)(void);
> diff --git a/kernel/module.c b/kernel/module.c
> index dc58274..94abc7e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -61,6 +61,7 @@
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
>  #include <linux/fips.h>
> +#include <linux/delay.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -644,8 +645,6 @@ static int module_unload_init(struct module *mod)
>  
>  	/* Hold reference count during initialization. */
>  	__this_cpu_write(mod->refptr->incs, 1);
> -	/* Backwards compatibility macros put refcount during init. */
> -	mod->waiter = current;
>  
>  	return 0;
>  }
> @@ -813,19 +812,38 @@ EXPORT_SYMBOL(module_refcount);
>  /* This exists whether we can unload or not */
>  static void free_module(struct module *mod);
>  
> +/* Final destruction now no one is using it. */
> +static void module_final_free(struct module *mod)
> +{
> +	if (mod->exit != NULL)
> +		mod->exit();
> +	blocking_notifier_call_chain(&module_notify_list,
> +				     MODULE_STATE_GOING, mod);
> +	async_synchronize_full();
> +
> +	/* Store the name of the last unloaded module for diagnostic purposes */
> +	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
> +
> +	free_module(mod);
> +}
> +
>  static void wait_for_zero_refcount(struct module *mod)
>  {
> -	/* Since we might sleep for some time, release the mutex first */
> -	mutex_unlock(&module_mutex);
>  	for (;;) {
>  		pr_debug("Looking at refcount...\n");
> -		set_current_state(TASK_UNINTERRUPTIBLE);
>  		if (module_refcount(mod) == 0)
>  			break;
> -		schedule();
> +		msleep(1000);
>  	}
> -	current->state = TASK_RUNNING;
> -	mutex_lock(&module_mutex);
> +	module_final_free(mod);
> +}
> +
> +static void wait_module_refcount(struct work_struct *work)
> +{
> +	struct module *mod = container_of(work,
> +		struct module, wait_refcount_work);
> +
> +	wait_for_zero_refcount(mod);
>  }
>  
>  SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> @@ -859,8 +877,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>  
>  	/* Doing init or already dying? */
>  	if (mod->state != MODULE_STATE_LIVE) {
> -		/* FIXME: if (force), slam module count and wake up
> -                   waiter --RR */
> +		/* FIXME: if (force), slam module count */
>  		pr_debug("%s already dying\n", mod->name);
>  		ret = -EBUSY;
>  		goto out;
> @@ -876,30 +893,23 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>  		}
>  	}
>  
> -	/* Set this up before setting mod->state */
> -	mod->waiter = current;
> -
>  	/* Stop the machine so refcounts can't move and disable module. */
>  	ret = try_stop_module(mod, flags, &forced);
>  	if (ret != 0)
>  		goto out;
>  
>  	/* Never wait if forced. */
> -	if (!forced && module_refcount(mod) != 0)
> -		wait_for_zero_refcount(mod);
> +	if (!forced && module_refcount(mod) != 0) {
> +		INIT_WORK(&mod->wait_refcount_work, wait_module_refcount);
> +		schedule_work(&mod->wait_refcount_work);
> +		ret = -EBUSY;
> +		goto out;
> +	}
>  
>  	mutex_unlock(&module_mutex);
> -	/* Final destruction now no one is using it. */
> -	if (mod->exit != NULL)
> -		mod->exit();
> -	blocking_notifier_call_chain(&module_notify_list,
> -				     MODULE_STATE_GOING, mod);
> -	async_synchronize_full();
>  
> -	/* Store the name of the last unloaded module for diagnostic purposes */
> -	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
> +	module_final_free(mod);
>  
> -	free_module(mod);
>  	return 0;
>  out:
>  	mutex_unlock(&module_mutex);
> @@ -1005,9 +1015,6 @@ void module_put(struct module *module)
>  		__this_cpu_inc(module->refptr->decs);
>  
>  		trace_module_put(module, _RET_IP_);
> -		/* Maybe they're waiting for us to drop reference? */
> -		if (unlikely(!module_is_live(module)))
> -			wake_up_process(module->waiter);
>  		preempt_enable();
>  	}
>  }
> -- 
> 1.7.1

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Peter Chen]

On Fri, Sep 13, 2013 at 10:00:33AM +0930, Rusty Russell wrote:
> Peter Chen <peter.chen@freescale.com> writes:
> > Currently, if module's refcount is not zero during the unload,
> > it waits there until the user decreases that refcount.
> 
> Hi Peter,
> 
>         In practice userspace uses O_NONBLOCK.  In fact, I've been
> thinking of removing the blocking case altogether, since it's not really
> what people want.
> 
> That would solve your problem and make the code simpler.  Thoughts?
> 

So, it will like "Force unload" case, right?
If it is the case, it is better have a warning message to indicate
some users are still using it, since there may null pointer
dereference when the user module has unloaded, and the end user
can understand it may be triggered by wrong module unload sequence.

Thanks.

-- 

Best Regards,
Peter Chen

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Lucas De Marchi]

On Thu, Sep 12, 2013 at 9:30 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Peter Chen <peter.chen@freescale.com> writes:
>> Currently, if module's refcount is not zero during the unload,
>> it waits there until the user decreases that refcount.
>
> Hi Peter,
>
>         In practice userspace uses O_NONBLOCK.  In fact, I've been
> thinking of removing the blocking case altogether, since it's not really
> what people want.
>
> That would solve your problem and make the code simpler.  Thoughts?

I'm all in favor of this. It's been almost 1 year it's deprecated in
kmod and if anyone tries to use we force a 10s delay on module
removal. So far nobody complained.

Lucas De Marchi

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Rusty Russell]

Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
> On Thu, Sep 12, 2013 at 9:30 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Peter Chen <peter.chen@freescale.com> writes:
>>> Currently, if module's refcount is not zero during the unload,
>>> it waits there until the user decreases that refcount.
>>
>> Hi Peter,
>>
>>         In practice userspace uses O_NONBLOCK.  In fact, I've been
>> thinking of removing the blocking case altogether, since it's not really
>> what people want.
>>
>> That would solve your problem and make the code simpler.  Thoughts?
>
> I'm all in favor of this. It's been almost 1 year it's deprecated in
> kmod and if anyone tries to use we force a 10s delay on module
> removal. So far nobody complained.
>
> Lucas De Marchi

Here's what I've got in my pending-rebases tree.

Cheers,
Rusty.

module: remove rmmod --wait option.

The option to wait for a module reference count to reach zero was in
the initial module implementation, but it was never supported in
modprobe (you had to use rmmod --wait).  After discussion with Lucas,
It has been deprecated (with a 10 second sleep) in kmod for the last
year.

This finally removes it: the flag will evoke a printk warning and a
normal (non-blocking) remove attempt.

Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/linux/module.h b/include/linux/module.h
index 05f2447..15cd6b1 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -367,9 +367,6 @@ struct module
 	/* What modules do I depend on? */
 	struct list_head target_list;
 
-	/* Who is waiting for us to be unloaded */
-	struct task_struct *waiter;
-
 	/* Destruction function. */
 	void (*exit)(void);
 
diff --git a/kernel/module.c b/kernel/module.c
index dc58274..947105f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -644,8 +644,6 @@ static int module_unload_init(struct module *mod)
 
 	/* Hold reference count during initialization. */
 	__this_cpu_write(mod->refptr->incs, 1);
-	/* Backwards compatibility macros put refcount during init. */
-	mod->waiter = current;
 
 	return 0;
 }
@@ -771,16 +769,9 @@ static int __try_stop_module(void *_sref)
 
 static int try_stop_module(struct module *mod, int flags, int *forced)
 {
-	if (flags & O_NONBLOCK) {
-		struct stopref sref = { mod, flags, forced };
+	struct stopref sref = { mod, flags, forced };
 
-		return stop_machine(__try_stop_module, &sref, NULL);
-	} else {
-		/* We don't need to stop the machine for this. */
-		mod->state = MODULE_STATE_GOING;
-		synchronize_sched();
-		return 0;
-	}
+	return stop_machine(__try_stop_module, &sref, NULL);
 }
 
 unsigned long module_refcount(struct module *mod)
@@ -813,21 +804,6 @@ EXPORT_SYMBOL(module_refcount);
 /* This exists whether we can unload or not */
 static void free_module(struct module *mod);
 
-static void wait_for_zero_refcount(struct module *mod)
-{
-	/* Since we might sleep for some time, release the mutex first */
-	mutex_unlock(&module_mutex);
-	for (;;) {
-		pr_debug("Looking at refcount...\n");
-		set_current_state(TASK_UNINTERRUPTIBLE);
-		if (module_refcount(mod) == 0)
-			break;
-		schedule();
-	}
-	current->state = TASK_RUNNING;
-	mutex_lock(&module_mutex);
-}
-
 SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		unsigned int, flags)
 {
@@ -842,6 +818,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		return -EFAULT;
 	name[MODULE_NAME_LEN-1] = '\0';
 
+	if (!(flags & O_NONBLOCK)) {
+		printk(KERN_WARNING
+		       "waiting module removal not supported: please upgrade");
+	}
+
 	if (mutex_lock_interruptible(&module_mutex) != 0)
 		return -EINTR;
 
@@ -859,8 +840,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 
 	/* Doing init or already dying? */
 	if (mod->state != MODULE_STATE_LIVE) {
-		/* FIXME: if (force), slam module count and wake up
-                   waiter --RR */
+		/* FIXME: if (force), slam module count damn the torpedoes */
 		pr_debug("%s already dying\n", mod->name);
 		ret = -EBUSY;
 		goto out;
@@ -876,18 +856,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 		}
 	}
 
-	/* Set this up before setting mod->state */
-	mod->waiter = current;
-
 	/* Stop the machine so refcounts can't move and disable module. */
 	ret = try_stop_module(mod, flags, &forced);
 	if (ret != 0)
 		goto out;
 
-	/* Never wait if forced. */
-	if (!forced && module_refcount(mod) != 0)
-		wait_for_zero_refcount(mod);
-
 	mutex_unlock(&module_mutex);
 	/* Final destruction now no one is using it. */
 	if (mod->exit != NULL)
@@ -1005,9 +978,6 @@ void module_put(struct module *module)
 		__this_cpu_inc(module->refptr->decs);
 
 		trace_module_put(module, _RET_IP_);
-		/* Maybe they're waiting for us to drop reference? */
-		if (unlikely(!module_is_live(module)))
-			wake_up_process(module->waiter);
 		preempt_enable();
 	}
 }

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Lucas De Marchi]

On Mon, Sep 16, 2013 at 12:47 AM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Lucas De Marchi <lucas.de.marchi@gmail.com> writes:
>> On Thu, Sep 12, 2013 at 9:30 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>> Peter Chen <peter.chen@freescale.com> writes:
>>>> Currently, if module's refcount is not zero during the unload,
>>>> it waits there until the user decreases that refcount.
>>>
>>> Hi Peter,
>>>
>>>         In practice userspace uses O_NONBLOCK.  In fact, I've been
>>> thinking of removing the blocking case altogether, since it's not really
>>> what people want.
>>>
>>> That would solve your problem and make the code simpler.  Thoughts?
>>
>> I'm all in favor of this. It's been almost 1 year it's deprecated in
>> kmod and if anyone tries to use we force a 10s delay on module
>> removal. So far nobody complained.
>>
>> Lucas De Marchi
>
> Here's what I've got in my pending-rebases tree.
>
> Cheers,
> Rusty.
>
> module: remove rmmod --wait option.
>
> The option to wait for a module reference count to reach zero was in
> the initial module implementation, but it was never supported in
> modprobe (you had to use rmmod --wait).  After discussion with Lucas,
> It has been deprecated (with a 10 second sleep) in kmod for the last
> year.
>
> This finally removes it: the flag will evoke a printk warning and a
> normal (non-blocking) remove attempt.
>
> Cc: Lucas De Marchi <lucas.de.marchi@gmail.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 05f2447..15cd6b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -367,9 +367,6 @@ struct module
>         /* What modules do I depend on? */
>         struct list_head target_list;
>
> -       /* Who is waiting for us to be unloaded */
> -       struct task_struct *waiter;
> -
>         /* Destruction function. */
>         void (*exit)(void);
>
> diff --git a/kernel/module.c b/kernel/module.c
> index dc58274..947105f 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -644,8 +644,6 @@ static int module_unload_init(struct module *mod)
>
>         /* Hold reference count during initialization. */
>         __this_cpu_write(mod->refptr->incs, 1);
> -       /* Backwards compatibility macros put refcount during init. */
> -       mod->waiter = current;
>
>         return 0;
>  }
> @@ -771,16 +769,9 @@ static int __try_stop_module(void *_sref)
>
>  static int try_stop_module(struct module *mod, int flags, int *forced)
>  {
> -       if (flags & O_NONBLOCK) {
> -               struct stopref sref = { mod, flags, forced };
> +       struct stopref sref = { mod, flags, forced };
>
> -               return stop_machine(__try_stop_module, &sref, NULL);
> -       } else {
> -               /* We don't need to stop the machine for this. */
> -               mod->state = MODULE_STATE_GOING;
> -               synchronize_sched();
> -               return 0;
> -       }
> +       return stop_machine(__try_stop_module, &sref, NULL);
>  }
>
>  unsigned long module_refcount(struct module *mod)
> @@ -813,21 +804,6 @@ EXPORT_SYMBOL(module_refcount);
>  /* This exists whether we can unload or not */
>  static void free_module(struct module *mod);
>
> -static void wait_for_zero_refcount(struct module *mod)
> -{
> -       /* Since we might sleep for some time, release the mutex first */
> -       mutex_unlock(&module_mutex);
> -       for (;;) {
> -               pr_debug("Looking at refcount...\n");
> -               set_current_state(TASK_UNINTERRUPTIBLE);
> -               if (module_refcount(mod) == 0)
> -                       break;
> -               schedule();
> -       }
> -       current->state = TASK_RUNNING;
> -       mutex_lock(&module_mutex);
> -}
> -
>  SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>                 unsigned int, flags)
>  {
> @@ -842,6 +818,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>                 return -EFAULT;
>         name[MODULE_NAME_LEN-1] = '\0';
>
> +       if (!(flags & O_NONBLOCK)) {
> +               printk(KERN_WARNING
> +                      "waiting module removal not supported: please upgrade");
> +       }
> +
>         if (mutex_lock_interruptible(&module_mutex) != 0)
>                 return -EINTR;
>
> @@ -859,8 +840,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>
>         /* Doing init or already dying? */
>         if (mod->state != MODULE_STATE_LIVE) {
> -               /* FIXME: if (force), slam module count and wake up
> -                   waiter --RR */
> +               /* FIXME: if (force), slam module count damn the torpedoes */
>                 pr_debug("%s already dying\n", mod->name);
>                 ret = -EBUSY;
>                 goto out;
> @@ -876,18 +856,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>                 }
>         }
>
> -       /* Set this up before setting mod->state */
> -       mod->waiter = current;
> -
>         /* Stop the machine so refcounts can't move and disable module. */
>         ret = try_stop_module(mod, flags, &forced);
>         if (ret != 0)
>                 goto out;
>
> -       /* Never wait if forced. */
> -       if (!forced && module_refcount(mod) != 0)
> -               wait_for_zero_refcount(mod);
> -
>         mutex_unlock(&module_mutex);
>         /* Final destruction now no one is using it. */
>         if (mod->exit != NULL)
> @@ -1005,9 +978,6 @@ void module_put(struct module *module)
>                 __this_cpu_inc(module->refptr->decs);
>
>                 trace_module_put(module, _RET_IP_);
> -               /* Maybe they're waiting for us to drop reference? */
> -               if (unlikely(!module_is_live(module)))
> -                       wake_up_process(module->waiter);
>                 preempt_enable();
>         }
>  }

Acked-by: Lucas De Marchi <lucas.demarchi@intel.com>


Lucas De Marchi

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Jan Engelhardt]

On Monday 2013-09-16 05:47, Rusty Russell wrote:
>
>Here's what I've got in my pending-rebases tree.
>
>@@ -842,6 +818,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
> 		return -EFAULT;
> 	name[MODULE_NAME_LEN-1] = '\0';
> 
>+	if (!(flags & O_NONBLOCK)) {
>+		printk(KERN_WARNING
>+		       "waiting module removal not supported: please upgrade");
>+	}
>+

This patch has come to my attention via the opensuse lists, where
a poster reported about this new user-visible message (it appears
in dmesg!) and rightfully asked: upgrade what component?

It's probably the userspace module loading utility, but it _really_
should say so. There's good room to read that message - a few months
into the future - as "upgrade your kernel" instead ;)

Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async

[Rusty Russell]

Jan Engelhardt <jengelh@inai.de> writes:
> On Monday 2013-09-16 05:47, Rusty Russell wrote:
>>
>>Here's what I've got in my pending-rebases tree.
>>
>>@@ -842,6 +818,11 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>> 		return -EFAULT;
>> 	name[MODULE_NAME_LEN-1] = '\0';
>> 
>>+	if (!(flags & O_NONBLOCK)) {
>>+		printk(KERN_WARNING
>>+		       "waiting module removal not supported: please upgrade");
>>+	}
>>+
>
> This patch has come to my attention via the opensuse lists, where
> a poster reported about this new user-visible message (it appears
> in dmesg!) and rightfully asked: upgrade what component?
>
> It's probably the userspace module loading utility, but it _really_
> should say so. There's good room to read that message - a few months
> into the future - as "upgrade your kernel" instead ;)

True, but honestly I never expected to see it!

Please try to discover what is doing rmmod --wait?

Thanks,
Rusty.