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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1F552A07-5799-4228-ACE2-0A8706F1D814@nvidia.com>
Date: Thu, 27 Mar 2025 17:08:35 +0000
From: Joel Fernandes <joelagnelf@...dia.com>
To: "paulmck@...nel.org" <paulmck@...nel.org>
CC: qiang Z <qiang.zhang1211@...il.com>, "rcu@...r.kernel.org"
	<rcu@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "kernel-team@...a.com"
	<kernel-team@...a.com>, "rostedt@...dmis.org" <rostedt@...dmis.org>
Subject: Re: [PATCH 5/9] rcutorture: Add tests for SRCU up/down reader
 primitives



> On Mar 27, 2025, at 12:48 PM, Paul E. McKenney <paulmck@...nel.org> 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.

- 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@...nel.org>
> 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@...nel.org>
> 
> 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++;