One-off rcu_nocb_rdp_deoffload bug

One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

Hello, Frederic,

I have seen this once on Neeraj's tree with a few commits on top (-rcu
commit 46774278c74f ("rcutorture: Test start-poll primitives with
interrupts disabled").  But only the once so far.

This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
rcu_nocb_rdp_deoffload().

Thoughts?

						Thanx, Paul

------------------------------------------------------------------------

[18839.499664] ------------[ cut here ]------------
[18839.500993] WARNING: CPU: 19 PID: 100 at kernel/rcu/tree_nocb.h:1061 rcu_nocb_rdp_deoffload+0x292/0x2a0
[18839.503545] Modules linked in:
[18839.504443] CPU: 19 UID: 0 PID: 100 Comm: rcu_nocb_toggle Not tainted 6.11.0-rc3-00459-gba3eb80edec7-dirty #1553
[18839.507190] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[18839.510002] RIP: 0010:rcu_nocb_rdp_deoffload+0x292/0x2a0
[18839.511451] Code: e9 3c ff ff ff 4c 89 e6 48 89 ef e8 98 0d e1 00 e9 3b fe ff ff 90 0f 0b 90 48 8b 83 c0 00 00 00 48 85 c0 0f 84 0b fe ff ff 90 <0f> 0b 90 e9 02 fe ff ff e8 e1 60 e0 00 90 90 90 90 90 90 90 90 90
[18839.516469] RSP: 0000:ffffa05a8049be30 EFLAGS: 00010006
[18839.517908] RAX: 0000000000000081 RBX: ffff956c5eaaf480 RCX: 0000000000000000
[18839.519849] RDX: 0000000000000001 RSI: 0000000000000287 RDI: ffff956c5eaaf5f0
[18839.521804] RBP: ffff956c5eaaf5f0 R08: 0000112269aee7fd R09: 0000000000000001
[18839.523747] R10: 00000000000020e8 R11: 0000000000000000 R12: 0000000000000292
[18839.525623] R13: 0000000000000000 R14: ffff956c5ea2f480 R15: 0000000000000001
[18839.527557] FS:  0000000000000000(0000) GS:ffff956c5eec0000(0000) knlGS:0000000000000000
[18839.529917] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[18839.531556] CR2: 0000000000000000 CR3: 0000000001cfa000 CR4: 00000000000006f0
[18839.533495] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[18839.535395] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[18839.537322] Call Trace:
[18839.538006]  <TASK>
[18839.538596]  ? __warn+0x7e/0x120
[18839.539491]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
[18839.540757]  ? report_bug+0x18e/0x1a0
[18839.541805]  ? handle_bug+0x3d/0x70
[18839.542837]  ? exc_invalid_op+0x18/0x70
[18839.543959]  ? asm_exc_invalid_op+0x1a/0x20
[18839.545165]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
[18839.546547]  rcu_nocb_cpu_deoffload+0x70/0xa0
[18839.547814]  rcu_nocb_toggle+0x136/0x1c0
[18839.548960]  ? __pfx_rcu_nocb_toggle+0x10/0x10
[18839.550073]  kthread+0xd1/0x100
[18839.550958]  ? __pfx_kthread+0x10/0x10
[18839.552008]  ret_from_fork+0x2f/0x50
[18839.553002]  ? __pfx_kthread+0x10/0x10
[18839.553968]  ret_from_fork_asm+0x1a/0x30
[18839.555038]  </TASK>

Re: One-off rcu_nocb_rdp_deoffload bug

[Z qiang]

>
> Hello, Frederic,
>
> I have seen this once on Neeraj's tree with a few commits on top (-rcu
> commit 46774278c74f ("rcutorture: Test start-poll primitives with
> interrupts disabled").  But only the once so far.
>
> This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
> rcu_nocb_rdp_deoffload().
>
> Thoughts?
>

The rcu_segcblist_extract_done_cbs() doesn't reduce the count of rsclp->len,
in rcu_do_bacth().  this may cause that after we execute the barrier rcu
callback, before decrementing the count of rsclp->len,  the rcu_barrier()
returns and makes a judgment of rcu_segcblist_n_cbs(&rdp->cblist)  in
rcu_nocb_rdp_deoffload().

maybe can use WARN_ON_ONCE(rcu_segcblist_n_segment_cbs()) instead
of WARN_ON_ONCE(rcu_segcblist_n_cbs())

Thoughts?

Thanks
Zqiang




>                                                 Thanx, Paul
>
> ------------------------------------------------------------------------
>
> [18839.499664] ------------[ cut here ]------------
> [18839.500993] WARNING: CPU: 19 PID: 100 at kernel/rcu/tree_nocb.h:1061 rcu_nocb_rdp_deoffload+0x292/0x2a0
> [18839.503545] Modules linked in:
> [18839.504443] CPU: 19 UID: 0 PID: 100 Comm: rcu_nocb_toggle Not tainted 6.11.0-rc3-00459-gba3eb80edec7-dirty #1553
> [18839.507190] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
> [18839.510002] RIP: 0010:rcu_nocb_rdp_deoffload+0x292/0x2a0
> [18839.511451] Code: e9 3c ff ff ff 4c 89 e6 48 89 ef e8 98 0d e1 00 e9 3b fe ff ff 90 0f 0b 90 48 8b 83 c0 00 00 00 48 85 c0 0f 84 0b fe ff ff 90 <0f> 0b 90 e9 02 fe ff ff e8 e1 60 e0 00 90 90 90 90 90 90 90 90 90
> [18839.516469] RSP: 0000:ffffa05a8049be30 EFLAGS: 00010006
> [18839.517908] RAX: 0000000000000081 RBX: ffff956c5eaaf480 RCX: 0000000000000000
> [18839.519849] RDX: 0000000000000001 RSI: 0000000000000287 RDI: ffff956c5eaaf5f0
> [18839.521804] RBP: ffff956c5eaaf5f0 R08: 0000112269aee7fd R09: 0000000000000001
> [18839.523747] R10: 00000000000020e8 R11: 0000000000000000 R12: 0000000000000292
> [18839.525623] R13: 0000000000000000 R14: ffff956c5ea2f480 R15: 0000000000000001
> [18839.527557] FS:  0000000000000000(0000) GS:ffff956c5eec0000(0000) knlGS:0000000000000000
> [18839.529917] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [18839.531556] CR2: 0000000000000000 CR3: 0000000001cfa000 CR4: 00000000000006f0
> [18839.533495] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [18839.535395] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [18839.537322] Call Trace:
> [18839.538006]  <TASK>
> [18839.538596]  ? __warn+0x7e/0x120
> [18839.539491]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
> [18839.540757]  ? report_bug+0x18e/0x1a0
> [18839.541805]  ? handle_bug+0x3d/0x70
> [18839.542837]  ? exc_invalid_op+0x18/0x70
> [18839.543959]  ? asm_exc_invalid_op+0x1a/0x20
> [18839.545165]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
> [18839.546547]  rcu_nocb_cpu_deoffload+0x70/0xa0
> [18839.547814]  rcu_nocb_toggle+0x136/0x1c0
> [18839.548960]  ? __pfx_rcu_nocb_toggle+0x10/0x10
> [18839.550073]  kthread+0xd1/0x100
> [18839.550958]  ? __pfx_kthread+0x10/0x10
> [18839.552008]  ret_from_fork+0x2f/0x50
> [18839.553002]  ? __pfx_kthread+0x10/0x10
> [18839.553968]  ret_from_fork_asm+0x1a/0x30
> [18839.555038]  </TASK>
>
>

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Tue, Aug 20, 2024 at 04:43:39PM +0200, Frederic Weisbecker wrote:
> Sorry for the html mail, I only have my phone ...
> 
> Le mar. 20 août 2024, 13:07, Z qiang <qiang.zhang1211@gmail.com> a écrit :
> 
> > >
> > > Hello, Frederic,
> > >
> > > I have seen this once on Neeraj's tree with a few commits on top (-rcu
> > > commit 46774278c74f ("rcutorture: Test start-poll primitives with
> > > interrupts disabled").  But only the once so far.
> > >
> > > This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
> > > rcu_nocb_rdp_deoffload().
> > >
> > > Thoughts?
> > >
> >
> > The rcu_segcblist_extract_done_cbs() doesn't reduce the count of
> > rsclp->len,
> > in rcu_do_bacth().  this may cause that after we execute the barrier rcu
> > callback, before decrementing the count of rsclp->len,  the rcu_barrier()
> > returns and makes a judgment of rcu_segcblist_n_cbs(&rdp->cblist)  in
> > rcu_nocb_rdp_deoffload().
> >
> 
> That sounds plausible! You just unlocked my thoughts running in circle
> since yesterday.
> 
> 
> > maybe can use WARN_ON_ONCE(rcu_segcblist_n_segment_cbs()) instead
> > of WARN_ON_ONCE(rcu_segcblist_n_cbs())
> >
> > Thoughts?
> 
> I'll test that once I'm back from vacation the september 2nd. Thanks!

Thank you both!!!

Running -next over last night hit a number of boot-time splats, so I
have no idea if this reproduces nicely.  Can't have everything!  ;-)

							Thanx, Paul

> > Thanks
> > Zqiang
> >
> >
> >
> >
> > >                                                 Thanx, Paul
> > >
> > > ------------------------------------------------------------------------
> > >
> > > [18839.499664] ------------[ cut here ]------------
> > > [18839.500993] WARNING: CPU: 19 PID: 100 at kernel/rcu/tree_nocb.h:1061
> > rcu_nocb_rdp_deoffload+0x292/0x2a0
> > > [18839.503545] Modules linked in:
> > > [18839.504443] CPU: 19 UID: 0 PID: 100 Comm: rcu_nocb_toggle Not tainted
> > 6.11.0-rc3-00459-gba3eb80edec7-dirty #1553
> > > [18839.507190] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
> > rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
> > > [18839.510002] RIP: 0010:rcu_nocb_rdp_deoffload+0x292/0x2a0
> > > [18839.511451] Code: e9 3c ff ff ff 4c 89 e6 48 89 ef e8 98 0d e1 00 e9
> > 3b fe ff ff 90 0f 0b 90 48 8b 83 c0 00 00 00 48 85 c0 0f 84 0b fe ff ff 90
> > <0f> 0b 90 e9 02 fe ff ff e8 e1 60 e0 00 90 90 90 90 90 90 90 90 90
> > > [18839.516469] RSP: 0000:ffffa05a8049be30 EFLAGS: 00010006
> > > [18839.517908] RAX: 0000000000000081 RBX: ffff956c5eaaf480 RCX:
> > 0000000000000000
> > > [18839.519849] RDX: 0000000000000001 RSI: 0000000000000287 RDI:
> > ffff956c5eaaf5f0
> > > [18839.521804] RBP: ffff956c5eaaf5f0 R08: 0000112269aee7fd R09:
> > 0000000000000001
> > > [18839.523747] R10: 00000000000020e8 R11: 0000000000000000 R12:
> > 0000000000000292
> > > [18839.525623] R13: 0000000000000000 R14: ffff956c5ea2f480 R15:
> > 0000000000000001
> > > [18839.527557] FS:  0000000000000000(0000) GS:ffff956c5eec0000(0000)
> > knlGS:0000000000000000
> > > [18839.529917] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > [18839.531556] CR2: 0000000000000000 CR3: 0000000001cfa000 CR4:
> > 00000000000006f0
> > > [18839.533495] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> > 0000000000000000
> > > [18839.535395] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> > 0000000000000400
> > > [18839.537322] Call Trace:
> > > [18839.538006]  <TASK>
> > > [18839.538596]  ? __warn+0x7e/0x120
> > > [18839.539491]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
> > > [18839.540757]  ? report_bug+0x18e/0x1a0
> > > [18839.541805]  ? handle_bug+0x3d/0x70
> > > [18839.542837]  ? exc_invalid_op+0x18/0x70
> > > [18839.543959]  ? asm_exc_invalid_op+0x1a/0x20
> > > [18839.545165]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
> > > [18839.546547]  rcu_nocb_cpu_deoffload+0x70/0xa0
> > > [18839.547814]  rcu_nocb_toggle+0x136/0x1c0
> > > [18839.548960]  ? __pfx_rcu_nocb_toggle+0x10/0x10
> > > [18839.550073]  kthread+0xd1/0x100
> > > [18839.550958]  ? __pfx_kthread+0x10/0x10
> > > [18839.552008]  ret_from_fork+0x2f/0x50
> > > [18839.553002]  ? __pfx_kthread+0x10/0x10
> > > [18839.553968]  ret_from_fork_asm+0x1a/0x30
> > > [18839.555038]  </TASK>
> > >
> > >
> >
> >

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Tue, Aug 20, 2024 at 10:46:43AM -0700, Paul E. McKenney wrote:
> On Tue, Aug 20, 2024 at 04:43:39PM +0200, Frederic Weisbecker wrote:
> > Sorry for the html mail, I only have my phone ...
> > 
> > Le mar. 20 août 2024, 13:07, Z qiang <qiang.zhang1211@gmail.com> a écrit :
> > 
> > > >
> > > > Hello, Frederic,
> > > >
> > > > I have seen this once on Neeraj's tree with a few commits on top (-rcu
> > > > commit 46774278c74f ("rcutorture: Test start-poll primitives with
> > > > interrupts disabled").  But only the once so far.
> > > >
> > > > This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
> > > > rcu_nocb_rdp_deoffload().
> > > >
> > > > Thoughts?
> > > >
> > >
> > > The rcu_segcblist_extract_done_cbs() doesn't reduce the count of
> > > rsclp->len,
> > > in rcu_do_bacth().  this may cause that after we execute the barrier rcu
> > > callback, before decrementing the count of rsclp->len,  the rcu_barrier()
> > > returns and makes a judgment of rcu_segcblist_n_cbs(&rdp->cblist)  in
> > > rcu_nocb_rdp_deoffload().
> > >
> > 
> > That sounds plausible! You just unlocked my thoughts running in circle
> > since yesterday.
> > 
> > 
> > > maybe can use WARN_ON_ONCE(rcu_segcblist_n_segment_cbs()) instead
> > > of WARN_ON_ONCE(rcu_segcblist_n_cbs())
> > >
> > > Thoughts?
> > 
> > I'll test that once I'm back from vacation the september 2nd. Thanks!
> 
> Thank you both!!!
> 
> Running -next over last night hit a number of boot-time splats, so I
> have no idea if this reproduces nicely.  Can't have everything!  ;-)

And it is now a two-off given another one last night's testing.  This was
from 168 hours of TREE01 on my -rcu "dev" branch (as opposed to -next),
but I have run many runs over the past two weeks.  So it is reproducible,
but rare.

Ah, and if it matters, I synched up to Neeraj's latest as of about 18
hours ago just before starting this test.


							Thanx, Paul

------------------------------------------------------------------------

[13375.559536] ------------[ cut here ]------------
[13375.560748] WARNING: CPU: 27 PID: 103 at kernel/rcu/tree_nocb.h:1061 rcu_nocb_rdp_deoffload+0x292/0x2a0
[13375.563088] Modules linked in:
[13375.563861] CPU: 27 UID: 0 PID: 103 Comm: rcu_nocb_toggle Not tainted 6.11.0-rc1-00141-gc5fc1889f28b #1923
[13375.566261] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[13375.569047] RIP: 0010:rcu_nocb_rdp_deoffload+0x292/0x2a0
[13375.570373] Code: e9 3c ff ff ff 4c 89 e6 48 89 ef e8 88 fd e0 00 e9 3b fe ff ff 90 0f 0b 90 48 8b 83 c0 00 00 00 48 85 c0 0f 84 0b fe ff ff 90 <0f> 0b 90 e9 02 fe ff ff e8 d1 4f e0 00 90 90 90 90 90 90 90 90 90
[13375.574999] RSP: 0018:ffffafa9c04b3e30 EFLAGS: 00010002
[13375.576302] RAX: 000000000000007c RBX: ffff8f4a1eb6f480 RCX: 000000000000002b
[13375.578069] RDX: 0000000000000001 RSI: 000000000000002b RDI: ffff8f4a1eb6f5f0
[13375.579837] RBP: ffff8f4a1eb6f5f0 R08: 000000000000002a R09: 0000000000000001
[13375.581608] R10: ffffffff8d99b408 R11: 00000000001d1c76 R12: 0000000000000246
[13375.583376] R13: 0000000000000000 R14: ffff8f4a1ea2f480 R15: 0000000000000001
[13375.585149] FS:  0000000000000000(0000) GS:ffff8f4a1f0c0000(0000) knlGS:0000000000000000
[13375.587158] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[13375.588598] CR2: 0000000000000000 CR3: 0000000002e0c000 CR4: 00000000000006f0
[13375.590360] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[13375.592163] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[13375.593943] Call Trace:
[13375.594572]  <TASK>
[13375.595102]  ? __warn+0x7e/0x120
[13375.595917]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
[13375.597091]  ? report_bug+0x18e/0x1a0
[13375.598013]  ? handle_bug+0x3d/0x70
[13375.598892]  ? exc_invalid_op+0x18/0x70
[13375.599935]  ? asm_exc_invalid_op+0x1a/0x20
[13375.600987]  ? rcu_nocb_rdp_deoffload+0x292/0x2a0
[13375.602163]  rcu_nocb_cpu_deoffload+0x70/0xa0
[13375.603262]  rcu_nocb_toggle+0x136/0x1c0
[13375.604250]  ? __pfx_rcu_nocb_toggle+0x10/0x10
[13375.605361]  kthread+0xd1/0x100
[13375.606159]  ? __pfx_kthread+0x10/0x10
[13375.607102]  ret_from_fork+0x2f/0x50
[13375.608001]  ? __pfx_kthread+0x10/0x10
[13375.608941]  ret_from_fork_asm+0x1a/0x30
[13375.609928]  </TASK>
[13375.610486] ---[ end trace 0000000000000000 ]---

Re: One-off rcu_nocb_rdp_deoffload bug

[Frederic Weisbecker]

Le Wed, Sep 04, 2024 at 05:59:46AM -0700, Paul E. McKenney a écrit :
> On Tue, Aug 20, 2024 at 10:46:43AM -0700, Paul E. McKenney wrote:
> > On Tue, Aug 20, 2024 at 04:43:39PM +0200, Frederic Weisbecker wrote:
> > > Sorry for the html mail, I only have my phone ...
> > > 
> > > Le mar. 20 août 2024, 13:07, Z qiang <qiang.zhang1211@gmail.com> a écrit :
> > > 
> > > > >
> > > > > Hello, Frederic,
> > > > >
> > > > > I have seen this once on Neeraj's tree with a few commits on top (-rcu
> > > > > commit 46774278c74f ("rcutorture: Test start-poll primitives with
> > > > > interrupts disabled").  But only the once so far.
> > > > >
> > > > > This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
> > > > > rcu_nocb_rdp_deoffload().
> > > > >
> > > > > Thoughts?
> > > > >
> > > >
> > > > The rcu_segcblist_extract_done_cbs() doesn't reduce the count of
> > > > rsclp->len,
> > > > in rcu_do_bacth().  this may cause that after we execute the barrier rcu
> > > > callback, before decrementing the count of rsclp->len,  the rcu_barrier()
> > > > returns and makes a judgment of rcu_segcblist_n_cbs(&rdp->cblist)  in
> > > > rcu_nocb_rdp_deoffload().
> > > >
> > > 
> > > That sounds plausible! You just unlocked my thoughts running in circle
> > > since yesterday.
> > > 
> > > 
> > > > maybe can use WARN_ON_ONCE(rcu_segcblist_n_segment_cbs()) instead
> > > > of WARN_ON_ONCE(rcu_segcblist_n_cbs())
> > > >
> > > > Thoughts?
> > > 
> > > I'll test that once I'm back from vacation the september 2nd. Thanks!
> > 
> > Thank you both!!!
> > 
> > Running -next over last night hit a number of boot-time splats, so I
> > have no idea if this reproduces nicely.  Can't have everything!  ;-)
> 
> And it is now a two-off given another one last night's testing.  This was
> from 168 hours of TREE01 on my -rcu "dev" branch (as opposed to -next),
> but I have run many runs over the past two weeks.  So it is reproducible,
> but rare.
> 
> Ah, and if it matters, I synched up to Neeraj's latest as of about 18
> hours ago just before starting this test.

Yes, I'm preparing an update for the offending patch (which has one more
embarassing issue while I'm going through it again).

Thanks.

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Wed, Sep 04, 2024 at 03:48:02PM +0200, Frederic Weisbecker wrote:
> Le Wed, Sep 04, 2024 at 05:59:46AM -0700, Paul E. McKenney a écrit :
> > On Tue, Aug 20, 2024 at 10:46:43AM -0700, Paul E. McKenney wrote:
> > > On Tue, Aug 20, 2024 at 04:43:39PM +0200, Frederic Weisbecker wrote:
> > > > Sorry for the html mail, I only have my phone ...
> > > > 
> > > > Le mar. 20 août 2024, 13:07, Z qiang <qiang.zhang1211@gmail.com> a écrit :
> > > > 
> > > > > >
> > > > > > Hello, Frederic,
> > > > > >
> > > > > > I have seen this once on Neeraj's tree with a few commits on top (-rcu
> > > > > > commit 46774278c74f ("rcutorture: Test start-poll primitives with
> > > > > > interrupts disabled").  But only the once so far.
> > > > > >
> > > > > > This is the WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) in
> > > > > > rcu_nocb_rdp_deoffload().
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > >
> > > > > The rcu_segcblist_extract_done_cbs() doesn't reduce the count of
> > > > > rsclp->len,
> > > > > in rcu_do_bacth().  this may cause that after we execute the barrier rcu
> > > > > callback, before decrementing the count of rsclp->len,  the rcu_barrier()
> > > > > returns and makes a judgment of rcu_segcblist_n_cbs(&rdp->cblist)  in
> > > > > rcu_nocb_rdp_deoffload().
> > > > >
> > > > 
> > > > That sounds plausible! You just unlocked my thoughts running in circle
> > > > since yesterday.
> > > > 
> > > > 
> > > > > maybe can use WARN_ON_ONCE(rcu_segcblist_n_segment_cbs()) instead
> > > > > of WARN_ON_ONCE(rcu_segcblist_n_cbs())
> > > > >
> > > > > Thoughts?
> > > > 
> > > > I'll test that once I'm back from vacation the september 2nd. Thanks!
> > > 
> > > Thank you both!!!
> > > 
> > > Running -next over last night hit a number of boot-time splats, so I
> > > have no idea if this reproduces nicely.  Can't have everything!  ;-)
> > 
> > And it is now a two-off given another one last night's testing.  This was
> > from 168 hours of TREE01 on my -rcu "dev" branch (as opposed to -next),
> > but I have run many runs over the past two weeks.  So it is reproducible,
> > but rare.
> > 
> > Ah, and if it matters, I synched up to Neeraj's latest as of about 18
> > hours ago just before starting this test.
> 
> Yes, I'm preparing an update for the offending patch (which has one more
> embarassing issue while I'm going through it again).

Very good, thank you!

							Thanx, Paul

Re: One-off rcu_nocb_rdp_deoffload bug

[Frederic Weisbecker]

Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > Yes, I'm preparing an update for the offending patch (which has one more
> > embarassing issue while I'm going through it again).
> 
> Very good, thank you!

So my proposal for a replacement patch is this (to replace the patch
of the same name in Neeraj tree):

---
From a5865b1c7ccc4efd62c44591d2be0c60e58b0ecf Mon Sep 17 00:00:00 2001
From: Frederic Weisbecker <frederic@kernel.org>
Date: Thu, 4 Jul 2024 00:56:40 +0200
Subject: [PATCH] rcu/nocb: Simplify (de-)offloading state machine

Now that the (de-)offloading process can only apply to offline CPUs,
there is no more concurrency between rcu_core and nocb kthreads. Also
the mutation now happens on empty queues.

Therefore the state machine can be reduced to a single bit called
SEGCBLIST_OFFLOADED. Simplify the transition as follows:

* Upon offloading: queue the rdp to be added to the rcuog list and
  wait for the rcuog kthread to set the SEGCBLIST_OFFLOADED bit. Unpark
  rcuo kthread.

* Upon de-offloading: Park rcuo kthread. Queue the rdp to be removed
  from the rcuog list and wait for the rcuog kthread to clear the
  SEGCBLIST_OFFLOADED bit.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Neeraj Upadhyay <neeraj.upadhyay@kernel.org>
---
 include/linux/rcu_segcblist.h |   4 +-
 kernel/rcu/rcu_segcblist.c    |  11 ---
 kernel/rcu/rcu_segcblist.h    |   2 +-
 kernel/rcu/tree_nocb.h        | 138 ++++++++++++++++------------------
 4 files changed, 68 insertions(+), 87 deletions(-)

diff --git a/include/linux/rcu_segcblist.h b/include/linux/rcu_segcblist.h
index 1ef1bb54853d..2fdc2208f1ca 100644
--- a/include/linux/rcu_segcblist.h
+++ b/include/linux/rcu_segcblist.h
@@ -185,9 +185,7 @@ struct rcu_cblist {
  *  ----------------------------------------------------------------------------
  */
 #define SEGCBLIST_ENABLED	BIT(0)
-#define SEGCBLIST_LOCKING	BIT(1)
-#define SEGCBLIST_KTHREAD_GP	BIT(2)
-#define SEGCBLIST_OFFLOADED	BIT(3)
+#define SEGCBLIST_OFFLOADED	BIT(1)
 
 struct rcu_segcblist {
 	struct rcu_head *head;
diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
index 1693ea22ef1b..298a2c573f02 100644
--- a/kernel/rcu/rcu_segcblist.c
+++ b/kernel/rcu/rcu_segcblist.c
@@ -260,17 +260,6 @@ void rcu_segcblist_disable(struct rcu_segcblist *rsclp)
 	rcu_segcblist_clear_flags(rsclp, SEGCBLIST_ENABLED);
 }
 
-/*
- * Mark the specified rcu_segcblist structure as offloaded (or not)
- */
-void rcu_segcblist_offload(struct rcu_segcblist *rsclp, bool offload)
-{
-	if (offload)
-		rcu_segcblist_set_flags(rsclp, SEGCBLIST_LOCKING | SEGCBLIST_OFFLOADED);
-	else
-		rcu_segcblist_clear_flags(rsclp, SEGCBLIST_OFFLOADED);
-}
-
 /*
  * Does the specified rcu_segcblist structure contain callbacks that
  * are ready to be invoked?
diff --git a/kernel/rcu/rcu_segcblist.h b/kernel/rcu/rcu_segcblist.h
index 7a0962dfee86..259904075636 100644
--- a/kernel/rcu/rcu_segcblist.h
+++ b/kernel/rcu/rcu_segcblist.h
@@ -89,7 +89,7 @@ static inline bool rcu_segcblist_is_enabled(struct rcu_segcblist *rsclp)
 static inline bool rcu_segcblist_is_offloaded(struct rcu_segcblist *rsclp)
 {
 	if (IS_ENABLED(CONFIG_RCU_NOCB_CPU) &&
-	    rcu_segcblist_test_flags(rsclp, SEGCBLIST_LOCKING))
+	    rcu_segcblist_test_flags(rsclp, SEGCBLIST_OFFLOADED))
 		return true;
 
 	return false;
diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 6dac4f266c41..d7dc8ffa6ba8 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -604,37 +604,33 @@ static void call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *head,
 	}
 }
 
-static int nocb_gp_toggle_rdp(struct rcu_data *rdp)
+static void nocb_gp_toggle_rdp(struct rcu_data *rdp_gp, struct rcu_data *rdp)
 {
 	struct rcu_segcblist *cblist = &rdp->cblist;
 	unsigned long flags;
-	int ret;
 
-	rcu_nocb_lock_irqsave(rdp, flags);
-	if (rcu_segcblist_test_flags(cblist, SEGCBLIST_OFFLOADED) &&
-	    !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_GP)) {
+	/*
+	 * Locking orders future de-offloaded callbacks enqueue against previous
+	 * handling of this rdp. Ie: Make sure rcuog is done with this rdp before
+	 * deoffloaded callbacks can be enqueued.
+	 */
+	raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
+	if (!rcu_segcblist_test_flags(cblist, SEGCBLIST_OFFLOADED)) {
 		/*
 		 * Offloading. Set our flag and notify the offload worker.
 		 * We will handle this rdp until it ever gets de-offloaded.
 		 */
-		rcu_segcblist_set_flags(cblist, SEGCBLIST_KTHREAD_GP);
-		ret = 1;
-	} else if (!rcu_segcblist_test_flags(cblist, SEGCBLIST_OFFLOADED) &&
-		   rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_GP)) {
+		list_add_tail(&rdp->nocb_entry_rdp, &rdp_gp->nocb_head_rdp);
+		rcu_segcblist_set_flags(cblist, SEGCBLIST_OFFLOADED);
+	} else {
 		/*
 		 * De-offloading. Clear our flag and notify the de-offload worker.
 		 * We will ignore this rdp until it ever gets re-offloaded.
 		 */
-		rcu_segcblist_clear_flags(cblist, SEGCBLIST_KTHREAD_GP);
-		ret = 0;
-	} else {
-		WARN_ON_ONCE(1);
-		ret = -1;
+		list_del(&rdp->nocb_entry_rdp);
+		rcu_segcblist_clear_flags(cblist, SEGCBLIST_OFFLOADED);
 	}
-
-	rcu_nocb_unlock_irqrestore(rdp, flags);
-
-	return ret;
+	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 }
 
 static void nocb_gp_sleep(struct rcu_data *my_rdp, int cpu)
@@ -841,14 +837,7 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
 	}
 
 	if (rdp_toggling) {
-		int ret;
-
-		ret = nocb_gp_toggle_rdp(rdp_toggling);
-		if (ret == 1)
-			list_add_tail(&rdp_toggling->nocb_entry_rdp, &my_rdp->nocb_head_rdp);
-		else if (ret == 0)
-			list_del(&rdp_toggling->nocb_entry_rdp);
-
+		nocb_gp_toggle_rdp(my_rdp, rdp_toggling);
 		swake_up_one(&rdp_toggling->nocb_state_wq);
 	}
 
@@ -1018,16 +1007,11 @@ void rcu_nocb_flush_deferred_wakeup(void)
 }
 EXPORT_SYMBOL_GPL(rcu_nocb_flush_deferred_wakeup);
 
-static int rdp_offload_toggle(struct rcu_data *rdp,
-			       bool offload, unsigned long flags)
-	__releases(rdp->nocb_lock)
+static int rcu_nocb_queue_toggle_rdp(struct rcu_data *rdp)
 {
-	struct rcu_segcblist *cblist = &rdp->cblist;
 	struct rcu_data *rdp_gp = rdp->nocb_gp_rdp;
 	bool wake_gp = false;
-
-	rcu_segcblist_offload(cblist, offload);
-	rcu_nocb_unlock_irqrestore(rdp, flags);
+	unsigned long flags;
 
 	raw_spin_lock_irqsave(&rdp_gp->nocb_gp_lock, flags);
 	// Queue this rdp for add/del to/from the list to iterate on rcuog
@@ -1041,9 +1025,25 @@ static int rdp_offload_toggle(struct rcu_data *rdp,
 	return wake_gp;
 }
 
+static bool rcu_nocb_rdp_deoffload_wait_cond(struct rcu_data *rdp)
+{
+	unsigned long flags;
+	bool ret;
+
+	/*
+	 * Locking makes sure rcuog is done handling this rdp before deoffloaded
+	 * enqueue can happen. Also it keeps the SEGCBLIST_OFFLOADED flag stable
+	 * while the ->nocb_lock is held.
+	 */
+	raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
+	ret = !rcu_segcblist_test_flags(&rdp->cblist, SEGCBLIST_OFFLOADED);
+	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
+
+	return ret;
+}
+
 static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
 {
-	struct rcu_segcblist *cblist = &rdp->cblist;
 	unsigned long flags;
 	int wake_gp;
 	struct rcu_data *rdp_gp = rdp->nocb_gp_rdp;
@@ -1056,51 +1056,42 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
 	/* Flush all callbacks from segcblist and bypass */
 	rcu_barrier();
 
+	/*
+	 * Make sure the rcuoc kthread isn't in the middle of a nocb locked
+	 * sequence while offloading is deactivated, along with nocb locking.
+	 */
+	if (rdp->nocb_cb_kthread)
+		kthread_park(rdp->nocb_cb_kthread);
+
 	rcu_nocb_lock_irqsave(rdp, flags);
 	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
 	WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
+	rcu_nocb_unlock_irqrestore(rdp, flags);
 
-	wake_gp = rdp_offload_toggle(rdp, false, flags);
+	wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
 
 	mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
+
 	if (rdp_gp->nocb_gp_kthread) {
 		if (wake_gp)
 			wake_up_process(rdp_gp->nocb_gp_kthread);
 
 		swait_event_exclusive(rdp->nocb_state_wq,
-				      !rcu_segcblist_test_flags(cblist,
-								SEGCBLIST_KTHREAD_GP));
-		if (rdp->nocb_cb_kthread)
-			kthread_park(rdp->nocb_cb_kthread);
+				      rcu_nocb_rdp_deoffload_wait_cond(rdp));
 	} else {
 		/*
 		 * No kthread to clear the flags for us or remove the rdp from the nocb list
 		 * to iterate. Do it here instead. Locking doesn't look stricly necessary
 		 * but we stick to paranoia in this rare path.
 		 */
-		rcu_nocb_lock_irqsave(rdp, flags);
-		rcu_segcblist_clear_flags(&rdp->cblist, SEGCBLIST_KTHREAD_GP);
-		rcu_nocb_unlock_irqrestore(rdp, flags);
+		raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
+		rcu_segcblist_clear_flags(&rdp->cblist, SEGCBLIST_OFFLOADED);
+		raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 
 		list_del(&rdp->nocb_entry_rdp);
 	}
-	mutex_unlock(&rdp_gp->nocb_gp_kthread_mutex);
 
-	/*
-	 * Lock one last time to acquire latest callback updates from kthreads
-	 * so we can later handle callbacks locally without locking.
-	 */
-	rcu_nocb_lock_irqsave(rdp, flags);
-	/*
-	 * Theoretically we could clear SEGCBLIST_LOCKING after the nocb
-	 * lock is released but how about being paranoid for once?
-	 */
-	rcu_segcblist_clear_flags(cblist, SEGCBLIST_LOCKING);
-	/*
-	 * Without SEGCBLIST_LOCKING, we can't use
-	 * rcu_nocb_unlock_irqrestore() anymore.
-	 */
-	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
+	mutex_unlock(&rdp_gp->nocb_gp_kthread_mutex);
 
 	return 0;
 }
@@ -1129,10 +1120,20 @@ int rcu_nocb_cpu_deoffload(int cpu)
 }
 EXPORT_SYMBOL_GPL(rcu_nocb_cpu_deoffload);
 
-static int rcu_nocb_rdp_offload(struct rcu_data *rdp)
+static bool rcu_nocb_rdp_offload_wait_cond(struct rcu_data *rdp)
 {
-	struct rcu_segcblist *cblist = &rdp->cblist;
 	unsigned long flags;
+	bool ret;
+
+	raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
+	ret = rcu_segcblist_test_flags(&rdp->cblist, SEGCBLIST_OFFLOADED);
+	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
+
+	return ret;
+}
+
+static int rcu_nocb_rdp_offload(struct rcu_data *rdp)
+{
 	int wake_gp;
 	struct rcu_data *rdp_gp = rdp->nocb_gp_rdp;
 
@@ -1152,20 +1153,14 @@ static int rcu_nocb_rdp_offload(struct rcu_data *rdp)
 	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
 	WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
 
-	/*
-	 * Can't use rcu_nocb_lock_irqsave() before SEGCBLIST_LOCKING
-	 * is set.
-	 */
-	raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
-
-	wake_gp = rdp_offload_toggle(rdp, true, flags);
+	wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
 	if (wake_gp)
 		wake_up_process(rdp_gp->nocb_gp_kthread);
 
-	kthread_unpark(rdp->nocb_cb_kthread);
-
 	swait_event_exclusive(rdp->nocb_state_wq,
-			      rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_GP));
+			      rcu_nocb_rdp_offload_wait_cond(rdp));
+
+	kthread_unpark(rdp->nocb_cb_kthread);
 
 	return 0;
 }
