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    [W:0.043 / U:63.500 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site