public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Michael Ellerman <mpe@ellerman.id.au>
Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	acme@kernel.org, mingo@redhat.com, paulus@samba.org
Subject: Re: hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc
Date: Thu, 3 Mar 2016 11:20:02 +0100	[thread overview]
Message-ID: <20160303102002.GK6356@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1456997018.335.7.camel@ellerman.id.au>

On Thu, Mar 03, 2016 at 08:23:38PM +1100, Michael Ellerman wrote:
> 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.

Yes, I saw that.

> The prima facie reason it's using ctx is to get at task->thread to clear
> last_hit_ubp.

Indeed, but if there's a preemption point in between setting and using
that state, the ctx->task pointer might not actually still point to the
same task. With inherited events the event might get swapped to the next
task if it has the exact same (inherited) event configuration instead of
reprogramming the hardware.

And if there's no preemption point in between then:

> 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?").

this seems possible. And indeed, that was part of that question. The
other part was wondering how per-cpu breakpoints were treated, although
for those the per-cpu storage thing is an 'obvious' solution.

> 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.

Right, so I'm not really up to snuff on the whole hw_breakpoint stuff
either, that was bolted onto perf by mingo, fweisbec, kprasad and others
while I was doing PMU bits, and I've never really dug into it.

I understand kprasad is no longer with IBM, his email bounced. That's a
shame because he knew about this stuff.. :/

> 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).

OK, that works for me.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

  reply	other threads:[~2016-03-03 10:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-02  9:55 [PATCH] hw_breakpoint: Fix Oops at destroying hw_breakpoint event on powerpc Ravi Bangoria
2016-03-02 11:44 ` Peter Zijlstra
2016-03-02 11:53 ` Michael Ellerman
2016-03-02 11:59   ` Peter Zijlstra
2016-03-03  9:23     ` Michael Ellerman
2016-03-03 10:20       ` Peter Zijlstra [this message]
2016-03-04  8:43         ` Michael Ellerman
2016-03-05 22:29 ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160303102002.GK6356@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=acme@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --cc=paulus@samba.org \
    --cc=ravi.bangoria@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox