* One-off rcu_nocb_rdp_deoffload bug
@ 2024-08-19 19:44 Paul E. McKenney
2024-08-20 11:07 ` Z qiang
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-08-19 19:44 UTC (permalink / raw)
To: frederic; +Cc: rcu
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>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-08-19 19:44 One-off rcu_nocb_rdp_deoffload bug Paul E. McKenney
@ 2024-08-20 11:07 ` Z qiang
[not found] ` <CAFTL4hzY09QEGE+sTqc02d2LkuH_ObYKiuVTfGnrtwjtYiwt4A@mail.gmail.com>
0 siblings, 1 reply; 19+ messages in thread
From: Z qiang @ 2024-08-20 11:07 UTC (permalink / raw)
To: paulmck; +Cc: frederic, rcu
>
> 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>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
[not found] ` <CAFTL4hzY09QEGE+sTqc02d2LkuH_ObYKiuVTfGnrtwjtYiwt4A@mail.gmail.com>
@ 2024-08-20 17:46 ` Paul E. McKenney
2024-09-04 12:59 ` Paul E. McKenney
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-08-20 17:46 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Z qiang, rcu
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>
> > >
> > >
> >
> >
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-08-20 17:46 ` Paul E. McKenney
@ 2024-09-04 12:59 ` Paul E. McKenney
2024-09-04 13:48 ` Frederic Weisbecker
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-04 12:59 UTC (permalink / raw)
To: Frederic Weisbecker, Z qiang, rcu
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 ]---
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-04 12:59 ` Paul E. McKenney
@ 2024-09-04 13:48 ` Frederic Weisbecker
2024-09-04 13:52 ` Paul E. McKenney
0 siblings, 1 reply; 19+ messages in thread
From: Frederic Weisbecker @ 2024-09-04 13:48 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Z qiang, rcu
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-04 13:48 ` Frederic Weisbecker
@ 2024-09-04 13:52 ` Paul E. McKenney
2024-09-05 18:32 ` Frederic Weisbecker
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-04 13:52 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Z qiang, rcu
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-04 13:52 ` Paul E. McKenney
@ 2024-09-05 18:32 ` Frederic Weisbecker
2024-09-05 18:41 ` Frederic Weisbecker
0 siblings, 1 reply; 19+ messages in thread
From: Frederic Weisbecker @ 2024-09-05 18:32 UTC (permalink / raw)
To: Paul E. McKenney, Neeraj Upadhyay; +Cc: Z qiang, rcu, 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
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-05 18:32 ` Frederic Weisbecker
@ 2024-09-05 18:41 ` Frederic Weisbecker
2024-09-06 6:04 ` Paul E. McKenney
2024-10-03 14:01 ` Z qiang
0 siblings, 2 replies; 19+ messages in thread
From: Frederic Weisbecker @ 2024-09-05 18:41 UTC (permalink / raw)
To: Paul E. McKenney, Neeraj Upadhyay; +Cc: Z qiang, rcu
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 {
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-05 18:41 ` Frederic Weisbecker
@ 2024-09-06 6:04 ` Paul E. McKenney
2024-09-06 6:32 ` Neeraj upadhyay
2024-09-06 13:12 ` Frederic Weisbecker
2024-10-03 14:01 ` Z qiang
1 sibling, 2 replies; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-06 6:04 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Neeraj Upadhyay, Z qiang, rcu
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 {
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-06 6:04 ` Paul E. McKenney
@ 2024-09-06 6:32 ` Neeraj upadhyay
2024-09-06 7:48 ` Paul E. McKenney
2024-09-06 13:12 ` Frederic Weisbecker
1 sibling, 1 reply; 19+ messages in thread
From: Neeraj upadhyay @ 2024-09-06 6:32 UTC (permalink / raw)
To: paulmck; +Cc: Frederic Weisbecker, Neeraj Upadhyay, Z qiang, rcu
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 {
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-06 6:32 ` Neeraj upadhyay
@ 2024-09-06 7:48 ` Paul E. McKenney
0 siblings, 0 replies; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-06 7:48 UTC (permalink / raw)
To: Neeraj upadhyay; +Cc: Frederic Weisbecker, Neeraj Upadhyay, Z qiang, rcu
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 {
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-06 6:04 ` Paul E. McKenney
2024-09-06 6:32 ` Neeraj upadhyay
@ 2024-09-06 13:12 ` Frederic Weisbecker
2024-09-06 16:28 ` Paul E. McKenney
1 sibling, 1 reply; 19+ messages in thread
From: Frederic Weisbecker @ 2024-09-06 13:12 UTC (permalink / raw)
To: Paul E. McKenney; +Cc: Neeraj Upadhyay, Z qiang, rcu
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-06 13:12 ` Frederic Weisbecker
@ 2024-09-06 16:28 ` Paul E. McKenney
2024-09-07 9:17 ` Neeraj upadhyay
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-06 16:28 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Neeraj Upadhyay, Z qiang, rcu
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-06 16:28 ` Paul E. McKenney
@ 2024-09-07 9:17 ` Neeraj upadhyay
2024-09-07 9:29 ` Paul E. McKenney
0 siblings, 1 reply; 19+ messages in thread
From: Neeraj upadhyay @ 2024-09-07 9:17 UTC (permalink / raw)
To: paulmck; +Cc: Frederic Weisbecker, Neeraj Upadhyay, Z qiang, rcu
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-07 9:17 ` Neeraj upadhyay
@ 2024-09-07 9:29 ` Paul E. McKenney
2024-09-07 9:33 ` Paul E. McKenney
0 siblings, 1 reply; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-07 9:29 UTC (permalink / raw)
To: Neeraj upadhyay; +Cc: Frederic Weisbecker, Neeraj Upadhyay, Z qiang, rcu
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-07 9:29 ` Paul E. McKenney
@ 2024-09-07 9:33 ` Paul E. McKenney
0 siblings, 0 replies; 19+ messages in thread
From: Paul E. McKenney @ 2024-09-07 9:33 UTC (permalink / raw)
To: Neeraj upadhyay; +Cc: Frederic Weisbecker, Neeraj Upadhyay, Z qiang, rcu
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
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-09-05 18:41 ` Frederic Weisbecker
2024-09-06 6:04 ` Paul E. McKenney
@ 2024-10-03 14:01 ` Z qiang
2024-10-03 23:50 ` Frederic Weisbecker
1 sibling, 1 reply; 19+ messages in thread
From: Z qiang @ 2024-10-03 14:01 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Paul E. McKenney, Neeraj Upadhyay, rcu
>
> 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 {
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-10-03 14:01 ` Z qiang
@ 2024-10-03 23:50 ` Frederic Weisbecker
2024-10-08 2:13 ` Z qiang
0 siblings, 1 reply; 19+ messages in thread
From: Frederic Weisbecker @ 2024-10-03 23:50 UTC (permalink / raw)
To: Z qiang; +Cc: Paul E. McKenney, Neeraj Upadhyay, rcu
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.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: One-off rcu_nocb_rdp_deoffload bug
2024-10-03 23:50 ` Frederic Weisbecker
@ 2024-10-08 2:13 ` Z qiang
0 siblings, 0 replies; 19+ messages in thread
From: Z qiang @ 2024-10-08 2:13 UTC (permalink / raw)
To: Frederic Weisbecker; +Cc: Paul E. McKenney, Neeraj Upadhyay, rcu
>
> 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.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2024-10-08 2:13 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-19 19:44 One-off rcu_nocb_rdp_deoffload bug Paul E. McKenney
2024-08-20 11:07 ` Z qiang
[not found] ` <CAFTL4hzY09QEGE+sTqc02d2LkuH_ObYKiuVTfGnrtwjtYiwt4A@mail.gmail.com>
2024-08-20 17:46 ` Paul E. McKenney
2024-09-04 12:59 ` Paul E. McKenney
2024-09-04 13:48 ` Frederic Weisbecker
2024-09-04 13:52 ` Paul E. McKenney
2024-09-05 18:32 ` Frederic Weisbecker
2024-09-05 18:41 ` Frederic Weisbecker
2024-09-06 6:04 ` Paul E. McKenney
2024-09-06 6:32 ` Neeraj upadhyay
2024-09-06 7:48 ` Paul E. McKenney
2024-09-06 13:12 ` Frederic Weisbecker
2024-09-06 16:28 ` Paul E. McKenney
2024-09-07 9:17 ` Neeraj upadhyay
2024-09-07 9:29 ` Paul E. McKenney
2024-09-07 9:33 ` Paul E. McKenney
2024-10-03 14:01 ` Z qiang
2024-10-03 23:50 ` Frederic Weisbecker
2024-10-08 2:13 ` Z qiang
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.