@@ -1340,8 +1335,7 @@ void __init rcu_init_nohz(void)
 		rdp = per_cpu_ptr(&rcu_data, cpu);
 		if (rcu_segcblist_empty(&rdp->cblist))
 			rcu_segcblist_init(&rdp->cblist);
-		rcu_segcblist_offload(&rdp->cblist, true);
-		rcu_segcblist_set_flags(&rdp->cblist, SEGCBLIST_KTHREAD_GP);
+		rcu_segcblist_set_flags(&rdp->cblist, SEGCBLIST_OFFLOADED);
 	}
 	rcu_organize_nocb_kthreads();
 }
-- 
2.46.0

Re: One-off rcu_nocb_rdp_deoffload bug

[Frederic Weisbecker]

Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > Yes, I'm preparing an update for the offending patch (which has one more
> > > embarassing issue while I'm going through it again).
> > 
> > Very good, thank you!
> 
> So my proposal for a replacement patch is this (to replace the patch
> of the same name in Neeraj tree):

FYI, the diffstat against the previous version of the same patch is as follows.
The rationale being:

1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex

2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
   after the barrier's completion.

3) This fixes another issue: rcuoc must be parked _before_
   rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
   within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
   leaving the nocb locked forever.


diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 755ada098035..97b99cd06923 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1056,6 +1056,13 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
 	/* Flush all callbacks from segcblist and bypass */
 	rcu_barrier();
 
+	/*
+	 * Make sure the rcuoc kthread isn't in the middle of a nocb locked
+	 * sequence while offloading is deactivated, along with nocb locking.
+	 */
+	if (rdp->nocb_cb_kthread)
+		kthread_park(rdp->nocb_cb_kthread);
+
 	rcu_nocb_lock_irqsave(rdp, flags);
 	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
 	WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
@@ -1064,13 +1071,11 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
 	wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
 
 	mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
+
 	if (rdp_gp->nocb_gp_kthread) {
 		if (wake_gp)
 			wake_up_process(rdp_gp->nocb_gp_kthread);
 
-		if (rdp->nocb_cb_kthread)
-			kthread_park(rdp->nocb_cb_kthread);
-
 		swait_event_exclusive(rdp->nocb_state_wq,
 				      rcu_nocb_rdp_deoffload_wait_cond(rdp));
 	} else {

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > embarassing issue while I'm going through it again).
> > > 
> > > Very good, thank you!
> > 
> > So my proposal for a replacement patch is this (to replace the patch
> > of the same name in Neeraj tree):
> 
> FYI, the diffstat against the previous version of the same patch is as follows.
> The rationale being:
> 
> 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> 
> 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
>    after the barrier's completion.
> 
> 3) This fixes another issue: rcuoc must be parked _before_
>    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
>    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
>    leaving the nocb locked forever.

