lkml.org 
[lkml]   [2024]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH V3 1/2] powercap: intel_rapl: Introduce APIs for PMU support
Date
> > +static u64 rapl_event_update(struct perf_event *event)
> > +{
> > +       struct hw_perf_event *hwc = &event->hw;
> > +       struct rapl_package_pmu_data *data =
> > event_to_pmu_data(event);
> > +       u64 prev_raw_count, new_raw_count;
> > +       s64 delta, sdelta;
> > +       s64 tmp;
> > +
> > +       do {
> > +               prev_raw_count = local64_read(&hwc->prev_count);
> > +               new_raw_count = event_read_counter(event);
> > +               tmp = local64_cmpxchg(&hwc->prev_count,
> > prev_raw_count, new_raw_count);
> > +       } while (tmp != prev_raw_count);
>
> I think that it is only safe to call this function for draining an
> event going away, because otherwise the above may turn into an
> endless
> loop, and the function is called under a spinlock.

IMO, the logic above is needed by the hardware events that can generate
interrupts and update hwc->prev_count in the interrupt handler.

For the other PMUs, I suspect this loop actually makes any difference.
However, currently, this is the common logic followed by all PMU
drivers.

>
> I would add a comment (above the loop) explaining that this is about
> draining, so the counter is expected to stop incrementing shortly.
>
> The rest of the patch LGTM.
>

Thanks,
rui
\
 
 \ /
  Last update: 2024-04-28 11:36    [W:0.055 / U:2.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site