Re: [PATCH 6/9] perf: expose perf capability to other modules.

[Frederic Weisbecker <fweisbec@gmail.com>]

On Mon, Nov 07, 2011 at 02:45:17PM +0000, Will Deacon wrote:
> Hi Frederic,
> 
> On Wed, Nov 02, 2011 at 07:42:04AM +0000, Frederic Weisbecker wrote:
> > On Tue, Nov 01, 2011 at 10:20:04AM -0600, David Ahern wrote:
> > > Right. Originally it could be enabled/disabled. Right now it cannot be,
> > > but I believe Frederic is working on making it configurable again.
> > > 
> > > David
> > 
> > Yep. Will Deacon is working on making the breakpoints able to process
> > pure arch informations (ie: without beeing forced to use the perf attr
> > as a midlayer to define them).
> > 
> > Once we have that I can seperate the breakpoints implementation from perf
> > and make it opt-able.
> 
> How do you foresee kdb fitting into this? I see that currently [on x86] we
> cook up perf_event structures with a specific overflow handler set. If we
> want to move this over to using a completely arch-defined structure, then
> we're going to end up with an overflow handler field in both perf_event
> *and* the arch-specific structure, which doesn't feel right to me.
> 
> Of course, if the goal is only to separate ptrace (i.e. user debugging) from
> the perf dependency then we don't need the overflow handler because we'll
> always just send SIGTRAP to the current task.
> 
> Any ideas?

I don't know if we want to convert x86/kgdb to use pure arch breakpoints.
If kgdb one day wants to extend this use to generic code, it may be a good
idea to keep the things as is. I don't know, I'm adding Jason in Cc.

In any case I think we have a problem if we want to default to send a
SIGTRAP. Look at this:

		bp = per_cpu(bp_per_reg[i], cpu);
		/*
		 * Reset the 'i'th TRAP bit in dr6 to denote completion of
		 * exception handling
		 */
		(*dr6_p) &= ~(DR_TRAP0 << i);
		/*
		 * bp can be NULL due to lazy debug register switching
		 * or due to concurrent perf counter removing.
		 */
		if (!bp) {
			rcu_read_unlock();
			break;
		}

		perf_bp_event(bp, args->regs);


I don't have the details about how lazy the debug register switching
can be. And also we want to avoid a locking between the perf event
scheduling (removing) and the breakpoint triggering path.

A solution is to look at the ptrace breakpoints in the thread
struct and see if the one in the index is there. That can reside
in its own callback or as a fallback in hw_breakpoint_handler().
I don't feel that strong with choosing either of those solutions.