Re: [PATCH 1/3] srcu: Remove superfluous ssp initialization on deferred work queue

2021-04-01 Thread Paul E. McKenney
On Fri, Apr 02, 2021 at 02:58:13AM +0200, Frederic Weisbecker wrote:
> On Thu, Apr 01, 2021 at 05:48:56PM -0700, Paul E. McKenney wrote:
> > On Fri, Apr 02, 2021 at 01:47:02AM +0200, Frederic Weisbecker wrote:
> > > When an ssp has already started a grace period and queued an early work
> > > to flush after SRCU workqueues are created, we expect the ssp to be
> > > properly initialized already. So we can skip this step at this stage.
> > > 
> > > Signed-off-by: Frederic Weisbecker 
> > > Cc: Boqun Feng 
> > > Cc: Lai Jiangshan 
> > > Cc: Neeraj Upadhyay 
> > > Cc: Josh Triplett 
> > > Cc: Joel Fernandes 
> > > Cc: Uladzislau Rezki 
> > > ---
> > >  kernel/rcu/srcutree.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > > index 036ff5499ad5..7197156418e4 100644
> > > --- a/kernel/rcu/srcutree.c
> > > +++ b/kernel/rcu/srcutree.c
> > > @@ -1396,7 +1396,6 @@ void __init srcu_init(void)
> > >   while (!list_empty(_boot_list)) {
> > >   ssp = list_first_entry(_boot_list, struct srcu_struct,
> > > work.work.entry);
> > > - check_init_srcu_struct(ssp);
> > 
> > You lost me on this one.  What happens if the only pre-initialization
> > invocation on the statically allocated srcu_struct pointed to by ssp
> > was call_srcu()?  I am not seeing how the initialization has already
> > happened in that case.
> > 
> > What am I missing here?
> 
> call_srcu() -> __call_srcu() -> srcu_gp_start_if_needed() ->
> check_init_srcu_struct() ?
> 
> Or is it me missing something?

Nope, me getting confused between Tree SRCU's and Tiny SRCU's
call_srcu() implementation.  :-/

I have queued this patch and started testing.

Thanx, Paul


Re: [PATCH 1/3] srcu: Remove superfluous ssp initialization on deferred work queue

2021-04-01 Thread Paul E. McKenney
On Thu, Apr 01, 2021 at 05:48:56PM -0700, Paul E. McKenney wrote:
> On Fri, Apr 02, 2021 at 01:47:02AM +0200, Frederic Weisbecker wrote:
> > When an ssp has already started a grace period and queued an early work
> > to flush after SRCU workqueues are created, we expect the ssp to be
> > properly initialized already. So we can skip this step at this stage.
> > 
> > Signed-off-by: Frederic Weisbecker 
> > Cc: Boqun Feng 
> > Cc: Lai Jiangshan 
> > Cc: Neeraj Upadhyay 
> > Cc: Josh Triplett 
> > Cc: Joel Fernandes 
> > Cc: Uladzislau Rezki 
> > ---
> >  kernel/rcu/srcutree.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 036ff5499ad5..7197156418e4 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -1396,7 +1396,6 @@ void __init srcu_init(void)
> > while (!list_empty(_boot_list)) {
> > ssp = list_first_entry(_boot_list, struct srcu_struct,
> >   work.work.entry);
> > -   check_init_srcu_struct(ssp);
> 
> You lost me on this one.  What happens if the only pre-initialization
> invocation on the statically allocated srcu_struct pointed to by ssp
> was call_srcu()?  I am not seeing how the initialization has already
> happened in that case.
> 
> What am I missing here?

Idiot here was looking at Tiny SRCU's call_srcu(), not that of Tree SRCU.
Never mind, I will queue this one as well.

Thanx, Paul

> > list_del_init(>work.work.entry);
> > queue_work(rcu_gp_wq, >work.work);
> > }
> > -- 
> > 2.25.1
> > 


Re: [PATCH 1/3] srcu: Remove superfluous ssp initialization on deferred work queue

2021-04-01 Thread Frederic Weisbecker
On Thu, Apr 01, 2021 at 05:48:56PM -0700, Paul E. McKenney wrote:
> On Fri, Apr 02, 2021 at 01:47:02AM +0200, Frederic Weisbecker wrote:
> > When an ssp has already started a grace period and queued an early work
> > to flush after SRCU workqueues are created, we expect the ssp to be
> > properly initialized already. So we can skip this step at this stage.
> > 
> > Signed-off-by: Frederic Weisbecker 
> > Cc: Boqun Feng 
> > Cc: Lai Jiangshan 
> > Cc: Neeraj Upadhyay 
> > Cc: Josh Triplett 
> > Cc: Joel Fernandes 
> > Cc: Uladzislau Rezki 
> > ---
> >  kernel/rcu/srcutree.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> > index 036ff5499ad5..7197156418e4 100644
> > --- a/kernel/rcu/srcutree.c
> > +++ b/kernel/rcu/srcutree.c
> > @@ -1396,7 +1396,6 @@ void __init srcu_init(void)
> > while (!list_empty(_boot_list)) {
> > ssp = list_first_entry(_boot_list, struct srcu_struct,
> >   work.work.entry);
> > -   check_init_srcu_struct(ssp);
> 
> You lost me on this one.  What happens if the only pre-initialization
> invocation on the statically allocated srcu_struct pointed to by ssp
> was call_srcu()?  I am not seeing how the initialization has already
> happened in that case.
> 
> What am I missing here?

call_srcu() -> __call_srcu() -> srcu_gp_start_if_needed() ->
check_init_srcu_struct() ?

Or is it me missing something?


Re: [PATCH 1/3] srcu: Remove superfluous ssp initialization on deferred work queue

2021-04-01 Thread Paul E. McKenney
On Fri, Apr 02, 2021 at 01:47:02AM +0200, Frederic Weisbecker wrote:
> When an ssp has already started a grace period and queued an early work
> to flush after SRCU workqueues are created, we expect the ssp to be
> properly initialized already. So we can skip this step at this stage.
> 
> Signed-off-by: Frederic Weisbecker 
> Cc: Boqun Feng 
> Cc: Lai Jiangshan 
> Cc: Neeraj Upadhyay 
> Cc: Josh Triplett 
> Cc: Joel Fernandes 
> Cc: Uladzislau Rezki 
> ---
>  kernel/rcu/srcutree.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 036ff5499ad5..7197156418e4 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -1396,7 +1396,6 @@ void __init srcu_init(void)
>   while (!list_empty(_boot_list)) {
>   ssp = list_first_entry(_boot_list, struct srcu_struct,
> work.work.entry);
> - check_init_srcu_struct(ssp);

You lost me on this one.  What happens if the only pre-initialization
invocation on the statically allocated srcu_struct pointed to by ssp
was call_srcu()?  I am not seeing how the initialization has already
happened in that case.

What am I missing here?

Thanx, Paul

>   list_del_init(>work.work.entry);
>   queue_work(rcu_gp_wq, >work.work);
>   }
> -- 
> 2.25.1
> 


[PATCH 1/3] srcu: Remove superfluous ssp initialization on deferred work queue

2021-04-01 Thread Frederic Weisbecker
When an ssp has already started a grace period and queued an early work
to flush after SRCU workqueues are created, we expect the ssp to be
properly initialized already. So we can skip this step at this stage.

Signed-off-by: Frederic Weisbecker 
Cc: Boqun Feng 
Cc: Lai Jiangshan 
Cc: Neeraj Upadhyay 
Cc: Josh Triplett 
Cc: Joel Fernandes 
Cc: Uladzislau Rezki 
---
 kernel/rcu/srcutree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 036ff5499ad5..7197156418e4 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -1396,7 +1396,6 @@ void __init srcu_init(void)
while (!list_empty(_boot_list)) {
ssp = list_first_entry(_boot_list, struct srcu_struct,
  work.work.entry);
-   check_init_srcu_struct(ssp);
list_del_init(>work.work.entry);
queue_work(rcu_gp_wq, >work.work);
}
-- 
2.25.1