[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87wqmh1l8b.fsf@rustcorp.com.au>
Date: Mon, 16 Sep 2013 13:17:16 +0930
From: Rusty Russell <rusty@...tcorp.com.au>
To: Lucas De Marchi <lucas.de.marchi@...il.com>
Cc: Peter Chen <peter.chen@...escale.com>,
lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/1] module: Make wait module's refcount to zero procedure as async
Lucas De Marchi <lucas.de.marchi@...il.com> writes:
> On Thu, Sep 12, 2013 at 9:30 PM, Rusty Russell <rusty@...tcorp.com.au> wrote:
>> Peter Chen <peter.chen@...escale.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@...il.com>
Signed-off-by: Rusty Russell <rusty@...tcorp.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();
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists