Currenly, it uses synchronize_rcu() to wait old rcu reader to go away
in update_shadow.In embedded platform like ARM CA7X,
load_module blocks 40~50ms in update_shadow.
When there are more than one hundred kernel modules,
it blocks several seconds.

To accelerate load_module,change synchronize_rcu to call_rcu.

Signed-off-by: Haibo Li <haibo.li@mediatek.com>
Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
---
 kernel/cfi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/cfi.c b/kernel/cfi.c
index 456771c8e454..a4836d59ca27 100644
--- a/kernel/cfi.c
+++ b/kernel/cfi.c
@@ -43,6 +43,8 @@ typedef u16 shadow_t;
 struct cfi_shadow {
 	/* Page index for the beginning of the shadow */
 	unsigned long base;
+	/* rcu to free old cfi_shadow asynchronously */
+	struct rcu_head rcu;
 	/* An array of __cfi_check locations (as indices to the shadow) */
 	shadow_t shadow[1];
 } __packed;
@@ -182,6 +184,13 @@ static void remove_module_from_shadow(struct cfi_shadow *s, struct module *mod,
 	}
 }
 
+static void _cfi_shadow_free_rcu(struct rcu_head *rcu)
+{
+	struct cfi_shadow *old = container_of(rcu, struct cfi_shadow, rcu);
+
+	vfree(old);
+}
+
 typedef void (*update_shadow_fn)(struct cfi_shadow *, struct module *,
 			unsigned long min_addr, unsigned long max_addr);
 
@@ -211,11 +220,10 @@ static void update_shadow(struct module *mod, unsigned long base_addr,
 
 	rcu_assign_pointer(cfi_shadow, next);
 	mutex_unlock(&shadow_update_lock);
-	synchronize_rcu();
 
 	if (prev) {
 		set_memory_rw((unsigned long)prev, SHADOW_PAGES);
-		vfree(prev);
+		call_rcu(&prev->rcu, _cfi_shadow_free_rcu);
 	}
 }
 
-- 
2.25.1

Hi,

On Thu, Jun 30, 2022 at 2:47 AM Haibo Li <haibo.li@mediatek.com> wrote:
>
> Currenly, it uses synchronize_rcu() to wait old rcu reader to go away
> in update_shadow.In embedded platform like ARM CA7X,
> load_module blocks 40~50ms in update_shadow.
> When there are more than one hundred kernel modules,
> it blocks several seconds.
>
> To accelerate load_module,change synchronize_rcu to call_rcu.
>
> Signed-off-by: Haibo Li <haibo.li@mediatek.com>
> Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>

Thanks for the patch! Please drop ANDROID: from the subject line,
that's only used in the Android kernel trees.

>  kernel/cfi.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cfi.c b/kernel/cfi.c
> index 456771c8e454..a4836d59ca27 100644
> --- a/kernel/cfi.c
> +++ b/kernel/cfi.c
> @@ -43,6 +43,8 @@ typedef u16 shadow_t;
>  struct cfi_shadow {
>         /* Page index for the beginning of the shadow */
>         unsigned long base;
> +       /* rcu to free old cfi_shadow asynchronously */
> +       struct rcu_head rcu;
>         /* An array of __cfi_check locations (as indices to the shadow) */
>         shadow_t shadow[1];
>  } __packed;
> @@ -182,6 +184,13 @@ static void remove_module_from_shadow(struct cfi_shadow *s, struct module *mod,
>         }
>  }
>
> +static void _cfi_shadow_free_rcu(struct rcu_head *rcu)

I think this can be simply renamed to free_shadow.

> +{
> +       struct cfi_shadow *old = container_of(rcu, struct cfi_shadow, rcu);
> +
> +       vfree(old);
> +}
> +
>  typedef void (*update_shadow_fn)(struct cfi_shadow *, struct module *,
>                         unsigned long min_addr, unsigned long max_addr);
>
> @@ -211,11 +220,10 @@ static void update_shadow(struct module *mod, unsigned long base_addr,
>
>         rcu_assign_pointer(cfi_shadow, next);
>         mutex_unlock(&shadow_update_lock);
> -       synchronize_rcu();
>
>         if (prev) {
>                 set_memory_rw((unsigned long)prev, SHADOW_PAGES);
> -               vfree(prev);
> +               call_rcu(&prev->rcu, _cfi_shadow_free_rcu);
>         }
>  }

It's probably better to keep the pages read-only until they're
actually released. Can you move the set_memory_rw call to the new
function?

Sami

> Hi,
> 
> On Thu, Jun 30, 2022 at 2:47 AM Haibo Li <haibo.li@mediatek.com> wrote:
> >
> > Currenly, it uses synchronize_rcu() to wait old rcu reader to go away
> > in update_shadow.In embedded platform like ARM CA7X, load_module
> > blocks 40~50ms in update_shadow.
> > When there are more than one hundred kernel modules, it blocks several
> > seconds.
> >
> > To accelerate load_module,change synchronize_rcu to call_rcu.
> >
> > Signed-off-by: Haibo Li <haibo.li@mediatek.com>
> > Signed-off-by: Lecopzer Chen <lecopzer.chen@mediatek.com>
> 
> Thanks for the patch! Please drop ANDROID: from the subject line, that's only
> used in the Android kernel trees.
> 
   Thanks for the comment.I will change it in next patch.  
> >  kernel/cfi.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/cfi.c b/kernel/cfi.c index
> > 456771c8e454..a4836d59ca27 100644
> > --- a/kernel/cfi.c
> > +++ b/kernel/cfi.c
> > @@ -43,6 +43,8 @@ typedef u16 shadow_t;  struct cfi_shadow {
> >         /* Page index for the beginning of the shadow */
> >         unsigned long base;
> > +       /* rcu to free old cfi_shadow asynchronously */
> > +       struct rcu_head rcu;
> >         /* An array of __cfi_check locations (as indices to the shadow) */
> >         shadow_t shadow[1];
> >  } __packed;
> > @@ -182,6 +184,13 @@ static void remove_module_from_shadow(struct
> cfi_shadow *s, struct module *mod,
> >         }
> >  }
> >
> > +static void _cfi_shadow_free_rcu(struct rcu_head *rcu)
> 
> I think this can be simply renamed to free_shadow.
   Thanks for the comment.I will change it in next patch.
> 
> > +{
> > +       struct cfi_shadow *old = container_of(rcu, struct cfi_shadow,
> > +rcu);
> > +
> > +       vfree(old);
> > +}
> > +
> >  typedef void (*update_shadow_fn)(struct cfi_shadow *, struct module *,
> >                         unsigned long min_addr, unsigned long
> > max_addr);
> >
> > @@ -211,11 +220,10 @@ static void update_shadow(struct module *mod,
> > unsigned long base_addr,
> >
> >         rcu_assign_pointer(cfi_shadow, next);
> >         mutex_unlock(&shadow_update_lock);
> > -       synchronize_rcu();
> >
> >         if (prev) {
> >                 set_memory_rw((unsigned long)prev, SHADOW_PAGES);
> > -               vfree(prev);
> > +               call_rcu(&prev->rcu, _cfi_shadow_free_rcu);
> >         }
> >  }
> 
> It's probably better to keep the pages read-only until they're actually released.
> Can you move the set_memory_rw call to the new function?
> 
   Since call_rcu and rcu callbacks change members in &prev->rcu,the old pages need to be rw at first.