Thank you!!!

Just to make sure that I understand, I apply this patch on top of
Neeraj's current set of branches to get the fix, correct?

							Thanx, Paul

> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 755ada098035..97b99cd06923 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1056,6 +1056,13 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
>  	/* Flush all callbacks from segcblist and bypass */
>  	rcu_barrier();
>  
> +	/*
> +	 * Make sure the rcuoc kthread isn't in the middle of a nocb locked
> +	 * sequence while offloading is deactivated, along with nocb locking.
> +	 */
> +	if (rdp->nocb_cb_kthread)
> +		kthread_park(rdp->nocb_cb_kthread);
> +
>  	rcu_nocb_lock_irqsave(rdp, flags);
>  	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
>  	WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
> @@ -1064,13 +1071,11 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
>  	wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
>  
>  	mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
> +
>  	if (rdp_gp->nocb_gp_kthread) {
>  		if (wake_gp)
>  			wake_up_process(rdp_gp->nocb_gp_kthread);
>  
> -		if (rdp->nocb_cb_kthread)
> -			kthread_park(rdp->nocb_cb_kthread);
> -
>  		swait_event_exclusive(rdp->nocb_state_wq,
>  				      rcu_nocb_rdp_deoffload_wait_cond(rdp));
>  	} else {

Re: One-off rcu_nocb_rdp_deoffload bug

[Neeraj upadhyay]

On Fri, Sep 6, 2024 at 11:34 AM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > embarassing issue while I'm going through it again).
> > > >
> > > > Very good, thank you!
> > >
> > > So my proposal for a replacement patch is this (to replace the patch
> > > of the same name in Neeraj tree):
> >
> > FYI, the diffstat against the previous version of the same patch is as follows.
> > The rationale being:
> >
> > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> >
> > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> >    after the barrier's completion.
> >
> > 3) This fixes another issue: rcuoc must be parked _before_
> >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> >    leaving the nocb locked forever.
>
> Thank you!!!
>
> Just to make sure that I understand, I apply this patch on top of
> Neeraj's current set of branches to get the fix, correct?
>

