Messages in this thread | | | From | Alexey Brodkin <> | Subject | RE: [PATCH v2 3/8] ARCv2: perf: implement "event_set_period" for future use with interrupts | Date | Thu, 20 Aug 2015 11:25:10 +0000 |
| |
Hi Peter,
> -----Original Message----- > From: Peter Zijlstra [mailto:peterz@infradead.org] > Sent: 18 августа 2015 г. 20:55 > To: Alexey Brodkin > Cc: linux-arch@vger.kernel.org; linux-kernel@vger.kernel.org; Vineet.Gupta1@synopsys.com; arc-linux-dev@synopsys.com; > arnd@arndb.de; Arnaldo Carvalho de Melo > Subject: Re: [PATCH v2 3/8] ARCv2: perf: implement "event_set_period" for future use with interrupts > > On Wed, Aug 05, 2015 at 06:13:29PM +0300, Alexey Brodkin wrote: > > +static int arc_pmu_event_set_period(struct perf_event *event) > > +{ > > + struct hw_perf_event *hwc = &event->hw; > > + s64 left = local64_read(&hwc->period_left); > > + s64 period = hwc->sample_period; > > + int idx = hwc->idx; > > + int overflow = 0; > > + u64 value; > > + > > + if (unlikely(left <= -period)) { > > + /* left underflowed by more than period. */ > > + left = period; > > + local64_set(&hwc->period_left, left); > > + hwc->last_period = period; > > + overflow = 1; > > + } else if (unlikely(left <= 0)) { > > + /* left underflowed by less than period. */ > > + left += period; > > + local64_set(&hwc->period_left, left); > > + hwc->last_period = period; > > + overflow = 1; > > + } > > + > > + if (left > arc_pmu->max_period) { > > + left = arc_pmu->max_period; > > + local64_set(&hwc->period_left, left); > > Given that you set counter_size to 32+bct_bcr.s << 4, I'm assuming these > counters are not 64bit wide (or at least the hardware has the option of > not being full width).
Indeed our counters could be 32/48(default)/64 bits wide.
> That means this local64_set() is wrong.
You mean the one used for setting "hwc->period_left"?
> The purpose here is to emulate a longer period with a short counter. So > even though we have to take the interrupt to observe the counter width > overflow and reprogram, we must not decrease the @left value. > > Doing so will trigger one of the above two cases and result in @overflow > == 1, even though we've not actually had hwc->sample_period counts.
My understanding was that here we're just checking if for some reason in arc_perf_event_update() we decremented "hwc->period_left" too much and it became either just <0 or even <(0 - period). IMHO that may happen if not in sampling even case (where we expect interrupt to happen close to a period being crossed) but in case of non-sampling event IMHO that is pretty possible if frequency of checking counter value is way too low.
-Alexey
| |