Messages in this thread | | | Date | Tue, 21 Nov 2023 16:17:42 +0000 | From | Will Deacon <> | Subject | Re: [PATCH 07/34] perf/arm: optimize opencoded atomic find_bit() API |
| |
On Tue, Nov 21, 2023 at 08:16:13AM -0800, Yury Norov wrote: > On Tue, Nov 21, 2023 at 03:53:44PM +0000, Will Deacon wrote: > > On Sat, Nov 18, 2023 at 07:50:38AM -0800, Yury Norov wrote: > > > Switch subsystem to use atomic find_bit() or atomic iterators as > > > appropriate. > > > > > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > > --- > > > drivers/perf/arm-cci.c | 23 +++++------------------ > > > drivers/perf/arm-ccn.c | 10 ++-------- > > > drivers/perf/arm_dmc620_pmu.c | 9 ++------- > > > drivers/perf/arm_pmuv3.c | 8 ++------ > > > 4 files changed, 11 insertions(+), 39 deletions(-) > > > > > > diff --git a/drivers/perf/arm-cci.c b/drivers/perf/arm-cci.c > > > index 61de861eaf91..70fbf9d09d37 100644 > > > --- a/drivers/perf/arm-cci.c > > > +++ b/drivers/perf/arm-cci.c > > > @@ -320,12 +320,8 @@ static int cci400_get_event_idx(struct cci_pmu *cci_pmu, > > > return CCI400_PMU_CYCLE_CNTR_IDX; > > > } > > > > > > - for (idx = CCI400_PMU_CNTR0_IDX; idx <= CCI_PMU_CNTR_LAST(cci_pmu); ++idx) > > > - if (!test_and_set_bit(idx, hw->used_mask)) > > > - return idx; > > > - > > > - /* No counters available */ > > > - return -EAGAIN; > > > + idx = find_and_set_bit(hw->used_mask, CCI_PMU_CNTR_LAST(cci_pmu) + 1); > > > > CCI400_PMU_CNTR0_IDX is defined as 1, so isn't this wrong? > > You're right. Will fix in v2 > > > [...] > > > > > diff --git a/drivers/perf/arm_dmc620_pmu.c b/drivers/perf/arm_dmc620_pmu.c > > > index 30cea6859574..e41c84dabc3e 100644 > > > --- a/drivers/perf/arm_dmc620_pmu.c > > > +++ b/drivers/perf/arm_dmc620_pmu.c > > > @@ -303,13 +303,8 @@ static int dmc620_get_event_idx(struct perf_event *event) > > > end_idx = DMC620_PMU_MAX_COUNTERS; > > > } > > > > > > - for (idx = start_idx; idx < end_idx; ++idx) { > > > - if (!test_and_set_bit(idx, dmc620_pmu->used_mask)) > > > - return idx; > > > - } > > > - > > > - /* The counters are all in use. */ > > > - return -EAGAIN; > > > + idx = find_and_set_next_bit(dmc620_pmu->used_mask, end_idx, start_idx); > > > > It might just be me, but I'd find this a tonne easier to read if you swapped > > the last two arguments around so that the offset came before the limit in > > the new function. > > I personally agree, but we already have find_next_*_bit(addr, nbits, offset) > functions, and having atomic versions of the same with different order > of arguments will make it even more messy...
Urgh, and there's loads of them too :(
Will
| |