ARM64: smp: Silence suspicious RCU usage with ipi tracepoints

PATCH In review Stephen Boyd 6 participants 8 messages Jun 24, 2015 – Jun 29, 2015 arm64

lore

Share & tools

lore

Stephen Boyd

author

2015-06-24 20:14 UTC #0 lore raw

PATCH 1/1 ARM64: smp: Silence suspicious RCU usage with ipi tracepoints

John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.

At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.

Cc John Stultz <[email protected]>

Cc Steven Rostedt <[email protected]>

Cc Paul E. McKenney <[email protected]>

Cc Nicolas Pitre <[email protected]>

Fixes 45ed695ac10a "ARM64: add IPI tracepoints"

Signed-off-by Stephen Boyd <[email protected]>

---

I haven't confirmed this, but it looks an awful lot like the same problem exists on ARM64.

 arch/arm64/kernel/smp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/kernel/smp.c +2 -2

index 4b2121bd7f9c..a1883bfdd9d6 100644
@@ -693,7 +693,7 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
 	if ((unsigned)ipinr < NR_IPI) {
-		trace_ipi_entry(ipi_types[ipinr]);
+		trace_ipi_entry_rcuidle(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
 	}

@@ -736,7 +736,7 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	}
 
 	if ((unsigned)ipinr < NR_IPI)
-		trace_ipi_exit(ipi_types[ipinr]);
+		trace_ipi_exit_rcuidle(ipi_types[ipinr]);
 	set_irq_regs(old_regs);
 }

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Paul E. McKenney

Reviewed-by on v1 1/1

2015-06-24 20:20 UTC #1 lore raw

Quoted 9 lines · Stephen Boyd

John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.

Quoted 6 lines

Cc John Stultz <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Nicolas Pitre <[email protected]>
Fixes 45ed695ac10a "ARM64: add IPI tracepoints"
Signed-off-by Stephen Boyd <[email protected]>

Reviewed-by Paul E. McKenney <[email protected]>

Quoted 34 lines

---

I haven't confirmed this, but it looks an awful lot like the same problem exists on ARM64.

 arch/arm64/kernel/smp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm64/kernel/smp.c +2 -2

index 4b2121bd7f9c..a1883bfdd9d6 100644
@@ -693,7 +693,7 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
 	if ((unsigned)ipinr < NR_IPI) {
-		trace_ipi_entry(ipi_types[ipinr]);
+		trace_ipi_entry_rcuidle(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
 	}

@@ -736,7 +736,7 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	}
 
 	if ((unsigned)ipinr < NR_IPI)
-		trace_ipi_exit(ipi_types[ipinr]);
+		trace_ipi_exit_rcuidle(ipi_types[ipinr]);
 	set_irq_regs(old_regs);
 }

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Peter Zijlstra

2015-06-24 21:29 UTC #2 lore raw

In reply to Stephen Boyd

Quoted 8 lines · Stephen Boyd

John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.

I have a problem with $subject. It says 'silence', whereas afaict this fixes an actual bug, so it should be 'fixes'.

Stephen Boyd

author

2015-06-24 21:59 UTC #3 lore raw

Quoted 11 lines · Peter Zijlstra

Quoted 8 lines · Stephen Boyd
John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.
I have a problem with $subject. It says 'silence', whereas afaict this fixes an actual bug, so it should be 'fixes'.

Fair enough. I can resend or whoever applies this patch can s/Silence/Fix/ on the $subject.

Signature · 2 lines

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Catalin Marinas

maintainer

2015-06-25 13:24 UTC #4 lore raw

Quoted 10 lines · Stephen Boyd

Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Stephen Boyd
John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.

Quoted 5 lines

Quoted 2 lines
I have a problem with $subject. It says 'silence', whereas afaict this fixes an actual bug, so it should be 'fixes'.
Fair enough. I can resend or whoever applies this patch can s/Silence/Fix/ on the $subject.

I'll apply the patch and change the subject as well.

Thanks.

Signature · 1 line

Catalin

Steven Rostedt

2015-06-25 01:01 UTC #5 lore raw

In reply to Peter Zijlstra

Quoted 4 lines · On Wed, 24 Jun 2015 23:29:30 +0200 Peter Zijlstra <[email protected]> wrote:

Quoted 3 lines · Stephen Boyd
John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.

Quoted 8 lines

Quoted 5 lines
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.
I have a problem with $subject. It says 'silence', whereas afaict this fixes an actual bug, so it should be 'fixes'.

Agreed, otherwise Acked-by: Steven Rostedt <[email protected]>

-- Steve

Russell King - ARM Linux

2015-06-29 10:02 UTC #6 lore raw

In reply to Stephen Boyd

Quoted 9 lines · Stephen Boyd

John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.

Quoted 6 lines

Cc John Stultz <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Nicolas Pitre <[email protected]>
Fixes 45ed695ac10a "ARM64: add IPI tracepoints"
Signed-off-by Stephen Boyd <[email protected]>

I can find no trace of your equivalent ARM patch in my mailbox, but my comment is as relevant to that message as it is to this one.

The Fixes: line has a standard format, which is not the formatting above. Please see Documentation/SubmittingPatches section 2. Thanks.

I've fixed it when applying the ARM version of this patch. However, I'm left wondering whether the lack of update to change the "Silence" to "Fix" which I _stumbled_ over later in this thread is laziness on your part - had it not been for the malformed Fixes: line, I would not have known to update that.

We all gets lots of email every day, expecting people to remember some comment in the dim and distant past made to a similar but different patch is not really on. Please ensure that you update your patches in future.

Signature · 2 lines

FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Catalin Marinas

maintainer

2015-06-29 11:23 UTC #7 lore raw

Quoted 15 lines · Russell King - ARM Linux

Quoted 14 lines · Stephen Boyd
John Stultz reported an RCU splat on ARM with ipi trace events enabled. It looks like the same problem exists on ARM64.
At this point in the IPI handling path we haven't called irq_enter() yet, so RCU doesn't know that we're about to exit idle and properly warns that we're using RCU from an idle CPU. Use trace_ipi_entry_rcuidle() instead of trace_ipi_entry() so that RCU is informed about our exit from idle.
Cc John Stultz <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Paul E. McKenney <[email protected]>
Cc Nicolas Pitre <[email protected]>
Fixes 45ed695ac10a "ARM64: add IPI tracepoints"

[...]

Quoted 2 lines

The Fixes: line has a standard format, which is not the formatting above. Please see Documentation/SubmittingPatches section 2. Thanks.

I re-wrote it in this instance. What's not clear to me is whether cc:
stable is still necessary with a Fixes: tag (I add it just in case).

Anyway, to simplify the process, I came up with a .gitconfig alias:

[alias]

        show-fixes = !git show --pretty=format:\"Fixes: %h (\\\"%s\\\")\nCc: <[email protected]> "#" $(git describe --match="v*" --contains $1 | cut -d - -f -1)+\n\n%b\" $1 "#"

And another for .vimrc (to replace a Fixes: line):

command! GitFixes
\ :.!read tagname commit; git show-fixes $commit 2> /dev/null | head -n 2

Signature · 1 line

Catalin