All of lore.kernel.org
 help / color / mirror / Atom feed
* Query regarding srcu_funnel_exp_start()
@ 2017-10-27  8:53 Neeraj Upadhyay
  2017-10-27 12:26 ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Neeraj Upadhyay @ 2017-10-27  8:53 UTC (permalink / raw)
  To: paulmck, josh, rostedt, mathieu.desnoyers, jiangshanlai; +Cc: LKML

Hi,

One query regarding srcu_funnel_exp_start() function in 
kernel/rcu/srcutree.c.

static void srcu_funnel_exp_start(struct srcu_struct *sp, struct 
srcu_node *snp,
				  unsigned long s)
{
	<snip>
	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
		sp->srcu_gp_seq_needed_exp = s;
	<snip>
}

Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
's'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?


Thanks
Neeraj

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Query regarding srcu_funnel_exp_start()
  2017-10-27  8:53 Query regarding srcu_funnel_exp_start() Neeraj Upadhyay
@ 2017-10-27 12:26 ` Paul E. McKenney
  2017-10-27 16:45   ` Neeraj Upadhyay
  0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-27 12:26 UTC (permalink / raw)
  To: Neeraj Upadhyay; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, LKML

On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote:
> Hi,
> 
> One query regarding srcu_funnel_exp_start() function in
> kernel/rcu/srcutree.c.
> 
> static void srcu_funnel_exp_start(struct srcu_struct *sp, struct
> srcu_node *snp,
> 				  unsigned long s)
> {
> 	<snip>
> 	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
> 		sp->srcu_gp_seq_needed_exp = s;
> 	<snip>
> }
> 
> Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
> 's'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?

Let's suppose that it is incorrect as currently written.  Can you
construct a test case demonstrating a failure of some sort, then provide
a fix?

To start with, if it is currently incorrect, what would be the nature
of the failure?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Query regarding srcu_funnel_exp_start()
  2017-10-27 12:26 ` Paul E. McKenney
@ 2017-10-27 16:45   ` Neeraj Upadhyay
  2017-10-27 22:20     ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Neeraj Upadhyay @ 2017-10-27 16:45 UTC (permalink / raw)
  To: paulmck; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, LKML

On 10/27/2017 05:56 PM, Paul E. McKenney wrote:
> On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote:
>> Hi,
>>
>> One query regarding srcu_funnel_exp_start() function in
>> kernel/rcu/srcutree.c.
>>
>> static void srcu_funnel_exp_start(struct srcu_struct *sp, struct
>> srcu_node *snp,
>> 				  unsigned long s)
>> {
>> 	<snip>
>> 	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
>> 		sp->srcu_gp_seq_needed_exp = s;
>> 	<snip>
>> }
>>
>> Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
>> 's'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?
> 
> Let's suppose that it is incorrect as currently written.  Can you
> construct a test case demonstrating a failure of some sort, then provide
> a fix?

Will check this. Might take some time to build a test case.

> 
> To start with, if it is currently incorrect, what would be the nature
> of the failure?
> 
> 							Thanx, Paul
>

Hi Paul,

I see below scenario, where new gp won't be expedited. Please correct
me if I am missing something here.

1. CPU0 calls synchronize_srcu_expedited()

synchronize_srcu_expedited()
   __synchronize_srcu()
     __call_srcu()
             s = rcu_seq_snap(&sp->srcu_gp_seq); // lets say srcu_gp_seq 
  = 0;
                                                 // s = 0x100
             sdp->srcu_gp_seq_needed = s // 0x100
             needgp = true
             sdp->srcu_gp_seq_needed_exp = s // 0x100
         srcu_funnel_gp_start()
                 sp->srcu_gp_seq_needed_exp = s;
             srcu_gp_start(sp);
                 rcu_seq_start(&sp->srcu_gp_seq);

2. CPU1 calls normal synchronize_srcu()

