| Message ID | 20220919123233.8538-2-petr.pavlu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | module: Merge same-name module load requests | expand |
On Mon, Sep 19, 2022 at 02:32:32PM +0200, Petr Pavlu wrote: > The module_wq wait queue has only non-exclusive waiters and all waits > are interruptible, therefore for consistency use wake_up_interruptible() > to wake its waiters. > > Suggested-by: Petr Mladek <pmladek@suse.com> > Signed-off-by: Petr Pavlu <petr.pavlu@suse.com> Does this fix a bug? It seems like it does. Please think of this should go to stable, for instance, does it fix a bug not yet reported? Luis
On Fri 2022-09-30 13:22:25, Luis Chamberlain wrote: > On Mon, Sep 19, 2022 at 02:32:32PM +0200, Petr Pavlu wrote: > > The module_wq wait queue has only non-exclusive waiters and all waits > > are interruptible, therefore for consistency use wake_up_interruptible() > > to wake its waiters. > > > > Suggested-by: Petr Mladek <pmladek@suse.com> > > Signed-off-by: Petr Pavlu <petr.pavlu@suse.com> > > Does this fix a bug? It seems like it does. Please think of this should > go to stable, for instance, does it fix a bug not yet reported? It is rather a clean up. It should not change the existing behavior. wake_up_all() is needed only in special situations when exclusive waiters are used. Also wake_up_*() variant should match the related wait_for_completion_*() variants. The patch looks good: Reviewed-by: Petr Mladek <pmladek@suse.com> Best Regards, Petr
diff --git a/kernel/module/main.c b/kernel/module/main.c index a4e4d84b6f4e..06cb41c3d2a1 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -763,7 +763,7 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, free_module(mod); /* someone could wait for the module in add_unformed_module() */ - wake_up_all(&module_wq); + wake_up_interruptible(&module_wq); return 0; out: mutex_unlock(&module_mutex); @@ -2524,7 +2524,7 @@ static noinline int do_init_module(struct module *mod) schedule_work(&init_free_wq); mutex_unlock(&module_mutex); - wake_up_all(&module_wq); + wake_up_interruptible(&module_wq); return 0; @@ -2540,7 +2540,7 @@ static noinline int do_init_module(struct module *mod) klp_module_going(mod); ftrace_release_mod(mod); free_module(mod); - wake_up_all(&module_wq); + wake_up_interruptible(&module_wq); return ret; } @@ -2886,7 +2886,7 @@ static int load_module(struct load_info *info, const char __user *uargs, /* Unlink carefully: kallsyms could be walking list. */ list_del_rcu(&mod->list); mod_tree_remove(mod); - wake_up_all(&module_wq); + wake_up_interruptible(&module_wq); /* Wait for RCU-sched synchronizing before releasing mod->list. */ synchronize_rcu(); mutex_unlock(&module_mutex);
The module_wq wait queue has only non-exclusive waiters and all waits are interruptible, therefore for consistency use wake_up_interruptible() to wake its waiters. Suggested-by: Petr Mladek <pmladek@suse.com> Signed-off-by: Petr Pavlu <petr.pavlu@suse.com> --- kernel/module/main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)