lkml.org 
[lkml]   [2012]   [Feb]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH] perf_events: fix broken intr throttling (v3)
    From
    On Wed, Feb 8, 2012 at 12:32 PM, Anton Blanchard <anton@samba.org> wrote:
    >
    > Hi,
    >
    > On Thu, 26 Jan 2012 17:03:19 +0100
    > Stephane Eranian <eranian@google.com> wrote:
    >
    >> This patch fixes the throttling mechanism. It was broken
    >> in 3.2.0. Events were not being unthrottled. The unthrottling
    >> mechanism required that events be checked at each timer tick.
    >
    > This patch breaks perf on POWER. I haven't had a chance to work out why
    > yet, but will investigate tomorrow.
    >
    Did you apply the subsequent patch I posted yesterday:

    https://lkml.org/lkml/2012/2/7/185

    > Anton
    >
    >> This patch solves this problem and also separates:
    >>   - unthrottling
    >>   - multiplexing
    >>   - frequency-mode period adjustments
    >>
    >> Not all of them need to be executed at each timer tick.
    >>
    >> This third version of the patch is based on my original patch +
    >> PeterZ proposal (https://lkml.org/lkml/2012/1/7/87).
    >>
    >> At each timer tick, for each context:
    >>   - if the current CPU has throttled events, we unthrottle events
    >>
    >>   - if context has frequency-based events, we adjust sampling periods
    >>
    >>   - if we have reached the jiffies interval, we multiplex (rotate)
    >>
    >> We decoupled rotation (multiplexing) from frequency-mode sampling
    >> period adjustments.  They should not necessarily happen at the same
    >> rate. Multiplexing is subject to jiffies_interval (currently at 1
    >> but could be higher once the tunable is exposed via sysfs).
    >>
    >> We have grouped frequency-mode adjustment and unthrottling into the
    >> same routine to minimize code duplication. When throttled while in
    >> frequency mode, we scan the events only once.
    >>
    >> We have fixed the threshold enforcement code in
    >> __perf_event_overflow(). There was a bug whereby it would allow more
    >> than the authorized rate because an increment of hwc->interrupts was
    >> not executed at the right place.
    >>
    >> The patch was tested with low sampling limit (2000) and fixed periods,
    >> frequency mode, overcommitted PMU.
    >>
    >> On a 2.1GHz AMD CPU:
    >> $ cat /proc/sys/kernel/perf_event_max_sample_rate
    >> 2000
    >>
    >> We set a rate of 3000 samples/sec (2.1GHz/3000 = 700000):
    >>
    >> $ perf record -e cycles,cycles -c 700000  noploop 10
    >> $ perf report -D | tail -21
    >> Aggregated stats:
    >>            TOTAL events:      80086
    >>             MMAP events:         88
    >>             COMM events:          2
    >>             EXIT events:          4
    >>         THROTTLE events:      19996
    >>       UNTHROTTLE events:      19996
    >>           SAMPLE events:      40000
    >> cycles stats:
    >>            TOTAL events:      40006
    >>             MMAP events:          5
    >>             COMM events:          1
    >>             EXIT events:          4
    >>         THROTTLE events:       9998
    >>       UNTHROTTLE events:       9998
    >>           SAMPLE events:      20000
    >> cycles stats:
    >>            TOTAL events:      39996
    >>         THROTTLE events:       9998
    >>       UNTHROTTLE events:       9998
    >>           SAMPLE events:      20000
    >>
    >> For 10s, the cap is 2x2000x10 = 40000 samples.
    >> We get exactly that: 20000 samples/event.
    >>
    >> Signed-off-by: Stephane Eranian <eranian@google.com>
    >> ---
    >>
    >> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
    >> index 0b91db2..412b790 100644
    >> --- a/include/linux/perf_event.h
    >> +++ b/include/linux/perf_event.h
    >> @@ -589,6 +589,7 @@ struct hw_perf_event {
    >>       u64                             sample_period;
    >>       u64                             last_period;
    >>       local64_t                       period_left;
    >> +     u64                             interrupts_seq;
    >>       u64                             interrupts;
    >>
    >>       u64                             freq_time_stamp;
    >> diff --git a/kernel/events/core.c b/kernel/events/core.c
    >> index de859fb..7c3b9de 100644
    >> --- a/kernel/events/core.c
    >> +++ b/kernel/events/core.c
    >> @@ -2300,6 +2300,9 @@ do {                                    \
    >>       return div64_u64(dividend, divisor);
    >>  }
    >>
    >> +static DEFINE_PER_CPU(int, perf_throttled_count);
    >> +static DEFINE_PER_CPU(u64, perf_throttled_seq);
    >> +
    >>  static void perf_adjust_period(struct perf_event *event, u64 nsec,
    >> u64 count) {
    >>       struct hw_perf_event *hwc = &event->hw;
    >> @@ -2325,16 +2328,29 @@ static void perf_adjust_period(struct
    >> perf_event *event, u64 nsec, u64 count) }
    >>  }
    >>
    >> -static void perf_ctx_adjust_freq(struct perf_event_context *ctx, u64
    >> period) +/*
    >> + * combine freq adjustment with unthrottling to avoid two passes
    >> over the
    >> + * events. At the same time, make sure, having freq events does not
    >> change
    >> + * the rate of unthrottling as that would introduce bias.
    >> + */
    >> +static void perf_adjust_freq_unthr_context(struct perf_event_context
    >> *ctx,
    >> +                                        int needs_unthr)
    >>  {
    >>       struct perf_event *event;
    >>       struct hw_perf_event *hwc;
    >> -     u64 interrupts, now;
    >> +     u64 now, period = TICK_NSEC;
    >>       s64 delta;
    >>
    >> -     if (!ctx->nr_freq)
    >> +     /*
    >> +      * only need to iterate over all events iff:
    >> +      * - context have events in frequency mode (needs freq
    >> adjust)
    >> +      * - there are events to unthrottle on this cpu
    >> +      */
    >> +     if (!(ctx->nr_freq || needs_unthr))
    >>               return;
    >>
    >> +     raw_spin_lock(&ctx->lock);
    >> +
    >>       list_for_each_entry_rcu(event, &ctx->event_list,
    >> event_entry) { if (event->state != PERF_EVENT_STATE_ACTIVE)
    >>                       continue;
    >> @@ -2344,13 +2360,8 @@ static void perf_ctx_adjust_freq(struct
    >> perf_event_context *ctx, u64 period)
    >>               hwc = &event->hw;
    >>
    >> -             interrupts = hwc->interrupts;
    >> -             hwc->interrupts = 0;
    >> -
    >> -             /*
    >> -              * unthrottle events on the tick
    >> -              */
    >> -             if (interrupts == MAX_INTERRUPTS) {
    >> +             if (needs_unthr && hwc->interrupts ==
    >> MAX_INTERRUPTS) {
    >> +                     hwc->interrupts = 0;
    >>                       perf_log_throttle(event, 1);
    >>                       event->pmu->start(event, 0);
    >>               }
    >> @@ -2358,14 +2369,26 @@ static void perf_ctx_adjust_freq(struct
    >> perf_event_context *ctx, u64 period) if (!event->attr.freq
    >> || !event->attr.sample_freq) continue;
    >>
    >> -             event->pmu->read(event);
    >> +             /*
    >> +              * stop the event and update event->count
    >> +              */
    >> +             event->pmu->stop(event, PERF_EF_UPDATE);
    >> +
    >>               now = local64_read(&event->count);
    >>               delta = now - hwc->freq_count_stamp;
    >>               hwc->freq_count_stamp = now;
    >>
    >> +             /*
    >> +              * restart the event
    >> +              * reload only if value has changed
    >> +              */
    >>               if (delta > 0)
    >>                       perf_adjust_period(event, period, delta);
    >> +
    >> +             event->pmu->start(event, delta > 0 ?
    >> PERF_EF_RELOAD : 0); }
    >> +
    >> +     raw_spin_unlock(&ctx->lock);
    >>  }
    >>
    >>  /*
    >> @@ -2388,16 +2411,13 @@ static void rotate_ctx(struct
    >> perf_event_context *ctx) */
    >>  static void perf_rotate_context(struct perf_cpu_context *cpuctx)
    >>  {
    >> -     u64 interval = (u64)cpuctx->jiffies_interval * TICK_NSEC;
    >>       struct perf_event_context *ctx = NULL;
    >> -     int rotate = 0, remove = 1, freq = 0;
    >> +     int rotate = 0, remove = 1;
    >>
    >>       if (cpuctx->ctx.nr_events) {
    >>               remove = 0;
    >>               if (cpuctx->ctx.nr_events != cpuctx->ctx.nr_active)
    >>                       rotate = 1;
    >> -             if (cpuctx->ctx.nr_freq)
    >> -                     freq = 1;
    >>       }
    >>
    >>       ctx = cpuctx->task_ctx;
    >> @@ -2405,37 +2425,26 @@ static void perf_rotate_context(struct
    >> perf_cpu_context *cpuctx) remove = 0;
    >>               if (ctx->nr_events != ctx->nr_active)
    >>                       rotate = 1;
    >> -             if (ctx->nr_freq)
    >> -                     freq = 1;
    >>       }
    >>
    >> -     if (!rotate && !freq)
    >> +     if (!rotate)
    >>               goto done;
    >>
    >>       perf_ctx_lock(cpuctx, cpuctx->task_ctx);
    >>       perf_pmu_disable(cpuctx->ctx.pmu);
    >>
    >> -     if (freq) {
    >> -             perf_ctx_adjust_freq(&cpuctx->ctx, interval);
    >> -             if (ctx)
    >> -                     perf_ctx_adjust_freq(ctx, interval);
    >> -     }
    >> -
    >> -     if (rotate) {
    >> -             cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
    >> -             if (ctx)
    >> -                     ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE);
    >> +     cpu_ctx_sched_out(cpuctx, EVENT_FLEXIBLE);
    >> +     if (ctx)
    >> +             ctx_sched_out(ctx, cpuctx, EVENT_FLEXIBLE);
    >>
    >> -             rotate_ctx(&cpuctx->ctx);
    >> -             if (ctx)
    >> -                     rotate_ctx(ctx);
    >> +     rotate_ctx(&cpuctx->ctx);
    >> +     if (ctx)
    >> +             rotate_ctx(ctx);
    >>
    >> -             perf_event_sched_in(cpuctx, ctx, current);
    >> -     }
    >> +     perf_event_sched_in(cpuctx, ctx, current);
    >>
    >>       perf_pmu_enable(cpuctx->ctx.pmu);
    >>       perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
    >> -
    >>  done:
    >>       if (remove)
    >>               list_del_init(&cpuctx->rotation_list);
    >> @@ -2445,10 +2454,22 @@ void perf_event_task_tick(void)
    >>  {
    >>       struct list_head *head = &__get_cpu_var(rotation_list);
    >>       struct perf_cpu_context *cpuctx, *tmp;
    >> +     struct perf_event_context *ctx;
    >> +     int throttled;
    >>
    >>       WARN_ON(!irqs_disabled());
    >>
    >> +     __this_cpu_inc(perf_throttled_seq);
    >> +     throttled = __this_cpu_xchg(perf_throttled_count, 0);
    >> +
    >>       list_for_each_entry_safe(cpuctx, tmp, head, rotation_list) {
    >> +             ctx = &cpuctx->ctx;
    >> +             perf_adjust_freq_unthr_context(ctx, throttled);
    >> +
    >> +             ctx = cpuctx->task_ctx;
    >> +             if (ctx)
    >> +                     perf_adjust_freq_unthr_context(ctx,
    >> throttled); +
    >>               if (cpuctx->jiffies_interval == 1 ||
    >>                               !(jiffies %
    >> cpuctx->jiffies_interval)) perf_rotate_context(cpuctx);
    >> @@ -4514,6 +4535,7 @@ static int __perf_event_overflow(struct
    >> perf_event *event, {
    >>       int events = atomic_read(&event->event_limit);
    >>       struct hw_perf_event *hwc = &event->hw;
    >> +     u64 seq;
    >>       int ret = 0;
    >>
    >>       /*
    >> @@ -4523,14 +4545,20 @@ static int __perf_event_overflow(struct
    >> perf_event *event, if (unlikely(!is_sampling_event(event)))
    >>               return 0;
    >>
    >> -     if (unlikely(hwc->interrupts >= max_samples_per_tick)) {
    >> -             if (throttle) {
    >> +     seq = __this_cpu_read(perf_throttled_seq);
    >> +     if (seq != hwc->interrupts_seq) {
    >> +             hwc->interrupts_seq = seq;
    >> +             hwc->interrupts = 1;
    >> +     } else {
    >> +             hwc->interrupts++;
    >> +             if (unlikely(throttle
    >> +                          && hwc->interrupts >=
    >> max_samples_per_tick)) {
    >> +                     __this_cpu_inc(perf_throttled_count);
    >>                       hwc->interrupts = MAX_INTERRUPTS;
    >>                       perf_log_throttle(event, 0);
    >>                       ret = 1;
    >>               }
    >> -     } else
    >> -             hwc->interrupts++;
    >> +     }
    >>
    >>       if (event->attr.freq) {
    >>               u64 now = perf_clock();
    >> --
    >> To unsubscribe from this list: send the line "unsubscribe
    >> linux-kernel" in the body of a message to majordomo@vger.kernel.org
    >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
    >> Please read the FAQ at  http://www.tux.org/lkml/
    >>
    >
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-02-08 12:45    [W:0.050 / U:31.304 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site