synchronize_srcu()
     __synchronize_srcu(sp, true)
         __call_srcu()
                 s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
                                                     // s= 0x200
                 sdp->srcu_gp_seq_needed = s; // 0x200
             srcu_funnel_gp_start()
                 smp_store_release(&sp->srcu_gp_seq_needed, s); // 0x200

3. CPU3 calls synchronize_srcu_expedited()

synchronize_srcu_expedited()
   __synchronize_srcu()
     __call_srcu()
             s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
                                                 // s = 0x200
             sdp->srcu_gp_seq_needed_exp = s // 0x200
         srcu_funnel_exp_start(sp, sdp->mynode, s);
             // sp->srcu_gp_seq_needed_exp = 0x100
             // s = 0x200 ; sp->srcu_gp_seq_needed_exp is not updated
             if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
                 sp->srcu_gp_seq_needed_exp = s;


Thanks
Neeraj






-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Query regarding srcu_funnel_exp_start()
  2017-10-27 16:45   ` Neeraj Upadhyay
@ 2017-10-27 22:20     ` Paul E. McKenney
  2017-10-28  3:49       ` Neeraj Upadhyay
  0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-27 22:20 UTC (permalink / raw)
  To: Neeraj Upadhyay; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, LKML

On Fri, Oct 27, 2017 at 10:15:04PM +0530, Neeraj Upadhyay wrote:
> On 10/27/2017 05:56 PM, Paul E. McKenney wrote:
> >On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote:
> >>Hi,
> >>
> >>One query regarding srcu_funnel_exp_start() function in
> >>kernel/rcu/srcutree.c.
> >>
> >>static void srcu_funnel_exp_start(struct srcu_struct *sp, struct
> >>srcu_node *snp,
> >>				  unsigned long s)
> >>{
> >>	<snip>
> >>	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
> >>		sp->srcu_gp_seq_needed_exp = s;
> >>	<snip>
> >>}
> >>
> >>Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
> >>'s'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?
> >
> >Let's suppose that it is incorrect as currently written.  Can you
> >construct a test case demonstrating a failure of some sort, then provide
> >a fix?
> 
> Will check this. Might take some time to build a test case.

Fair enough!

I suggest checking to see if kernel/rcu/rcuperf.c can do what you need for
this test.  (Might not with a single test, but perhaps a before-and-after
comparison.  Or maybe you really do need to add some test code somewhere.)

> >To start with, if it is currently incorrect, what would be the nature
> >of the failure?
> >
> >							Thanx, Paul
> >
> 
> Hi Paul,
> 
> I see below scenario, where new gp won't be expedited. Please correct
> me if I am missing something here.
> 
> 1. CPU0 calls synchronize_srcu_expedited()
> 
> synchronize_srcu_expedited()
>   __synchronize_srcu()
>     __call_srcu()
>             s = rcu_seq_snap(&sp->srcu_gp_seq); // lets say
> srcu_gp_seq  = 0;
>                                                 // s = 0x100

Looks like you have one hex digit and then two binary digits, but why not?
(RCU_SEQ_STATE_MASK is 3 rather than 0xff.)

>             sdp->srcu_gp_seq_needed = s // 0x100
>             needgp = true
>             sdp->srcu_gp_seq_needed_exp = s // 0x100
>         srcu_funnel_gp_start()
>                 sp->srcu_gp_seq_needed_exp = s;
>             srcu_gp_start(sp);
>                 rcu_seq_start(&sp->srcu_gp_seq);
> 
> 2. CPU1 calls normal synchronize_srcu()
> 
> synchronize_srcu()
>     __synchronize_srcu(sp, true)
>         __call_srcu()
>                 s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
>                                                     // s= 0x200
>                 sdp->srcu_gp_seq_needed = s; // 0x200
>             srcu_funnel_gp_start()
>                 smp_store_release(&sp->srcu_gp_seq_needed, s); // 0x200
> 
> 3. CPU3 calls synchronize_srcu_expedited()
> 
> synchronize_srcu_expedited()
>   __synchronize_srcu()
>     __call_srcu()
>             s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
>                                                 // s = 0x200
>             sdp->srcu_gp_seq_needed_exp = s // 0x200
>         srcu_funnel_exp_start(sp, sdp->mynode, s);
>             // sp->srcu_gp_seq_needed_exp = 0x100
>             // s = 0x200 ; sp->srcu_gp_seq_needed_exp is not updated
>             if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
>                 sp->srcu_gp_seq_needed_exp = s;

Seems plausible, but you should be able to show the difference in
grace-period duration with a test.

While you are in srcu_funnel_exp_start(), should it be rechecking
rcu_seq_done(&sp->srcu_gp_seq, s) as well as the current
ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s) under the lock?
Why or why not?

							Thanx, Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Query regarding srcu_funnel_exp_start()
  2017-10-27 22:20     ` Paul E. McKenney
