lkml.org 
[lkml]   [2009]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [GIT PULL v6] hw-breakpoints: Rewrite on top of perf events v6
On Tue, Nov 24, 2009 at 06:51:27PM +0530, K.Prasad wrote:
> On Tue, Nov 24, 2009 at 11:13:42AM +0100, Ingo Molnar wrote:
> >
> > * K.Prasad <prasad@linux.vnet.ibm.com> wrote:
> >
> > >
> > > Hi Frederic, Ingo,
> > > Here are a few concerns (roughly in decreasing order of
> > > priority) about the perf-events integrated hw-breakpoint feature.
> > >
> > > - Freeze the breakpoint interfaces: Owing to the many
> > > current/potential users of hw-breakpoint feature it is important to
> > > provide a stable interface to the end-user. Changes underneath the
> > > interface can be done in due course in a manner that does not affect
> > > the end-user's behaviour or function signature. The present breakpoint
> > > interface requires parameters that are best embedded in a structure
> > > for extensibility.
> >
> > Well we have PERF_TYPE_BREAKPOINT right now. I agree that it should be
> > finalized in some sort of extensible ABI real soon - we dont want (and
> > dont need to) add all features that might be possible in the future.
> >
>
> It is not about implementing futuristic features, but provide an
> interface which we know isn't going to change in the near future and
> will be flexible to accommodate arch-specific requirements. For
> instance the register_wide_hw_breakpoint() has an interface as below:
>
> struct perf_event **
> register_wide_hw_breakpoint(unsigned long addr,
> int len,
> int type,
> perf_callback_t triggered,
> bool active)
>
> Given the diversity seen in debug registers across processors, it isn't
> prudent to demand/limit the parameters required to those seen above.
> It can be made a part of one of perf-events' structures (with some fields
> in arch-specific structures) and the ABI can accept a pointer to one
> such structure.
>
> In this way it would be easy to bring-in arch-specific quirks without
> altering the interface's signature.



Sure, I plan to convert all these parameters into a single one:
perf_event_attr.


> > > - Proposed migration of register allocation logic to arch-specific
> > > files from kernel/hw_breakpoint.c. This is best done early to help
> > > easy porting of code to other architectures (we have an active
> > > interest in bringing support for PPC64 and S390). If done later, it
> > > will entail additional effort in porting for each architecture.
> >
> > I think the general direction should be towards librarized common
> > frameworks.
> >
> > If an architecture wants to do something special it should either extend
> > the core code, or, if it's too weird to be added to the core, override
> > it via its own implementation.
> >
>
> Given the feeling that the generic set of constraints in the re-written
> kernel/hw_breakpoint.c cannot accommodate the needs of various
> processors (LKML ref:20091117013959.GG5293@nowher) and that
> the register allocation logic should move to arch-specific code, it is
> best done early to help easy porting for other archs. For instance
> there's already a port to PPC64 against the layered hw-breakpoint
> (found here: 20090903183930.GA4590@in.ibm.com) and one from the
> community for SH (20091018062558.GA20535@linux-sh.org).
>
> If such code migration is done while porting of a new architecture, then
> it involves making changes to every other arch on which it is previously
> implemented (or workaround using #ifdef).


As I said, we can probably workaround it by keeping the most part
in the generic code and delegate special arch things to arch
constraints.


> > > - Fix ptrace bugs that potentially alter the semantics of ptrace.
> >
> > Is there a specific list of these bugs?
> >
>
> As pointed out in 20091111130207.GA5676@in.ibm.com and
> 20091112042502.GA3145@in.ibm.com, ptrace requests can a) lose register
> slots when modifying the breakpoint addresses and b) new implementation
> assumes that every DR7 write to be preceded by a write on DR0-DR3 which
> need not be true.



The a) case is going to be fixed.
But the b) situation must be reported as a user mistake (which is what is
done currently): -EINVAL, -EIO or whatever. Enabling a breakpoint without
having given an address is a userland bug.



> > > - Bring either true system_wide support or atleast workaround the
> > > side-effects of iterative per-cpu registration using single atomic
> > > enablement of all per-cpu breakpoints. This can avoid stray exceptions
> > > which would get delivered to the end-user even for failed breakpoint
> > > requests.
> >
> > That can certainly be done when users of such facilities emerge. Right
> > now we have perf and ptrace as the two users - are they affected by
> > these problems?
> >
>
> ksym_tracer - the ftrace plugin (kernel/trace/trace_ksym.c) using
> hw-breakpoints will be affected. Spurious exceptions due to partially
> registered breakpoint requests can be dangerous here.


Will be fixed too.



\
 
 \ /
  Last update: 2009-11-26 07:01    [from the cache]
©2003-2011 Jasper Spaans