I have pushed this diff to branch next.06.09.24a of shared-rcu tree
and started testing.
Will squash this diff to the original commit later.


[1] https://git.kernel.org/pub/scm/linux/kernel/git/rcu/linux.git/commit/?h=next.06.09.24a&id=0fc7fc28b5afc3037ae4e1464013bc38c4b51c99


- Neeraj

>                                                         Thanx, Paul
>
> > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> > index 755ada098035..97b99cd06923 100644
> > --- a/kernel/rcu/tree_nocb.h
> > +++ b/kernel/rcu/tree_nocb.h
> > @@ -1056,6 +1056,13 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
> >       /* Flush all callbacks from segcblist and bypass */
> >       rcu_barrier();
> >
> > +     /*
> > +      * Make sure the rcuoc kthread isn't in the middle of a nocb locked
> > +      * sequence while offloading is deactivated, along with nocb locking.
> > +      */
> > +     if (rdp->nocb_cb_kthread)
> > +             kthread_park(rdp->nocb_cb_kthread);
> > +
> >       rcu_nocb_lock_irqsave(rdp, flags);
> >       WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
> >       WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
> > @@ -1064,13 +1071,11 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
> >       wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
> >
> >       mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
> > +
> >       if (rdp_gp->nocb_gp_kthread) {
> >               if (wake_gp)
> >                       wake_up_process(rdp_gp->nocb_gp_kthread);
> >
> > -             if (rdp->nocb_cb_kthread)
> > -                     kthread_park(rdp->nocb_cb_kthread);
> > -
> >               swait_event_exclusive(rdp->nocb_state_wq,
> >                                     rcu_nocb_rdp_deoffload_wait_cond(rdp));
> >       } else {

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Fri, Sep 06, 2024 at 12:02:00PM +0530, Neeraj upadhyay wrote:
> On Fri, Sep 6, 2024 at 11:34 AM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > > embarassing issue while I'm going through it again).
> > > > >
> > > > > Very good, thank you!
> > > >
> > > > So my proposal for a replacement patch is this (to replace the patch
> > > > of the same name in Neeraj tree):
> > >
> > > FYI, the diffstat against the previous version of the same patch is as follows.
> > > The rationale being:
> > >
> > > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > >
> > > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> > >    after the barrier's completion.
> > >
> > > 3) This fixes another issue: rcuoc must be parked _before_
> > >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> > >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> > >    leaving the nocb locked forever.
> >
> > Thank you!!!
> >
> > Just to make sure that I understand, I apply this patch on top of
> > Neeraj's current set of branches to get the fix, correct?
> >
> 
> I have pushed this diff to branch next.06.09.24a of shared-rcu tree
> and started testing.
> Will squash this diff to the original commit later.
> 
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/rcu/linux.git/commit/?h=next.06.09.24a&id=0fc7fc28b5afc3037ae4e1464013bc38c4b51c99

