diff mbox series

module: avoid calling synchronize_rcu()

Message ID 20220302011306.2054550-1-lv.ruyi@zte.com.cn (mailing list archive)
State New, archived
Headers show
Series module: avoid calling synchronize_rcu() | expand

Commit Message

CGEL March 2, 2022, 1:13 a.m. UTC
From: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>

Kfree_rcu() usually results in even simpler code than does 
synchronize_rcu() without synchronize_rcu()'s multi-millisecond
latency, so replace synchronize_rcu() with kfree_rcu().

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>
---
 kernel/module.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Miroslav Benes March 2, 2022, 9:14 a.m. UTC | #1
Hi,

On Wed, 2 Mar 2022, cgel.zte@gmail.com wrote:

> From: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>
> 
> Kfree_rcu() usually results in even simpler code than does 
> synchronize_rcu() without synchronize_rcu()'s multi-millisecond
> latency, so replace synchronize_rcu() with kfree_rcu().
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>
> ---
>  kernel/module.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 6cea788fd965..767b5f9e5819 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -4138,8 +4138,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>   ddebug_cleanup:
>  	ftrace_release_mod(mod);
>  	dynamic_debug_remove(mod, info->debug);
> -	synchronize_rcu();
> -	kfree(mod->args);
> +	kfree_rcu(mod->args);

this has been proposed already. synchronize_rcu() and kfree() here are not 
really tied together. See the discussion at 
https://lore.kernel.org/all/alpine.LSU.2.21.2111301132220.3922@pobox.suse.cz/T/#u

Regards
Miroslav
Luis Chamberlain March 2, 2022, 10:48 p.m. UTC | #2
On Wed, Mar 02, 2022 at 10:14:00AM +0100, Miroslav Benes wrote:
> Hi,
> 
> On Wed, 2 Mar 2022, cgel.zte@gmail.com wrote:
> 
> > From: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>
> > 
> > Kfree_rcu() usually results in even simpler code than does 
> > synchronize_rcu() without synchronize_rcu()'s multi-millisecond
> > latency, so replace synchronize_rcu() with kfree_rcu().
> > 
> > Reported-by: Zeal Robot <zealci@zte.com.cn>
> > Signed-off-by: Lv Ruyi (CGEL ZTE) <lv.ruyi@zte.com.cn>
> > ---
> >  kernel/module.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 6cea788fd965..767b5f9e5819 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -4138,8 +4138,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
> >   ddebug_cleanup:
> >  	ftrace_release_mod(mod);
> >  	dynamic_debug_remove(mod, info->debug);
> > -	synchronize_rcu();
> > -	kfree(mod->args);
> > +	kfree_rcu(mod->args);
> 
> this has been proposed already. synchronize_rcu() and kfree() here are not 
> really tied together. See the discussion at 
> https://lore.kernel.org/all/alpine.LSU.2.21.2111301132220.3922@pobox.suse.cz/T/#u

Aaron, can you add a nice comment here to explain this while at it?
Otherwise this will be lost tribal knowledge.

Lv Ruyi, please open source your Zeal Robot. Thanks!

  Luis
diff mbox series

Patch

diff --git a/kernel/module.c b/kernel/module.c
index 6cea788fd965..767b5f9e5819 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4138,8 +4138,7 @@  static int load_module(struct load_info *info, const char __user *uargs,
  ddebug_cleanup:
 	ftrace_release_mod(mod);
 	dynamic_debug_remove(mod, info->debug);
-	synchronize_rcu();
-	kfree(mod->args);
+	kfree_rcu(mod->args);
  free_arch_cleanup:
 	cfi_cleanup(mod);
 	module_arch_cleanup(mod);