* #PF from NMI
@ 2020-11-13 12:53 Peter Zijlstra
2020-11-13 17:16 ` Paul E. McKenney
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Peter Zijlstra @ 2020-11-13 12:53 UTC (permalink / raw)
To: Paul McKenney, eupm90, Thomas Gleixner; +Cc: linux-kernel, x86
Hi
Eugenio reported
(https://bugzilla.kernel.org/attachment.cgi?id=293659&action=edit):
[ 139.226723] ------------[ cut here ]------------
[ 139.226724] WARNING: CPU: 9 PID: 2290 at kernel/rcu/tree.c:932 __rcu_irq_enter_check_tick+0x84/0xd0
[ 139.226725] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_e
[ 139.226746] CPU: 9 PID: 2290 Comm: perf Tainted: G S 5.10.0-rc2+ #1
[ 139.226746] Hardware name: Dell Inc. PowerEdge R430/0CN7X8, BIOS 2.6.0 10/31/2017
[ 139.226747] RIP: 0010:__rcu_irq_enter_check_tick+0x84/0xd0
[ 139.226747] Code: 75 cc 48 8b 7b 18 e8 4b c9 84 00 80 bb 05 01 00 00 00 74 09 80 bb 06 01 00 00 00 74 11 48 8b 7b 18 c6 07 00 0f 1f 40 00
[ 139.226748] RSP: 0000:ffffbdb38586b8c8 EFLAGS: 00010006
[ 139.226749] RAX: 0000000080110000 RBX: ffff9cdc2f92b9c0 RCX: ffffffff9f600fb7
[ 139.226749] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffbdb38586b938
[ 139.226749] RBP: 000000000000000e R08: 0000000000000000 R09: 0000000000000000
[ 139.226750] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 139.226750] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 139.226750] FS: 00007fb0c5a628c0(0000) GS:ffff9cdc2f900000(0000) knlGS:0000000000000000
[ 139.226751] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 139.226751] CR2: 000000000000000e CR3: 000000010e0ea001 CR4: 00000000003706a0
[ 139.226752] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 139.226752] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 139.226752] Call Trace:
[ 139.226753] irqentry_enter+0x25/0x40
[ 139.226753] exc_page_fault+0x38/0x4c0
[ 139.226753] asm_exc_page_fault+0x1e/0x30
[ 139.226753] RIP: 0010:__get_user_nocheck_8+0x6/0x13
[ 139.226754] Code: 01 ca c3 90 0f 01 cb 0f ae e8 0f b7 10 31 c0 0f 01 ca c3 90 0f 01 cb 0f ae e8 8b 10 31 c0 0f 01 ca c3 66 90 0f 01 cb 0f
[ 139.226754] RSP: 0000:ffffbdb38586b9e8 EFLAGS: 00050087
[ 139.226755] RAX: 000000000000000e RBX: ffffbdb38586ba30 RCX: 00007fb0c586f0d2
[ 139.226756] RDX: 0000000000000080 RSI: 000000000000007f RDI: 00000000ffffdff0
[ 139.226756] RBP: ffffbdb38586bf58 R08: 00000000bffffff0 R09: ffffffffa05a0460
[ 139.226756] R10: 00ffffffffffeff0 R11: 00007fffffffeff0 R12: 000000000000000e
[ 139.226757] R13: ffff9cd4daa017c0 R14: 0000000000000000 R15: 000000000000007f
[ 139.226757] ? kvmclock_cpu_down_prep+0x20/0x20 [kvm]
[ 139.226757] perf_callchain_user+0xf4/0x280
[ 139.226758] get_perf_callchain+0x17b/0x1b0
[ 139.226758] perf_callchain+0x71/0x80
[ 139.226758] setup_pebs_fixed_sample_data+0x2a8/0x360
[ 139.226759] intel_pmu_drain_pebs_nhm+0x477/0x5d0
[ 139.226759] ? get_page_from_freelist+0x117d/0x1390
[ 139.226759] ? page_counter_cancel+0x1f/0x30
[ 139.226760] ? page_counter_uncharge+0x1d/0x30
[ 139.226760] ? drain_stock.isra.54+0x64/0xb0
[ 139.226760] ? get_page_from_freelist+0x117d/0x1390
[ 139.226761] handle_pmi_common+0xc3/0x2d0
[ 139.226761] ? alloc_set_pte+0xd3/0x5e0
[ 139.226761] ? filemap_map_pages+0x2d2/0x430
[ 139.226762] ? xfs_filemap_map_pages+0x44/0x60 [xfs]
[ 139.226762] ? xfs_iunlock+0x86/0xd0 [xfs]
[ 139.226762] ? handle_mm_fault+0x145b/0x1650
[ 139.226762] intel_pmu_handle_irq+0xc9/0x1c0
[ 139.226763] perf_event_nmi_handler+0x24/0x40
[ 139.226763] nmi_handle+0x55/0x130
[ 139.226763] default_do_nmi+0x49/0x100
[ 139.226764] exc_nmi+0x121/0x150
[ 139.226764] asm_exc_nmi+0x8e/0xd7
[ 139.226764] RIP: 0033:0x7fb0c586f0d2
[ 139.226765] Code: 48 89 44 24 30 48 8b 83 00 03 00 00 48 85 c0 0f 84 d3 00 00 00 48 8b 7c 24 20 8b 8b fc 02 00 00 8b 54 24 38 23 93 f8 08
[ 139.226766] RSP: 002b:00007ffcf13cde60 EFLAGS: 00010246
[ 139.226766] RAX: 00007fb0c309c2b0 RBX: 00007fb0c5a73500 RCX: 0000000000000006
[ 139.226766] RDX: 0000000000000000 RSI: 000000000000000a RDI: 00000000fc2c9fd0
[ 139.226767] RBP: 000000000000000e R08: 00007ffcf13cdf90 R09: 00007fb0c5a8f410
[ 139.226767] R10: 000000000000002b R11: 0000000000000246 R12: 0000000000000000
[ 139.226768] R13: 0000000000000001 R14: 00007fb0c32a1fbc R15: 00007fb0c5a69cd0
Which is a #PF from NMI context, which is perfectly fine. However
__rcu_irq_enter_check_tick() is triggering WARN.
AFAICT the right thing is to simply remove the warn like so.
---
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 430ba58d8bfe..9bda92d8b914 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
{
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
- // Enabling the tick is unsafe in NMI handlers.
- if (WARN_ON_ONCE(in_nmi()))
+ // if we're here from NMI, there's nothing to do.
+ if (in_nmi())
return;
RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: #PF from NMI
2020-11-13 12:53 #PF from NMI Peter Zijlstra
@ 2020-11-13 17:16 ` Paul E. McKenney
2020-11-13 17:23 ` Andy Lutomirski
2020-11-13 23:13 ` Thomas Gleixner
2 siblings, 0 replies; 9+ messages in thread
From: Paul E. McKenney @ 2020-11-13 17:16 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: eupm90, Thomas Gleixner, linux-kernel, x86
On Fri, Nov 13, 2020 at 01:53:32PM +0100, Peter Zijlstra wrote:
> Hi
>
> Eugenio reported
> (https://bugzilla.kernel.org/attachment.cgi?id=293659&action=edit):
>
> [ 139.226723] ------------[ cut here ]------------
> [ 139.226724] WARNING: CPU: 9 PID: 2290 at kernel/rcu/tree.c:932 __rcu_irq_enter_check_tick+0x84/0xd0
> [ 139.226725] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_e
> [ 139.226746] CPU: 9 PID: 2290 Comm: perf Tainted: G S 5.10.0-rc2+ #1
> [ 139.226746] Hardware name: Dell Inc. PowerEdge R430/0CN7X8, BIOS 2.6.0 10/31/2017
> [ 139.226747] RIP: 0010:__rcu_irq_enter_check_tick+0x84/0xd0
> [ 139.226747] Code: 75 cc 48 8b 7b 18 e8 4b c9 84 00 80 bb 05 01 00 00 00 74 09 80 bb 06 01 00 00 00 74 11 48 8b 7b 18 c6 07 00 0f 1f 40 00
> [ 139.226748] RSP: 0000:ffffbdb38586b8c8 EFLAGS: 00010006
> [ 139.226749] RAX: 0000000080110000 RBX: ffff9cdc2f92b9c0 RCX: ffffffff9f600fb7
> [ 139.226749] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffbdb38586b938
> [ 139.226749] RBP: 000000000000000e R08: 0000000000000000 R09: 0000000000000000
> [ 139.226750] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 139.226750] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 139.226750] FS: 00007fb0c5a628c0(0000) GS:ffff9cdc2f900000(0000) knlGS:0000000000000000
> [ 139.226751] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 139.226751] CR2: 000000000000000e CR3: 000000010e0ea001 CR4: 00000000003706a0
> [ 139.226752] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 139.226752] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 139.226752] Call Trace:
> [ 139.226753] irqentry_enter+0x25/0x40
> [ 139.226753] exc_page_fault+0x38/0x4c0
> [ 139.226753] asm_exc_page_fault+0x1e/0x30
> [ 139.226753] RIP: 0010:__get_user_nocheck_8+0x6/0x13
> [ 139.226754] Code: 01 ca c3 90 0f 01 cb 0f ae e8 0f b7 10 31 c0 0f 01 ca c3 90 0f 01 cb 0f ae e8 8b 10 31 c0 0f 01 ca c3 66 90 0f 01 cb 0f
> [ 139.226754] RSP: 0000:ffffbdb38586b9e8 EFLAGS: 00050087
> [ 139.226755] RAX: 000000000000000e RBX: ffffbdb38586ba30 RCX: 00007fb0c586f0d2
> [ 139.226756] RDX: 0000000000000080 RSI: 000000000000007f RDI: 00000000ffffdff0
> [ 139.226756] RBP: ffffbdb38586bf58 R08: 00000000bffffff0 R09: ffffffffa05a0460
> [ 139.226756] R10: 00ffffffffffeff0 R11: 00007fffffffeff0 R12: 000000000000000e
> [ 139.226757] R13: ffff9cd4daa017c0 R14: 0000000000000000 R15: 000000000000007f
> [ 139.226757] ? kvmclock_cpu_down_prep+0x20/0x20 [kvm]
> [ 139.226757] perf_callchain_user+0xf4/0x280
> [ 139.226758] get_perf_callchain+0x17b/0x1b0
> [ 139.226758] perf_callchain+0x71/0x80
> [ 139.226758] setup_pebs_fixed_sample_data+0x2a8/0x360
> [ 139.226759] intel_pmu_drain_pebs_nhm+0x477/0x5d0
> [ 139.226759] ? get_page_from_freelist+0x117d/0x1390
> [ 139.226759] ? page_counter_cancel+0x1f/0x30
> [ 139.226760] ? page_counter_uncharge+0x1d/0x30
> [ 139.226760] ? drain_stock.isra.54+0x64/0xb0
> [ 139.226760] ? get_page_from_freelist+0x117d/0x1390
> [ 139.226761] handle_pmi_common+0xc3/0x2d0
> [ 139.226761] ? alloc_set_pte+0xd3/0x5e0
> [ 139.226761] ? filemap_map_pages+0x2d2/0x430
> [ 139.226762] ? xfs_filemap_map_pages+0x44/0x60 [xfs]
> [ 139.226762] ? xfs_iunlock+0x86/0xd0 [xfs]
> [ 139.226762] ? handle_mm_fault+0x145b/0x1650
> [ 139.226762] intel_pmu_handle_irq+0xc9/0x1c0
> [ 139.226763] perf_event_nmi_handler+0x24/0x40
> [ 139.226763] nmi_handle+0x55/0x130
> [ 139.226763] default_do_nmi+0x49/0x100
> [ 139.226764] exc_nmi+0x121/0x150
> [ 139.226764] asm_exc_nmi+0x8e/0xd7
> [ 139.226764] RIP: 0033:0x7fb0c586f0d2
> [ 139.226765] Code: 48 89 44 24 30 48 8b 83 00 03 00 00 48 85 c0 0f 84 d3 00 00 00 48 8b 7c 24 20 8b 8b fc 02 00 00 8b 54 24 38 23 93 f8 08
> [ 139.226766] RSP: 002b:00007ffcf13cde60 EFLAGS: 00010246
> [ 139.226766] RAX: 00007fb0c309c2b0 RBX: 00007fb0c5a73500 RCX: 0000000000000006
> [ 139.226766] RDX: 0000000000000000 RSI: 000000000000000a RDI: 00000000fc2c9fd0
> [ 139.226767] RBP: 000000000000000e R08: 00007ffcf13cdf90 R09: 00007fb0c5a8f410
> [ 139.226767] R10: 000000000000002b R11: 0000000000000246 R12: 0000000000000000
> [ 139.226768] R13: 0000000000000001 R14: 00007fb0c32a1fbc R15: 00007fb0c5a69cd0
>
> Which is a #PF from NMI context, which is perfectly fine. However
> __rcu_irq_enter_check_tick() is triggering WARN.
>
> AFAICT the right thing is to simply remove the warn like so.
I have to defer to you guys on this one.
But is a corresponding change required on return-from-NMI side?
Looks OK to me at first glance, but I could be missing something.
Plus I am most likely looking at the wrong version of the x86
entry/exit code. :-/
Thanx, Paul
> ---
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 430ba58d8bfe..9bda92d8b914 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> - // Enabling the tick is unsafe in NMI handlers.
> - if (WARN_ON_ONCE(in_nmi()))
> + // if we're here from NMI, there's nothing to do.
> + if (in_nmi())
> return;
>
> RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: #PF from NMI
2020-11-13 12:53 #PF from NMI Peter Zijlstra
2020-11-13 17:16 ` Paul E. McKenney
@ 2020-11-13 17:23 ` Andy Lutomirski
2020-11-13 23:13 ` Thomas Gleixner
2 siblings, 0 replies; 9+ messages in thread
From: Andy Lutomirski @ 2020-11-13 17:23 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: Paul McKenney, eupm90, Thomas Gleixner, LKML, X86 ML
On Fri, Nov 13, 2020 at 4:53 AM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Hi
>
> Eugenio reported
> (https://bugzilla.kernel.org/attachment.cgi?id=293659&action=edit):
>
> [ 139.226723] ------------[ cut here ]------------
> [ 139.226724] WARNING: CPU: 9 PID: 2290 at kernel/rcu/tree.c:932 __rcu_irq_enter_check_tick+0x84/0xd0
> [ 139.226725] Modules linked in: xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_e
> [ 139.226746] CPU: 9 PID: 2290 Comm: perf Tainted: G S 5.10.0-rc2+ #1
> [ 139.226746] Hardware name: Dell Inc. PowerEdge R430/0CN7X8, BIOS 2.6.0 10/31/2017
> [ 139.226747] RIP: 0010:__rcu_irq_enter_check_tick+0x84/0xd0
> [ 139.226747] Code: 75 cc 48 8b 7b 18 e8 4b c9 84 00 80 bb 05 01 00 00 00 74 09 80 bb 06 01 00 00 00 74 11 48 8b 7b 18 c6 07 00 0f 1f 40 00
> [ 139.226748] RSP: 0000:ffffbdb38586b8c8 EFLAGS: 00010006
> [ 139.226749] RAX: 0000000080110000 RBX: ffff9cdc2f92b9c0 RCX: ffffffff9f600fb7
> [ 139.226749] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffbdb38586b938
> [ 139.226749] RBP: 000000000000000e R08: 0000000000000000 R09: 0000000000000000
> [ 139.226750] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 139.226750] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 139.226750] FS: 00007fb0c5a628c0(0000) GS:ffff9cdc2f900000(0000) knlGS:0000000000000000
> [ 139.226751] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 139.226751] CR2: 000000000000000e CR3: 000000010e0ea001 CR4: 00000000003706a0
> [ 139.226752] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 139.226752] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 139.226752] Call Trace:
> [ 139.226753] irqentry_enter+0x25/0x40
> [ 139.226753] exc_page_fault+0x38/0x4c0
> [ 139.226753] asm_exc_page_fault+0x1e/0x30
> [ 139.226753] RIP: 0010:__get_user_nocheck_8+0x6/0x13
> [ 139.226754] Code: 01 ca c3 90 0f 01 cb 0f ae e8 0f b7 10 31 c0 0f 01 ca c3 90 0f 01 cb 0f ae e8 8b 10 31 c0 0f 01 ca c3 66 90 0f 01 cb 0f
> [ 139.226754] RSP: 0000:ffffbdb38586b9e8 EFLAGS: 00050087
> [ 139.226755] RAX: 000000000000000e RBX: ffffbdb38586ba30 RCX: 00007fb0c586f0d2
> [ 139.226756] RDX: 0000000000000080 RSI: 000000000000007f RDI: 00000000ffffdff0
> [ 139.226756] RBP: ffffbdb38586bf58 R08: 00000000bffffff0 R09: ffffffffa05a0460
> [ 139.226756] R10: 00ffffffffffeff0 R11: 00007fffffffeff0 R12: 000000000000000e
> [ 139.226757] R13: ffff9cd4daa017c0 R14: 0000000000000000 R15: 000000000000007f
> [ 139.226757] ? kvmclock_cpu_down_prep+0x20/0x20 [kvm]
> [ 139.226757] perf_callchain_user+0xf4/0x280
> [ 139.226758] get_perf_callchain+0x17b/0x1b0
> [ 139.226758] perf_callchain+0x71/0x80
> [ 139.226758] setup_pebs_fixed_sample_data+0x2a8/0x360
> [ 139.226759] intel_pmu_drain_pebs_nhm+0x477/0x5d0
> [ 139.226759] ? get_page_from_freelist+0x117d/0x1390
> [ 139.226759] ? page_counter_cancel+0x1f/0x30
> [ 139.226760] ? page_counter_uncharge+0x1d/0x30
> [ 139.226760] ? drain_stock.isra.54+0x64/0xb0
> [ 139.226760] ? get_page_from_freelist+0x117d/0x1390
> [ 139.226761] handle_pmi_common+0xc3/0x2d0
> [ 139.226761] ? alloc_set_pte+0xd3/0x5e0
> [ 139.226761] ? filemap_map_pages+0x2d2/0x430
> [ 139.226762] ? xfs_filemap_map_pages+0x44/0x60 [xfs]
> [ 139.226762] ? xfs_iunlock+0x86/0xd0 [xfs]
> [ 139.226762] ? handle_mm_fault+0x145b/0x1650
> [ 139.226762] intel_pmu_handle_irq+0xc9/0x1c0
> [ 139.226763] perf_event_nmi_handler+0x24/0x40
> [ 139.226763] nmi_handle+0x55/0x130
> [ 139.226763] default_do_nmi+0x49/0x100
> [ 139.226764] exc_nmi+0x121/0x150
> [ 139.226764] asm_exc_nmi+0x8e/0xd7
> [ 139.226764] RIP: 0033:0x7fb0c586f0d2
> [ 139.226765] Code: 48 89 44 24 30 48 8b 83 00 03 00 00 48 85 c0 0f 84 d3 00 00 00 48 8b 7c 24 20 8b 8b fc 02 00 00 8b 54 24 38 23 93 f8 08
> [ 139.226766] RSP: 002b:00007ffcf13cde60 EFLAGS: 00010246
> [ 139.226766] RAX: 00007fb0c309c2b0 RBX: 00007fb0c5a73500 RCX: 0000000000000006
> [ 139.226766] RDX: 0000000000000000 RSI: 000000000000000a RDI: 00000000fc2c9fd0
> [ 139.226767] RBP: 000000000000000e R08: 00007ffcf13cdf90 R09: 00007fb0c5a8f410
> [ 139.226767] R10: 000000000000002b R11: 0000000000000246 R12: 0000000000000000
> [ 139.226768] R13: 0000000000000001 R14: 00007fb0c32a1fbc R15: 00007fb0c5a69cd0
>
> Which is a #PF from NMI context, which is perfectly fine. However
> __rcu_irq_enter_check_tick() is triggering WARN.
>
> AFAICT the right thing is to simply remove the warn like so.
>
> ---
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 430ba58d8bfe..9bda92d8b914 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> - // Enabling the tick is unsafe in NMI handlers.
> - if (WARN_ON_ONCE(in_nmi()))
> + // if we're here from NMI, there's nothing to do.
> + if (in_nmi())
> return;
>
> RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
Reviewed-by: Andy Lutomirski <luto@kernel.org>
with the following caveat that has nothing to do with NMI: the rest of
irqentry_enter() has tracing calls in it. Does anything prevent
infinite recursion if one of those tracing calls causes a page fault?
--Andy
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: #PF from NMI
2020-11-13 12:53 #PF from NMI Peter Zijlstra
2020-11-13 17:16 ` Paul E. McKenney
2020-11-13 17:23 ` Andy Lutomirski
@ 2020-11-13 23:13 ` Thomas Gleixner
2020-11-14 1:05 ` Paul E. McKenney
2020-11-16 16:46 ` #PF " Steven Rostedt
2 siblings, 2 replies; 9+ messages in thread
From: Thomas Gleixner @ 2020-11-13 23:13 UTC (permalink / raw)
To: Peter Zijlstra, Paul McKenney, eupm90
Cc: linux-kernel, x86, Andy Lutomirski, Steven Rostedt
On Fri, Nov 13 2020 at 13:53, Peter Zijlstra wrote:
> [ 139.226724] WARNING: CPU: 9 PID: 2290 at kernel/rcu/tree.c:932 __rcu_irq_enter_check_tick+0x84/0xd0
> [ 139.226753] irqentry_enter+0x25/0x40
> [ 139.226753] exc_page_fault+0x38/0x4c0
> [ 139.226753] asm_exc_page_fault+0x1e/0x30
...
> [ 139.226757] perf_callchain_user+0xf4/0x280
>
> Which is a #PF from NMI context, which is perfectly fine. However
> __rcu_irq_enter_check_tick() is triggering WARN.
>
> AFAICT the right thing is to simply remove the warn like so.
>
> ---
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 430ba58d8bfe..9bda92d8b914 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
> {
> struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>
> - // Enabling the tick is unsafe in NMI handlers.
> - if (WARN_ON_ONCE(in_nmi()))
> + // if we're here from NMI, there's nothing to do.
> + if (in_nmi())
> return;
>
> RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
Yes. That's right.
To answer Pauls question:
> But is a corresponding change required on return-from-NMI side?
> Looks OK to me at first glance, but I could be missing something.
No. The corresponding issue is not return from NMI. The corresponding
problem is the return from the page fault handler, but there is nothing
to worry about. That part is NMI safe already.
And Luto's as well:
> with the following caveat that has nothing to do with NMI: the rest of
> irqentry_enter() has tracing calls in it. Does anything prevent
> infinite recursion if one of those tracing calls causes a page fault?
nmi:
...
trace_hardirqs_off_finish() {
if (!this_cpu_read(tracing_irq_cpu)) {
this_cpu_write(tracing_irq_cpu, 1);
...
}
...
perf()
#PF
save_cr2()
irqentry_enter()
trace_hardirqs_off_finish()
if (!this_cpu_read(tracing_irq_cpu)) {
So yes, it is recursion protected unless I'm missing something.
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: #PF from NMI
2020-11-13 23:13 ` Thomas Gleixner
@ 2020-11-14 1:05 ` Paul E. McKenney
2020-11-16 12:10 ` [PATCH] rcu: Allow rcu_irq_enter_check_tick() " Peter Zijlstra
2020-11-16 16:46 ` #PF " Steven Rostedt
1 sibling, 1 reply; 9+ messages in thread
From: Paul E. McKenney @ 2020-11-14 1:05 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, eupm90, linux-kernel, x86, Andy Lutomirski,
Steven Rostedt
On Sat, Nov 14, 2020 at 12:13:58AM +0100, Thomas Gleixner wrote:
> On Fri, Nov 13 2020 at 13:53, Peter Zijlstra wrote:
> > [ 139.226724] WARNING: CPU: 9 PID: 2290 at kernel/rcu/tree.c:932 __rcu_irq_enter_check_tick+0x84/0xd0
> > [ 139.226753] irqentry_enter+0x25/0x40
> > [ 139.226753] exc_page_fault+0x38/0x4c0
> > [ 139.226753] asm_exc_page_fault+0x1e/0x30
>
> ...
>
> > [ 139.226757] perf_callchain_user+0xf4/0x280
> >
> > Which is a #PF from NMI context, which is perfectly fine. However
> > __rcu_irq_enter_check_tick() is triggering WARN.
> >
> > AFAICT the right thing is to simply remove the warn like so.
> >
> > ---
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 430ba58d8bfe..9bda92d8b914 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
> > {
> > struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> >
> > - // Enabling the tick is unsafe in NMI handlers.
> > - if (WARN_ON_ONCE(in_nmi()))
> > + // if we're here from NMI, there's nothing to do.
> > + if (in_nmi())
> > return;
> >
> > RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
>
> Yes. That's right.
>
> To answer Pauls question:
>
> > But is a corresponding change required on return-from-NMI side?
> > Looks OK to me at first glance, but I could be missing something.
>
> No. The corresponding issue is not return from NMI. The corresponding
> problem is the return from the page fault handler, but there is nothing
> to worry about. That part is NMI safe already.
In that case:
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Or let me know (and get me a Signed-off-by) if you want me to take it.
Thanx, Paul
> And Luto's as well:
>
> > with the following caveat that has nothing to do with NMI: the rest of
> > irqentry_enter() has tracing calls in it. Does anything prevent
> > infinite recursion if one of those tracing calls causes a page fault?
>
> nmi:
> ...
> trace_hardirqs_off_finish() {
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> ...
> }
> ...
> perf()
>
> #PF
> save_cr2()
>
> irqentry_enter()
> trace_hardirqs_off_finish()
> if (!this_cpu_read(tracing_irq_cpu)) {
>
> So yes, it is recursion protected unless I'm missing something.
>
> Thanks,
>
> tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] rcu: Allow rcu_irq_enter_check_tick() from NMI
2020-11-14 1:05 ` Paul E. McKenney
@ 2020-11-16 12:10 ` Peter Zijlstra
2020-11-16 17:24 ` Thomas Gleixner
0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2020-11-16 12:10 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Thomas Gleixner, eupm90, linux-kernel, x86, Andy Lutomirski,
Steven Rostedt
Any which way around; here's a proper patch...
---
Subject: rcu: Allow rcu_irq_enter_check_tick() from NMI
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon Nov 16 12:54:56 CET 2020
Eugenio managed to tickle #PF from NMI context which resulted in
hitting a WARN in RCU through irqentry_enter() ->
__rcu_irq_enter_check_tick().
However, this situation is perfectly sane and does not warrant an
WARN. The #PF will (necessarily) be atomic and not require messing
with the tick state, so early return is correct.
Fixes: aaf2bc50df1f ("rcu: Abstract out rcu_irq_enter_check_tick() from rcu_nmi_enter()")
Reported-by: "Eugenio Pérez" <eupm90@gmail.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
---
kernel/rcu/tree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -928,8 +928,8 @@ void __rcu_irq_enter_check_tick(void)
{
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
- // Enabling the tick is unsafe in NMI handlers.
- if (WARN_ON_ONCE(in_nmi()))
+ // If we're here from NMI there's nothing to do.
+ if (in_nmi())
return;
RCU_LOCKDEP_WARN(rcu_dynticks_curr_cpu_in_eqs(),
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: #PF from NMI
2020-11-13 23:13 ` Thomas Gleixner
2020-11-14 1:05 ` Paul E. McKenney
@ 2020-11-16 16:46 ` Steven Rostedt
1 sibling, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2020-11-16 16:46 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Paul McKenney, eupm90, linux-kernel, x86,
Andy Lutomirski
On Sat, 14 Nov 2020 00:13:58 +0100
Thomas Gleixner <tglx@linutronix.de> wrote:
> nmi:
> ...
> trace_hardirqs_off_finish() {
> if (!this_cpu_read(tracing_irq_cpu)) {
> this_cpu_write(tracing_irq_cpu, 1);
> ...
> }
> ...
> perf()
>
> #PF
> save_cr2()
>
> irqentry_enter()
> trace_hardirqs_off_finish()
> if (!this_cpu_read(tracing_irq_cpu)) {
>
> So yes, it is recursion protected unless I'm missing something.
That should work.
-- Steve
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rcu: Allow rcu_irq_enter_check_tick() from NMI
2020-11-16 12:10 ` [PATCH] rcu: Allow rcu_irq_enter_check_tick() " Peter Zijlstra
@ 2020-11-16 17:24 ` Thomas Gleixner
2020-11-16 20:00 ` Paul E. McKenney
0 siblings, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2020-11-16 17:24 UTC (permalink / raw)
To: Peter Zijlstra, Paul E. McKenney
Cc: eupm90, linux-kernel, x86, Andy Lutomirski, Steven Rostedt
On Mon, Nov 16 2020 at 13:10, Peter Zijlstra wrote:
> Any which way around; here's a proper patch...
>
> ---
>
> Subject: rcu: Allow rcu_irq_enter_check_tick() from NMI
> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon Nov 16 12:54:56 CET 2020
>
> Eugenio managed to tickle #PF from NMI context which resulted in
> hitting a WARN in RCU through irqentry_enter() ->
> __rcu_irq_enter_check_tick().
>
> However, this situation is perfectly sane and does not warrant an
> WARN. The #PF will (necessarily) be atomic and not require messing
> with the tick state, so early return is correct.
>
> Fixes: aaf2bc50df1f ("rcu: Abstract out rcu_irq_enter_check_tick() from rcu_nmi_enter()")
> Reported-by: "Eugenio Pérez" <eupm90@gmail.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Andy Lutomirski <luto@kernel.org>
> Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] rcu: Allow rcu_irq_enter_check_tick() from NMI
2020-11-16 17:24 ` Thomas Gleixner
@ 2020-11-16 20:00 ` Paul E. McKenney
0 siblings, 0 replies; 9+ messages in thread
From: Paul E. McKenney @ 2020-11-16 20:00 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, eupm90, linux-kernel, x86, Andy Lutomirski,
Steven Rostedt
On Mon, Nov 16, 2020 at 06:24:31PM +0100, Thomas Gleixner wrote:
> On Mon, Nov 16 2020 at 13:10, Peter Zijlstra wrote:
>
> > Any which way around; here's a proper patch...
> >
> > ---
> >
> > Subject: rcu: Allow rcu_irq_enter_check_tick() from NMI
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Mon Nov 16 12:54:56 CET 2020
> >
> > Eugenio managed to tickle #PF from NMI context which resulted in
> > hitting a WARN in RCU through irqentry_enter() ->
> > __rcu_irq_enter_check_tick().
> >
> > However, this situation is perfectly sane and does not warrant an
> > WARN. The #PF will (necessarily) be atomic and not require messing
> > with the tick state, so early return is correct.
> >
> > Fixes: aaf2bc50df1f ("rcu: Abstract out rcu_irq_enter_check_tick() from rcu_nmi_enter()")
> > Reported-by: "Eugenio Pérez" <eupm90@gmail.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Reviewed-by: Andy Lutomirski <luto@kernel.org>
> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
Queued, thank you all!
Thanx, Paul
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-16 20:00 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 12:53 #PF from NMI Peter Zijlstra
2020-11-13 17:16 ` Paul E. McKenney
2020-11-13 17:23 ` Andy Lutomirski
2020-11-13 23:13 ` Thomas Gleixner
2020-11-14 1:05 ` Paul E. McKenney
2020-11-16 12:10 ` [PATCH] rcu: Allow rcu_irq_enter_check_tick() " Peter Zijlstra
2020-11-16 17:24 ` Thomas Gleixner
2020-11-16 20:00 ` Paul E. McKenney
2020-11-16 16:46 ` #PF " Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).