Re: [PATCH 5/9] rcutorture: Add tests for SRCU up/down reader primitives

[Paul E. McKenney]

On Thu, Mar 27, 2025 at 05:08:35PM +0000, Joel Fernandes wrote:
> 
> 
> > On Mar 27, 2025, at 12:48 PM, Paul E. McKenney <paulmck@xxxxxxxxxx> wrote:
> > 
> > On Thu, Mar 27, 2025 at 12:22:12PM -0400, Joel Fernandes wrote:
> >> Paul,
> >> 
> >>>> If rtorsu_hrt timer is still in timer_queue, invoke hrtimer_cancel() will
> >>>> remove it from timerqueue and directly return, so the rcu_torture_updown_hrt()
> >>>> will not be executed and the rtorsup->rtorsu_inuse cannot be set false.
> >>>> 
> >>>> How about modifying it as follows:
> >>>> 
> >>>> diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> >>>> index 04d7a2173b95..ecf3d3797f7e 100644
> >>>> --- a/kernel/rcu/rcutorture.c
> >>>> +++ b/kernel/rcu/rcutorture.c
> >>>> @@ -2502,8 +2502,7 @@ static void rcu_torture_updown_cleanup(void)
> >>>>        for (rtorsup = updownreaders; rtorsup <
> >>>> &updownreaders[n_up_down]; rtorsup++) {
> >>>>                if (!smp_load_acquire(&rtorsup->rtorsu_inuse))
> >>>>                        continue;
> >>>> -               (void)hrtimer_cancel(&rtorsup->rtorsu_hrt);
> >>>> -               if (WARN_ON_ONCE(rtorsup->rtorsu_inuse)) {
> >>>> +               if (hrtimer_cancel(&rtorsup->rtorsu_hrt) ||
> >>>> WARN_ON_ONCE(rtorsup->rtorsu_inuse)) {
> >>>> 
> >>>> rcu_torture_one_read_end(&rtorsup->rtorsu_rtors, &rtorsup->rtorsu_trs,
> >>>> -1);
> >>>>                        WARN_ONCE(rtorsup->rtorsu_nups >=
> >>>> rtorsup->rtorsu_ndowns, "%s: Up without matching down #%zu.\n",
> >>>> __func__, rtorsup - updownreaders);
> >>>>                        rtorsup->rtorsu_nups++;
> >>> 
> >>> Good eyes, thank you!  I have applied this fix with attribution.
> >> 
> >> Could you re-send the series, or should I apply the fix the patch myself? Or
> >> provide the new patch inline here.
> > 
> > Your choice, just let me know.  If you have modified any of the other
> > patches in that series, it will probably be easier for you if I either
> > resend just that one patch or if you apply the changes.  If you haven't
> > done any modifications, it might be easier for you if I re-sent the
> > series.
> 
> Thanks, you could resend as I have not changed other patches.

Will do!  I expect to get that to you by end of this coming Monday
at the latest.

							Thanx, Paul

> - Joel
> 
> > 
> > I have the delta patch below, which I have pushed out for kernel test
> > robot ministrations and which I expect to merge into the original
> > later today.
> > 
> >                            Thanx, Paul
> > 
> > ------------------------------------------------------------------------
> > 
> > commit 55fcac5cb3fc96479d935db648c98503cb0a944b
> > Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > Date:   Thu Mar 27 07:29:48 2025 -0700
> > 
> >    squash! rcutorture: Add tests for SRCU up/down reader primitives
> > 
> >    [ paulmck: Apply Z qiang feedback. ]
> > 
> >    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > 
> > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
> > index 04d7a2173b95d..ecf3d3797f7e1 100644
> > --- a/kernel/rcu/rcutorture.c
> > +++ b/kernel/rcu/rcutorture.c
> > @@ -2502,8 +2502,7 @@ static void rcu_torture_updown_cleanup(void)
> >    for (rtorsup = updownreaders; rtorsup < &updownreaders[n_up_down]; rtorsup++) {
> >        if (!smp_load_acquire(&rtorsup->rtorsu_inuse))
> >            continue;
> > -        (void)hrtimer_cancel(&rtorsup->rtorsu_hrt);
> > -        if (WARN_ON_ONCE(rtorsup->rtorsu_inuse)) {
> > +        if (hrtimer_cancel(&rtorsup->rtorsu_hrt) || WARN_ON_ONCE(rtorsup->rtorsu_inuse)) {
> >            rcu_torture_one_read_end(&rtorsup->rtorsu_rtors, &rtorsup->rtorsu_trs, -1);
> >            WARN_ONCE(rtorsup->rtorsu_nups >= rtorsup->rtorsu_ndowns, "%s: Up without matching down #%zu.\n", __func__, rtorsup - updownreaders);
> >            rtorsup->rtorsu_nups++;