2022-03-02 04:05:26

by CGEL

Subject: [PATCH] module: avoid calling synchronize_rcu()

From: Lv Ruyi (CGEL ZTE) <[email protected]>

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 <[email protected]>
Signed-off-by: Lv Ruyi (CGEL ZTE) <[email protected]>
---
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);
free_arch_cleanup:
cfi_cleanup(mod);
module_arch_cleanup(mod);
--
2.25.1

2022-03-02 14:29:06

by Miroslav Benes

Subject: Re: [PATCH] module: avoid calling synchronize_rcu()

Hi,

On Wed, 2 Mar 2022, [email protected] wrote:

> From: Lv Ruyi (CGEL ZTE) <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Lv Ruyi (CGEL ZTE) <[email protected]>
> ---
> 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/[email protected]/T/#u

Regards
Miroslav

2022-03-03 00:33:46

by Luis Chamberlain

Subject: Re: [PATCH] module: avoid calling synchronize_rcu()

On Wed, Mar 02, 2022 at 10:14:00AM +0100, Miroslav Benes wrote:
> Hi,
>
> On Wed, 2 Mar 2022, [email protected] wrote:
>
> > From: Lv Ruyi (CGEL ZTE) <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Lv Ruyi (CGEL ZTE) <[email protected]>
> > ---
> > 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/[email protected]/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