lkml.org 
[lkml]   [2023]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH 11/13] perf: Simplify perf_adjust_freq_unthr_context()
    On Thu, Nov 2, 2023 at 8:32 AM Peter Zijlstra <peterz@infradead.org> wrote:
    >
    >
    > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    > ---
    > kernel/events/core.c | 51 +++++++++++++++++++++++----------------------------
    > 1 file changed, 23 insertions(+), 28 deletions(-)
    >
    > --- a/kernel/events/core.c
    > +++ b/kernel/events/core.c
    > @@ -4090,7 +4090,7 @@ perf_adjust_freq_unthr_context(struct pe
    > if (!(ctx->nr_freq || unthrottle))
    > return;
    >
    > - raw_spin_lock(&ctx->lock);
    > + guard(raw_spinlock)(&ctx->lock);
    >
    > list_for_each_entry_rcu(event, &ctx->event_list, event_entry) {
    > if (event->state != PERF_EVENT_STATE_ACTIVE)
    > @@ -4100,7 +4100,7 @@ perf_adjust_freq_unthr_context(struct pe
    > if (!event_filter_match(event))
    > continue;
    >
    > - perf_pmu_disable(event->pmu);
    > + guard(perf_pmu_disable)(event->pmu);
    >
    > hwc = &event->hw;
    >
    > @@ -4110,34 +4110,29 @@ perf_adjust_freq_unthr_context(struct pe
    > event->pmu->start(event, 0);
    > }
    >
    > - if (!event->attr.freq || !event->attr.sample_freq)
    > - goto next;
    > + if (event->attr.freq && event->attr.sample_freq) {

    Any reason for this change? I think we can just change the
    'goto next' to 'continue', no?

    Also I think this code needs changes to optimize the access.
    A similar reason for the cgroup switch, it accesses all the
    pmu/events in the context even before checking the sampling
    frequency. This is bad for uncore PMUs (and KVM too).

    But this is a different issue..

    Thanks,
    Namhyung


    > + /*
    > + * 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
    > + * we have stopped the event so tell that
    > + * to perf_adjust_period() to avoid stopping it
    > + * twice.
    > + */
    > + if (delta > 0)
    > + perf_adjust_period(event, period, delta, false);
    >
    > - /*
    > - * 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
    > - * we have stopped the event so tell that
    > - * to perf_adjust_period() to avoid stopping it
    > - * twice.
    > - */
    > - if (delta > 0)
    > - perf_adjust_period(event, period, delta, false);
    > -
    > - event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
    > - next:
    > - perf_pmu_enable(event->pmu);
    > + event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
    > + }
    > }
    > -
    > - raw_spin_unlock(&ctx->lock);
    > }
    >
    > /*
    >
    >

    \
     
     \ /
      Last update: 2023-11-20 13:48    [W:3.227 / U:0.476 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site