lkml.org 
[lkml]   [2013]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: perf: PERF_EVENT_IOC_PERIOD on ARM vs everywhere else
    On Wed, Oct 30, 2013 at 09:56:15AM +0000, Peter Zijlstra wrote:
    > On Tue, Oct 29, 2013 at 11:36:52AM -0400, Vince Weaver wrote:
    > > It is true the current behavior is unexpected. What was the logic behind
    > > deferring to the next overflow for the update? Was it a code simplicity
    > > thing? Or were there hardware reasons behind it?
    >
    > Mostly an oversight I think. The delay is simply how it worked out in
    > that the arch code has to reload the period once an event fires in order
    > to reprogram. Since nobody actually used the thing, nobody had
    > experience with it.
    >
    > Now it turns out someone had a complaint but hid it somewhere on some
    > obscure list :-(
    >
    > There is actually generic code that force resets the period; see
    > perf_event_period().
    >
    > > Definitely when an event is stopped, it makes more sense for
    > > PERF_EVENT_IOC_PERIOD to take place immediately.
    > >
    > > I'm not sure what happens if we try to use it on a running event,
    > > especially if we've already passed the new period value.
    >
    > The below code should deal with both cases I think -- completely
    > untested.

    [...]

    > diff --git a/kernel/events/core.c b/kernel/events/core.c
    > index 17b3c6cf1606..c45d53e561da 100644
    > --- a/kernel/events/core.c
    > +++ b/kernel/events/core.c
    > @@ -3530,7 +3530,7 @@ static void perf_event_for_each(struct perf_event *event,
    > static int perf_event_period(struct perf_event *event, u64 __user *arg)
    > {
    > struct perf_event_context *ctx = event->ctx;
    > - int ret = 0;
    > + int ret = 0, active;
    > u64 value;
    >
    > if (!is_sampling_event(event))
    > @@ -3554,6 +3554,20 @@ static int perf_event_period(struct perf_event *event, u64 __user *arg)
    > event->attr.sample_period = value;
    > event->hw.sample_period = value;
    > }
    > +
    > + active = (event->state == PERF_EVENT_STATE_ACTIVE);
    > + if (active) {
    > + perf_pmu_disable(ctx->pmu);
    > + event->pmu->stop(event, PERF_EF_UPDATE);
    > + }
    > +
    > + local64_set(event->hw.period_left, 0);

    Adding the missing '&' here, this patch does what's expected for ARM (i.e.
    Vince's ioctl_period test still fails).

    Tested-by: Will Deacon <will.deacon@arm.com>

    Will

    \
     
     \ /
      Last update: 2013-11-05 14:41    [W:3.727 / U:0.356 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site