Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1348928
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc |
| Date | 2016-03-03 10:30 +0100 |
| Message-ID | <r8xK2-4Sk-5@gated-at.bofh.it> (permalink) |
| References | <r8bJw-5SA-11@gated-at.bofh.it> <r8dBD-75k-13@gated-at.bofh.it> <r8dBE-75k-27@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, 2016-03-02 at 12:59 +0100, Peter Zijlstra wrote:
> On Wed, Mar 02, 2016 at 10:53:24PM +1100, Michael Ellerman wrote:
> > Peterz, acme, do you guys want to take this? Or should I?
>
> I'm not too happy its touching event->ctx at all. It really should not
> be doing that.
Hmm OK.
It's been using ctx->task since it was merged in 2010. In fact that commit also
added arch_unregister_hw_breakpoint(), and we're still the only user of that.
The prima facie reason it's using ctx is to get at task->thread to clear
last_hit_ubp.
It looks like other arches avoid needing to do something similar by storing the
break point in a per-cpu array. Which I guess is what you meant in your other
mail ("Why do you keep per task state anyway?").
I can't think of a reason why we can't also store it per-cpu, but I could be
wrong, I don't know the code well and I haven't thought about it for very long.
Do you mind if I merge the following fix for now as a band-aid, and we'll try
and fix it up properly in the next few weeks (but maybe not in time for 4.5
final).
cheers
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 05e804cdecaa..aec9a1b1d25b 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -109,8 +109,9 @@ void arch_unregister_hw_breakpoint(struct perf_event *bp)
* If the breakpoint is unregistered between a hw_breakpoint_handler()
* and the single_step_dabr_instruction(), then cleanup the breakpoint
* restoration variables to prevent dangling pointers.
+ * FIXME, this should not be using bp->ctx at all! Sayeth peterz.
*/
- if (bp->ctx && bp->ctx->task)
+ if (bp->ctx && bp->ctx->task && bp->ctx->task != ((void *)-1L))
bp->ctx->task->thread.last_hit_ubp = NULL;
}
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> - 2016-03-02 11:00 +0100
Re: [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-02 12:50 +0100
Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-02 13:00 +0100
Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-02 13:00 +0100
Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-03 10:30 +0100
Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Peter Zijlstra <peterz@infradead.org> - 2016-03-03 11:30 +0100
Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Michael Ellerman <mpe@ellerman.id.au> - 2016-03-04 09:50 +0100
csiph-web