@ 2017-10-28  3:49       ` Neeraj Upadhyay
  2017-10-29 19:24         ` Paul E. McKenney
  0 siblings, 1 reply; 6+ messages in thread
From: Neeraj Upadhyay @ 2017-10-28  3:49 UTC (permalink / raw)
  To: paulmck; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, LKML

On 10/28/2017 03:50 AM, Paul E. McKenney wrote:
> On Fri, Oct 27, 2017 at 10:15:04PM +0530, Neeraj Upadhyay wrote:
>> On 10/27/2017 05:56 PM, Paul E. McKenney wrote:
>>> On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote:
>>>> Hi,
>>>>
>>>> One query regarding srcu_funnel_exp_start() function in
>>>> kernel/rcu/srcutree.c.
>>>>
>>>> static void srcu_funnel_exp_start(struct srcu_struct *sp, struct
>>>> srcu_node *snp,
>>>> 				  unsigned long s)
>>>> {
>>>> 	<snip>
>>>> 	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
>>>> 		sp->srcu_gp_seq_needed_exp = s;
>>>> 	<snip>
>>>> }
>>>>
>>>> Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
>>>> 's'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?
>>>
>>> Let's suppose that it is incorrect as currently written.  Can you
>>> construct a test case demonstrating a failure of some sort, then provide
>>> a fix?
>>
>> Will check this. Might take some time to build a test case.
> 
> Fair enough!
> 
> I suggest checking to see if kernel/rcu/rcuperf.c can do what you need for
> this test.  (Might not with a single test, but perhaps a before-and-after
> comparison.  Or maybe you really do need to add some test code somewhere.)
> 

Thanks for the suggestion, will try that out.