Thank you!  I fired off light testing which is unlikely to be conclusive.
But if it passes, I will do some longer and more focused tests.

							Thanx, Paul

> - Neeraj
> 
> >                                                         Thanx, Paul
> >
> > > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> > > index 755ada098035..97b99cd06923 100644
> > > --- a/kernel/rcu/tree_nocb.h
> > > +++ b/kernel/rcu/tree_nocb.h
> > > @@ -1056,6 +1056,13 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
> > >       /* Flush all callbacks from segcblist and bypass */
> > >       rcu_barrier();
> > >
> > > +     /*
> > > +      * Make sure the rcuoc kthread isn't in the middle of a nocb locked
> > > +      * sequence while offloading is deactivated, along with nocb locking.
> > > +      */
> > > +     if (rdp->nocb_cb_kthread)
> > > +             kthread_park(rdp->nocb_cb_kthread);
> > > +
> > >       rcu_nocb_lock_irqsave(rdp, flags);
> > >       WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
> > >       WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
> > > @@ -1064,13 +1071,11 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
> > >       wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
> > >
> > >       mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
> > > +
> > >       if (rdp_gp->nocb_gp_kthread) {
> > >               if (wake_gp)
> > >                       wake_up_process(rdp_gp->nocb_gp_kthread);
> > >
> > > -             if (rdp->nocb_cb_kthread)
> > > -                     kthread_park(rdp->nocb_cb_kthread);
> > > -
> > >               swait_event_exclusive(rdp->nocb_state_wq,
> > >                                     rcu_nocb_rdp_deoffload_wait_cond(rdp));
> > >       } else {

Re: One-off rcu_nocb_rdp_deoffload bug

[Frederic Weisbecker]

Le Thu, Sep 05, 2024 at 11:04:22PM -0700, Paul E. McKenney a écrit :
> On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > embarassing issue while I'm going through it again).
> > > > 
> > > > Very good, thank you!
> > > 
> > > So my proposal for a replacement patch is this (to replace the patch
> > > of the same name in Neeraj tree):
> > 
> > FYI, the diffstat against the previous version of the same patch is as follows.
> > The rationale being:
> > 
> > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > 
> > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> >    after the barrier's completion.
> > 
> > 3) This fixes another issue: rcuoc must be parked _before_
> >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> >    leaving the nocb locked forever.
> 
> Thank you!!!
> 
> Just to make sure that I understand, I apply this patch on top of
> Neeraj's current set of branches to get the fix, correct?

Exactly!

Thanks.

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Fri, Sep 06, 2024 at 03:12:11PM +0200, Frederic Weisbecker wrote:
> Le Thu, Sep 05, 2024 at 11:04:22PM -0700, Paul E. McKenney a écrit :
> > On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > > embarassing issue while I'm going through it again).
> > > > > 
> > > > > Very good, thank you!
> > > > 
> > > > So my proposal for a replacement patch is this (to replace the patch
> > > > of the same name in Neeraj tree):
> > > 
> > > FYI, the diffstat against the previous version of the same patch is as follows.
> > > The rationale being:
> > > 
> > > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > > 
> > > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> > >    after the barrier's completion.
> > > 
> > > 3) This fixes another issue: rcuoc must be parked _before_
> > >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> > >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> > >    leaving the nocb locked forever.
> > 
> > Thank you!!!
> > 
> > Just to make sure that I understand, I apply this patch on top of
> > Neeraj's current set of branches to get the fix, correct?
> 
> Exactly!

It passes the initial tests, an hour of 200*TREE01 and a 10-minute
torture.sh (which Neeraj likely already ran a longer version of).  I fired
off a 12-hour 200*TREE01 run and a 60-minute torture.sh for overnight.

Here is hoping!  ;-)

							Thanx, Paul

Re: One-off rcu_nocb_rdp_deoffload bug

[Neeraj upadhyay]

On Fri, Sep 6, 2024 at 9:58 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Fri, Sep 06, 2024 at 03:12:11PM +0200, Frederic Weisbecker wrote:
> > Le Thu, Sep 05, 2024 at 11:04:22PM -0700, Paul E. McKenney a écrit :
> > > On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > > > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > > > embarassing issue while I'm going through it again).
> > > > > >
> > > > > > Very good, thank you!
> > > > >
> > > > > So my proposal for a replacement patch is this (to replace the patch
> > > > > of the same name in Neeraj tree):
> > > >
> > > > FYI, the diffstat against the previous version of the same patch is as follows.
> > > > The rationale being:
> > > >
> > > > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > > >
> > > > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> > > >    after the barrier's completion.
> > > >
> > > > 3) This fixes another issue: rcuoc must be parked _before_
> > > >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> > > >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> > > >    leaving the nocb locked forever.
> > >
> > > Thank you!!!
> > >
> > > Just to make sure that I understand, I apply this patch on top of
> > > Neeraj's current set of branches to get the fix, correct?
> >
> > Exactly!
>
> It passes the initial tests, an hour of 200*TREE01 and a 10-minute
> torture.sh (which Neeraj likely already ran a longer version of).  I fired

200-minute torture.sh completed successfully at my end.

- Neeraj

> off a 12-hour 200*TREE01 run and a 60-minute torture.sh for overnight.
>
> Here is hoping!  ;-)
>
>                                                         Thanx, Paul

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Sat, Sep 07, 2024 at 02:47:30PM +0530, Neeraj upadhyay wrote:
> On Fri, Sep 6, 2024 at 9:58 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Fri, Sep 06, 2024 at 03:12:11PM +0200, Frederic Weisbecker wrote:
> > > Le Thu, Sep 05, 2024 at 11:04:22PM -0700, Paul E. McKenney a écrit :
> > > > On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > > > > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > > > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > > > > embarassing issue while I'm going through it again).
> > > > > > >
> > > > > > > Very good, thank you!
> > > > > >
> > > > > > So my proposal for a replacement patch is this (to replace the patch
> > > > > > of the same name in Neeraj tree):
> > > > >
> > > > > FYI, the diffstat against the previous version of the same patch is as follows.
> > > > > The rationale being:
> > > > >
> > > > > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > > > >
> > > > > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> > > > >    after the barrier's completion.
> > > > >
> > > > > 3) This fixes another issue: rcuoc must be parked _before_
> > > > >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> > > > >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> > > > >    leaving the nocb locked forever.
> > > >
> > > > Thank you!!!
> > > >
> > > > Just to make sure that I understand, I apply this patch on top of
> > > > Neeraj's current set of branches to get the fix, correct?
> > >
> > > Exactly!
> >
> > It passes the initial tests, an hour of 200*TREE01 and a 10-minute
> > torture.sh (which Neeraj likely already ran a longer version of).  I fired
> 
> 200-minute torture.sh completed successfully at my end.

Nice!!!  And thank you!

							Thanx, Paul

> - Neeraj
> 
> > off a 12-hour 200*TREE01 run and a 60-minute torture.sh for overnight.
> >
> > Here is hoping!  ;-)
> >
> >                                                         Thanx, Paul

Re: One-off rcu_nocb_rdp_deoffload bug

[Paul E. McKenney]

On Sat, Sep 07, 2024 at 02:29:43AM -0700, Paul E. McKenney wrote:
> On Sat, Sep 07, 2024 at 02:47:30PM +0530, Neeraj upadhyay wrote:
> > On Fri, Sep 6, 2024 at 9:58 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > >
> > > On Fri, Sep 06, 2024 at 03:12:11PM +0200, Frederic Weisbecker wrote:
> > > > Le Thu, Sep 05, 2024 at 11:04:22PM -0700, Paul E. McKenney a écrit :
> > > > > On Thu, Sep 05, 2024 at 08:41:02PM +0200, Frederic Weisbecker wrote:
> > > > > > Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > > > > > > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > > > > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > > > > > > embarassing issue while I'm going through it again).
> > > > > > > >
> > > > > > > > Very good, thank you!
> > > > > > >
> > > > > > > So my proposal for a replacement patch is this (to replace the patch
> > > > > > > of the same name in Neeraj tree):
> > > > > >
> > > > > > FYI, the diffstat against the previous version of the same patch is as follows.
> > > > > > The rationale being:
> > > > > >
> > > > > > 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
> > > > > >
> > > > > > 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
> > > > > >    after the barrier's completion.
> > > > > >
> > > > > > 3) This fixes another issue: rcuoc must be parked _before_
> > > > > >    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
> > > > > >    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
> > > > > >    leaving the nocb locked forever.
> > > > >
> > > > > Thank you!!!
> > > > >
> > > > > Just to make sure that I understand, I apply this patch on top of
> > > > > Neeraj's current set of branches to get the fix, correct?
> > > >
> > > > Exactly!
> > >
> > > It passes the initial tests, an hour of 200*TREE01 and a 10-minute
> > > torture.sh (which Neeraj likely already ran a longer version of).  I fired
> > 
> > 200-minute torture.sh completed successfully at my end.
> 
> Nice!!!  And thank you!
> 
> 							Thanx, Paul
> 
> > - Neeraj
> > 
> > > off a 12-hour 200*TREE01 run and a 60-minute torture.sh for overnight.

And these passed as well.

> > > Here is hoping!  ;-)

                                                        Thanx, Paul

Re: One-off rcu_nocb_rdp_deoffload bug

[Z qiang]

>
> Le Thu, Sep 05, 2024 at 08:32:16PM +0200, Frederic Weisbecker a écrit :
> > Le Wed, Sep 04, 2024 at 06:52:36AM -0700, Paul E. McKenney a écrit :
> > > > Yes, I'm preparing an update for the offending patch (which has one more
> > > > embarassing issue while I'm going through it again).
> > >
> > > Very good, thank you!
> >
> > So my proposal for a replacement patch is this (to replace the patch
> > of the same name in Neeraj tree):
>
> FYI, the diffstat against the previous version of the same patch is as follows.
> The rationale being:
>
> 1) rdp->nocb_cb_kthread doesn't need to be protected by nocb_gp_kthread_mutex
>
> 2) Once rcuoc is parked, we really _must_ observe the callback list counter decremented
>    after the barrier's completion.
>
> 3) This fixes another issue: rcuoc must be parked _before_
>    rcu_nocb_queue_toggle_rdp() is called, otherwise a nocb locked sequence
>    within rcuoc would race with rcuog clearing SEGCBLIST_OFFLOADED concurrently,
>    leaving the nocb locked forever.
>
>

This WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)) warning still
occurs.
insmod rcutorture.ko torture_type=rcu fwd_progress=8 n_barrier_cbs=8
nocbs_nthreads=8 nocbs_toggle=100 onoff_interval=60 test_boost=2

I added some debug modifications:

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 97b99cd06923..f44eee88b8ad 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1042,6 +1042,20 @@ static bool
rcu_nocb_rdp_deoffload_wait_cond(struct rcu_data *rdp)
  return ret;
 }

+static void rcu_nocb_dump_cblist(struct rcu_segcblist *cblist, int ncheck)
+{
+ struct rcu_head *rhp;
+ int index = 0;
+
+ rhp = cblist->head;
+ while (rhp) {
+ pr_info("rcu_callback func: %ps\n", rhp->func);
+ if (++index >= ncheck)
+ break;
+ rhp = rhp->next;
+ }
+}
+
 static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
 {
  unsigned long flags;
@@ -1065,7 +1079,8 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)

  rcu_nocb_lock_irqsave(rdp, flags);
  WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
- WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
+ if (WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist)))
+ rcu_nocb_dump_cblist(&rdp->cblist, 10);
  rcu_nocb_unlock_irqrestore(rdp, flags);

  wake_gp = rcu_nocb_queue_toggle_rdp(rdp);

dmesg:

4,5360,150691992,-,caller=T453;WARNING: CPU: 4 PID: 453 at
kernel/rcu/tree_nocb.h:1082 rcu_nocb_rdp_deoffload+0x565/0x7e0
4,5361,150692011,-,caller=T453;Modules linked in: rcutorture torture
4,5362,150692040,-,caller=T453;CPU: 4 UID: 0 PID: 453 Comm:
rcu_nocb_toggle Not tainted
6.11.0-next-20240927-yoctodev-standard-dirty #67
3a2655b4a49977d4639e002ae2d5e9a4a1f2d7e2
4,5363,150692054,-,caller=T453;Hardware name: QEMU Standard PC (Q35 +
ICH9, 2009), BIOS rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org
04/01/2014
4,5364,150692064,-,caller=T453;RIP: 0010:rcu_nocb_rdp_deoffload+0x565/0x7e0
4,5365,150692082,-,caller=T453;Code: 48 b8 00 00 00 00 00 fc ff df 80
3c 02 00 0f 85 f2 01 00 00 48 b8 22 01 00 00 00 00 ad de 48 89 83 e0
04 00 00 e9 fa fd ff ff <0f> 0b 48 8d bb a8 00 00 00 48 b8 00 00a
4,5366,150692092,-,caller=T453;RSP: 0018:ffff888006a0fcf8 EFLAGS: 00010002
4,5367,150692114,-,caller=T453;RAX: 0000000000000001 RBX:
ffff88806ce103c0 RCX: 1ffff1100d9c2096
4,5368,150692124,-,caller=T453;RDX: 0000000000000000 RSI:
0000000000000008 RDI: ffff88806ce104b0
4,5369,150692133,-,caller=T453;RBP: ffff888006a0fda8 R08:
ffffed100d9c2097 R09: ffffed100d9c2097
4,5370,150692142,-,caller=T453;R10: ffffed100d9c2096 R11:
ffff88806ce104b7 R12: ffff88806ce105e0
4,5371,150692152,-,caller=T453;R13: ffff88806c2103c0 R14:
0000000000000246 R15: ffff88806ce104b0
4,5372,150692162,-,caller=T453;FS:  0000000000000000(0000)
GS:ffff88806c000000(0000) knlGS:0000000000000000
4,5373,150692174,-,caller=T453;CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
4,5374,150692183,-,caller=T453;CR2: 0000000000000000 CR3:
000000000bbd2000 CR4: 00000000001506f0
4,5375,150692193,-,caller=T453;Call Trace:
4,5376,150692202,-,caller=T453; <TASK>
4,5377,150692214,-,caller=T453; ? show_regs+0x65/0x70
4,5378,150692232,-,caller=T453; ? __warn+0xd5/0x310
4,5379,150692255,-,caller=T453; ? rcu_nocb_rdp_deoffload+0x565/0x7e0
4,5380,150692274,-,caller=T453; ? report_bug+0x23e/0x490
4,5381,150692312,-,caller=T453; ? handle_bug+0x5b/0xa0
4,5382,150692327,-,caller=T453; ? exc_invalid_op+0x1c/0x50
4,5383,150692346,-,caller=T453; ? asm_exc_invalid_op+0x1f/0x30
4,5384,150692398,-,caller=T453; ? rcu_nocb_rdp_deoffload+0x565/0x7e0
4,5385,150692420,-,caller=T453; ? rcu_nocb_rdp_deoffload+0x1f4/0x7e0
4,5386,150692448,-,caller=T453; ? __pfx_rcu_nocb_rdp_deoffload+0x10/0x10
4,5387,150692498,-,caller=T453; rcu_nocb_cpu_deoffload+0xb9/0x140
4,5388,150692521,-,caller=T453; rcu_nocb_toggle+0x2ac/0x3c0
[rcutorture 4065733c5816d1d4498e7501cdbf52edab89e7f2]
4,5389,150692547,-,caller=T453; ? _raw_spin_unlock_irqrestore+0x4a/0x80
4,5390,150692579,-,caller=T453; ? __pfx_rcu_nocb_toggle+0x10/0x10
[rcutorture 4065733c5816d1d4498e7501cdbf52edab89e7f2]
4,5391,150692620,-,caller=T453; ? __kthread_parkme+0xb8/0x1e0
4,5392,150692637,-,caller=T453; ? preempt_count_sub+0x124/0x1b0
4,5393,150692661,-,caller=T453; ? __kasan_check_read+0x15/0x20
4,5394,150692675,-,caller=T453; ? __kthread_parkme+0xc5/0x1e0
4,5395,150692699,-,caller=T453; ? __pfx_rcu_nocb_toggle+0x10/0x10
[rcutorture 4065733c5816d1d4498e7501cdbf52edab89e7f2]
4,5396,150692728,-,caller=T453; kthread+0x306/0x3e0
4,5397,150692746,-,caller=T453; ? __pfx_kthread+0x10/0x10
4,5398,150692767,-,caller=T453; ret_from_fork+0x40/0x70
4,5399,150692779,-,caller=T453; ? __pfx_kthread+0x10/0x10
4,5400,150692797,-,caller=T453; ret_from_fork_asm+0x1a/0x30
4,5401,150692863,-,caller=T453; </TASK>
4,5402,150692872,-,caller=T453;irq event stamp: 21504
4,5403,150692880,-,caller=T453;hardirqs last  enabled at (21503):
[<ffffffffb3680a63>] _raw_spin_unlock_irqrestore+0x63/0x80
4,5404,150692894,-,caller=T453;hardirqs last disabled at (21504):
[<ffffffffb03fc77e>] rcu_nocb_rdp_deoffload+0x3ee/0x7e0
4,5405,150692906,-,caller=T453;softirqs last  enabled at (21324):
[<ffffffffb01dd618>] handle_softirqs+0x428/0x800
4,5406,150692917,-,caller=T453;softirqs last disabled at (21315):
[<ffffffffb01de8ef>] irq_exit_rcu+0x1af/0x220
4,5407,150692927,-,caller=T453;---[ end trace 0000000000000000 ]---