>>> To start with, if it is currently incorrect, what would be the nature
>>> of the failure?
>>>
>>> 							Thanx, Paul
>>>
>>
>> Hi Paul,
>>
>> I see below scenario, where new gp won't be expedited. Please correct
>> me if I am missing something here.
>>
>> 1. CPU0 calls synchronize_srcu_expedited()
>>
>> synchronize_srcu_expedited()
>>    __synchronize_srcu()
>>      __call_srcu()
>>              s = rcu_seq_snap(&sp->srcu_gp_seq); // lets say
>> srcu_gp_seq  = 0;
>>                                                  // s = 0x100
> 
> Looks like you have one hex digit and then two binary digits, but why not?
> (RCU_SEQ_STATE_MASK is 3 rather than 0xff >

Yeah, sorry I confused myself while representing the values. 0x100 need 
to be replaced with b'100' and 0x200 with b'1000'.

>>              sdp->srcu_gp_seq_needed = s // 0x100
>>              needgp = true
>>              sdp->srcu_gp_seq_needed_exp = s // 0x100
>>          srcu_funnel_gp_start()
>>                  sp->srcu_gp_seq_needed_exp = s;
>>              srcu_gp_start(sp);
>>                  rcu_seq_start(&sp->srcu_gp_seq);
>>
>> 2. CPU1 calls normal synchronize_srcu()
>>
>> synchronize_srcu()
>>      __synchronize_srcu(sp, true)
>>          __call_srcu()
>>                  s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
>>                                                      // s= 0x200
>>                  sdp->srcu_gp_seq_needed = s; // 0x200
>>              srcu_funnel_gp_start()
>>                  smp_store_release(&sp->srcu_gp_seq_needed, s); // 0x200
>>
>> 3. CPU3 calls synchronize_srcu_expedited()
>>
>> synchronize_srcu_expedited()
>>    __synchronize_srcu()
>>      __call_srcu()
>>              s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
>>                                                  // s = 0x200
>>              sdp->srcu_gp_seq_needed_exp = s // 0x200
>>          srcu_funnel_exp_start(sp, sdp->mynode, s);
>>              // sp->srcu_gp_seq_needed_exp = 0x100
>>              // s = 0x200 ; sp->srcu_gp_seq_needed_exp is not updated
>>              if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
>>                  sp->srcu_gp_seq_needed_exp = s;
> 
> Seems plausible, but you should be able to show the difference in
> grace-period duration with a test.
> 

Ok sure, will attempt that.

> While you are in srcu_funnel_exp_start(), should it be rechecking
> rcu_seq_done(&sp->srcu_gp_seq, s) as well as the current
> ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s) under the lock?
> Why or why not?
> 
> 							Thanx, Paul
> 

Hi Paul,

I don't see how it will impact. I have put markers in code snippet
below to explain my points. My understanding is

* rcu_seq_done check @a is a fastpath return, and avoid contention
for snp lock, if the gp has already elapsed.

* Checking it @b, inside srcu_node  lock might not make any
difference, as sp->srcu_gp_seq counter portion is updated
under srcu_struct lock. Also, we cannot lock srcu_struct at this
point, as it will cause lock contention among multiple CPUs.

* Checking rcu_seq_done @c also does not impact, as we have already
done all the work of traversing the entire parent chain and if
rcu_seq_done() is true srcu_gp_seq_needed_exp will be greater
than or equal to 's'.

   srcu_gp_end()
     raw_spin_lock_irq_rcu_node(sp);
     rcu_seq_end(&sp->srcu_gp_seq);
     gpseq = rcu_seq_current(&sp->srcu_gp_seq);
     if (ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, gpseq))
         sp->srcu_gp_seq_needed_exp = gpseq;
     raw_spin_unlock_irq_rcu_node(sp);

static void srcu_funnel_exp_start(...)
{
     <snip>

     for (; snp != NULL; snp = snp->srcu_parent) {
         if (rcu_seq_done(&sp->srcu_gp_seq, s) ||  /* a */
             ULONG_CMP_GE(READ_ONCE(snp->srcu_gp_seq_needed_exp), s))
             return;
         raw_spin_lock_irqsave_rcu_node(snp, flags);
         /* b */
         if (ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s)) {
             raw_spin_unlock_irqrestore_rcu_node(snp, flags);
             return;
         }
         <snip>
         raw_spin_unlock_irqrestore_rcu_node(snp, flags);
     }
     raw_spin_lock_irqsave_rcu_node(sp, flags);
     /* c */
     if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
         sp->srcu_gp_seq_needed_exp = s;
     raw_spin_unlock_irqrestore_rcu_node(sp, flags);
}

Thanks
Neeraj

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of the Code Aurora Forum, hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Query regarding srcu_funnel_exp_start()
  2017-10-28  3:49       ` Neeraj Upadhyay
@ 2017-10-29 19:24         ` Paul E. McKenney
  0 siblings, 0 replies; 6+ messages in thread
From: Paul E. McKenney @ 2017-10-29 19:24 UTC (permalink / raw)
  To: Neeraj Upadhyay; +Cc: josh, rostedt, mathieu.desnoyers, jiangshanlai, LKML