When the rcuoc kthreads process rcu callback, before invoke
rcu_segcblist_add_len(&rdp->cblist, -count),
the rcu_barrier() can insert rcu_barrier_callback() func to offline
cpu rdp's->list.

6,5408,150692937,-,caller=T453;rcu: rcu_callback func: rcu_barrier_callback

Maybe we can wait until rcu_segcblist_n_cbs(&rdp->cblist) return zero
and then invoke kthread_parkme() in rcuoc kthreads.

Any thoughts ?

Thanks
Zqiang


> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 755ada098035..97b99cd06923 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1056,6 +1056,13 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
>         /* Flush all callbacks from segcblist and bypass */
>         rcu_barrier();
>
> +       /*
> +        * Make sure the rcuoc kthread isn't in the middle of a nocb locked
> +        * sequence while offloading is deactivated, along with nocb locking.
> +        */
> +       if (rdp->nocb_cb_kthread)
> +               kthread_park(rdp->nocb_cb_kthread);
> +
>         rcu_nocb_lock_irqsave(rdp, flags);
>         WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
>         WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
> @@ -1064,13 +1071,11 @@ static int rcu_nocb_rdp_deoffload(struct rcu_data *rdp)
>         wake_gp = rcu_nocb_queue_toggle_rdp(rdp);
>
>         mutex_lock(&rdp_gp->nocb_gp_kthread_mutex);
> +
>         if (rdp_gp->nocb_gp_kthread) {
>                 if (wake_gp)
>                         wake_up_process(rdp_gp->nocb_gp_kthread);
>
> -               if (rdp->nocb_cb_kthread)
> -                       kthread_park(rdp->nocb_cb_kthread);
> -
>                 swait_event_exclusive(rdp->nocb_state_wq,
>                                       rcu_nocb_rdp_deoffload_wait_cond(rdp));
>         } else {

Re: One-off rcu_nocb_rdp_deoffload bug

[Frederic Weisbecker]

Le Thu, Oct 03, 2024 at 10:01:57PM +0800, Z qiang a écrit :
> When the rcuoc kthreads process rcu callback, before invoke
> rcu_segcblist_add_len(&rdp->cblist, -count),
> the rcu_barrier() can insert rcu_barrier_callback() func to offline
> cpu rdp's->list.

Excellent analysis! Indeed we can have:

CPU 0                         CPU 1                          CPU 2
-----                         -----                          -------

// deoffload                  // nocb_cb_wait                // rcutorture
rcu_barrier()
    rcu_segcblist_entrain()
        rcu_segcblist_add_len(1);
                             rcu_do_batch()
                                 rcu_barrier_callback()
                                                             rcu_barrier()
                                                                 // still see len == 1
                                                                 rcu_segcblist_entrain()
                                                                      rcu_segcblist_add_len(1);
                                 // decrement len
                                 rcu_segcblist_add_len(-1);
                             kthread_parkme()
// Warn because there is
// still a pending barrier
WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));


And the worst is that the second rcu_barrier() is ignored.

> 
> 6,5408,150692937,-,caller=T453;rcu: rcu_callback func: rcu_barrier_callback
> 
> Maybe we can wait until rcu_segcblist_n_cbs(&rdp->cblist) return zero
> and then invoke kthread_parkme() in rcuoc kthreads.

> Any thoughts ?

Sounds good, or simply make sure that rdp->nocb_cb_sleep == false before
parking? Since kthread_park() should only be called after rcu_barrier() and
then rdp->nocb_cb_sleep shouldn't be set to true as long as there is a pending
one? Well we can also add a WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist))
before calling kthread_park().

Would you like to send the fix?

Thanks.

Re: One-off rcu_nocb_rdp_deoffload bug

[Z qiang]

>
> Le Thu, Oct 03, 2024 at 10:01:57PM +0800, Z qiang a écrit :
> > When the rcuoc kthreads process rcu callback, before invoke
> > rcu_segcblist_add_len(&rdp->cblist, -count),
> > the rcu_barrier() can insert rcu_barrier_callback() func to offline
> > cpu rdp's->list.
>
> Excellent analysis! Indeed we can have:
>
> CPU 0                         CPU 1                          CPU 2
> -----                         -----                          -------
>
> // deoffload                  // nocb_cb_wait                // rcutorture
> rcu_barrier()
>     rcu_segcblist_entrain()
>         rcu_segcblist_add_len(1);
>                              rcu_do_batch()
>                                  rcu_barrier_callback()
>                                                              rcu_barrier()
>                                                                  // still see len == 1
>                                                                  rcu_segcblist_entrain()
>                                                                       rcu_segcblist_add_len(1);
>                                  // decrement len
>                                  rcu_segcblist_add_len(-1);
>                              kthread_parkme()
> // Warn because there is
> // still a pending barrier
> WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist));
>
>
> And the worst is that the second rcu_barrier() is ignored.
>
> >
> > 6,5408,150692937,-,caller=T453;rcu: rcu_callback func: rcu_barrier_callback
> >
> > Maybe we can wait until rcu_segcblist_n_cbs(&rdp->cblist) return zero
> > and then invoke kthread_parkme() in rcuoc kthreads.
>
> > Any thoughts ?
>
> Sounds good, or simply make sure that rdp->nocb_cb_sleep == false before
> parking? Since kthread_park() should only be called after rcu_barrier() and
> then rdp->nocb_cb_sleep shouldn't be set to true as long as there is a pending
> one? Well we can also add a WARN_ON_ONCE(rcu_segcblist_n_cbs(&rdp->cblist))
> before calling kthread_park().
>
> Would you like to send the fix?

I will update and send the fix :) .

Thanks
Zqiang

>
> Thanks.