On Sat, Oct 28, 2017 at 09:19:52AM +0530, Neeraj Upadhyay wrote:
> On 10/28/2017 03:50 AM, Paul E. McKenney wrote:
> >On Fri, Oct 27, 2017 at 10:15:04PM +0530, Neeraj Upadhyay wrote:
> >>On 10/27/2017 05:56 PM, Paul E. McKenney wrote:
> >>>On Fri, Oct 27, 2017 at 02:23:07PM +0530, Neeraj Upadhyay wrote:
> >>>>Hi,
> >>>>
> >>>>One query regarding srcu_funnel_exp_start() function in
> >>>>kernel/rcu/srcutree.c.
> >>>>
> >>>>static void srcu_funnel_exp_start(struct srcu_struct *sp, struct
> >>>>srcu_node *snp,
> >>>>				  unsigned long s)
> >>>>{
> >>>>	<snip>
> >>>>	if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
> >>>>		sp->srcu_gp_seq_needed_exp = s;
> >>>>	<snip>
> >>>>}
> >>>>
> >>>>Why is sp->srcu_gp_seq_needed_exp set to 's' if srcu_gp_seq_needed_exp is >=
> >>>>'s'. Shouldn't srcu_gp_seq_needed_exp be equal to the greater of both?
> >>>
> >>>Let's suppose that it is incorrect as currently written.  Can you
> >>>construct a test case demonstrating a failure of some sort, then provide
> >>>a fix?
> >>
> >>Will check this. Might take some time to build a test case.
> >
> >Fair enough!
> >
> >I suggest checking to see if kernel/rcu/rcuperf.c can do what you need for
> >this test.  (Might not with a single test, but perhaps a before-and-after
> >comparison.  Or maybe you really do need to add some test code somewhere.)
> 
> Thanks for the suggestion, will try that out.
> 
> >>>To start with, if it is currently incorrect, what would be the nature
> >>>of the failure?
> >>>
> >>>							Thanx, Paul
> >>>
> >>
> >>Hi Paul,
> >>
> >>I see below scenario, where new gp won't be expedited. Please correct
> >>me if I am missing something here.
> >>
> >>1. CPU0 calls synchronize_srcu_expedited()
> >>
> >>synchronize_srcu_expedited()
> >>   __synchronize_srcu()
> >>     __call_srcu()
> >>             s = rcu_seq_snap(&sp->srcu_gp_seq); // lets say
> >>srcu_gp_seq  = 0;
> >>                                                 // s = 0x100
> >
> >Looks like you have one hex digit and then two binary digits, but why not?
> >(RCU_SEQ_STATE_MASK is 3 rather than 0xff >
> 
> Yeah, sorry I confused myself while representing the values. 0x100
> need to be replaced with b'100' and 0x200 with b'1000'.

Sounds like something I would do!  ;-)

> >>             sdp->srcu_gp_seq_needed = s // 0x100
> >>             needgp = true
> >>             sdp->srcu_gp_seq_needed_exp = s // 0x100
> >>         srcu_funnel_gp_start()
> >>                 sp->srcu_gp_seq_needed_exp = s;
> >>             srcu_gp_start(sp);
> >>                 rcu_seq_start(&sp->srcu_gp_seq);
> >>
> >>2. CPU1 calls normal synchronize_srcu()
> >>
> >>synchronize_srcu()
> >>     __synchronize_srcu(sp, true)
> >>         __call_srcu()
> >>                 s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
> >>                                                     // s= 0x200
> >>                 sdp->srcu_gp_seq_needed = s; // 0x200
> >>             srcu_funnel_gp_start()
> >>                 smp_store_release(&sp->srcu_gp_seq_needed, s); // 0x200
> >>
> >>3. CPU3 calls synchronize_srcu_expedited()
> >>
> >>synchronize_srcu_expedited()
> >>   __synchronize_srcu()
> >>     __call_srcu()
> >>             s = rcu_seq_snap(&sp->srcu_gp_seq); // srcu_gp_seq = 1
> >>                                                 // s = 0x200
> >>             sdp->srcu_gp_seq_needed_exp = s // 0x200
> >>         srcu_funnel_exp_start(sp, sdp->mynode, s);
> >>             // sp->srcu_gp_seq_needed_exp = 0x100
> >>             // s = 0x200 ; sp->srcu_gp_seq_needed_exp is not updated
> >>             if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
> >>                 sp->srcu_gp_seq_needed_exp = s;
> >
> >Seems plausible, but you should be able to show the difference in
> >grace-period duration with a test.
> >
> 
> Ok sure, will attempt that.
> 
> >While you are in srcu_funnel_exp_start(), should it be rechecking
> >rcu_seq_done(&sp->srcu_gp_seq, s) as well as the current
> >ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s) under the lock?
> >Why or why not?
> >
> >							Thanx, Paul
> 
> Hi Paul,
> 
> I don't see how it will impact. I have put markers in code snippet
> below to explain my points. My understanding is
> 
> * rcu_seq_done check @a is a fastpath return, and avoid contention
> for snp lock, if the gp has already elapsed.
> 
> * Checking it @b, inside srcu_node  lock might not make any
> difference, as sp->srcu_gp_seq counter portion is updated
> under srcu_struct lock. Also, we cannot lock srcu_struct at this
> point, as it will cause lock contention among multiple CPUs.
> 
> * Checking rcu_seq_done @c also does not impact, as we have already
> done all the work of traversing the entire parent chain and if
> rcu_seq_done() is true srcu_gp_seq_needed_exp will be greater
> than or equal to 's'.
> 
>   srcu_gp_end()
>     raw_spin_lock_irq_rcu_node(sp);
>     rcu_seq_end(&sp->srcu_gp_seq);
>     gpseq = rcu_seq_current(&sp->srcu_gp_seq);
>     if (ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, gpseq))
>         sp->srcu_gp_seq_needed_exp = gpseq;
>     raw_spin_unlock_irq_rcu_node(sp);
> 
> static void srcu_funnel_exp_start(...)
> {
>     <snip>
> 
>     for (; snp != NULL; snp = snp->srcu_parent) {
>         if (rcu_seq_done(&sp->srcu_gp_seq, s) ||  /* a */
>             ULONG_CMP_GE(READ_ONCE(snp->srcu_gp_seq_needed_exp), s))
>             return;
>         raw_spin_lock_irqsave_rcu_node(snp, flags);
>         /* b */
>         if (ULONG_CMP_GE(snp->srcu_gp_seq_needed_exp, s)) {
>             raw_spin_unlock_irqrestore_rcu_node(snp, flags);
>             return;
>         }
>         <snip>
>         raw_spin_unlock_irqrestore_rcu_node(snp, flags);
>     }
>     raw_spin_lock_irqsave_rcu_node(sp, flags);
>     /* c */
>     if (!ULONG_CMP_LT(sp->srcu_gp_seq_needed_exp, s))
>         sp->srcu_gp_seq_needed_exp = s;
>     raw_spin_unlock_irqrestore_rcu_node(sp, flags);
> }

That does match my understanding, thank you for taking the time
to go through it!  Especially given that my understanding has
proven to be wrong from time to time.  ;-)

						Thanx, Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-29 19:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27  8:53 Query regarding srcu_funnel_exp_start() Neeraj Upadhyay
2017-10-27 12:26 ` Paul E. McKenney
2017-10-27 16:45   ` Neeraj Upadhyay
2017-10-27 22:20     ` Paul E. McKenney
2017-10-28  3:49       ` Neeraj Upadhyay
2017-10-29 19:24         ` Paul E. McKenney

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.