| Message ID | 20241107215921.3518636-1-ctshao@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | perf/arm-smmuv3: Fix lockdep assert in ->event_init() | expand |
On Thu, Nov 07, 2024 at 09:59:20PM +0000, Chun-Tse Shao wrote: > Same as > https://lore.kernel.org/all/20240514180050.182454-1-namhyung@kernel.org/, > we should skip `for_each_sibling_event()` for group leader since it > doesn't have the ctx yet. > > Fixes: f3c0eba28704 ("perf: Add a few assertions") > Reported-by: Greg Thelen <gthelen@google.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Robin Murphy <robin.murphy@arm.com> > Cc: Tuan Phan <tuanphan@os.amperecomputing.com> > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > --- > drivers/perf/arm_smmuv3_pmu.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > index d5fa92ba8373..09789599fccc 100644 > --- a/drivers/perf/arm_smmuv3_pmu.c > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -431,6 +431,11 @@ static int smmu_pmu_event_init(struct perf_event *event) > return -EINVAL; > } > > + hwc->idx = -1; I think you should update the event->cpu as well. Thanks, Namhyung > + > + if (event->group_leader == event) > + return 0; > + > for_each_sibling_event(sibling, event->group_leader) { > if (is_software_event(sibling)) > continue; > @@ -442,8 +447,6 @@ static int smmu_pmu_event_init(struct perf_event *event) > return -EINVAL; > } > > - hwc->idx = -1; > - > /* > * Ensure all events are on the same cpu so all events are in the > * same cpu context, to avoid races on pmu_enable etc. > -- > 2.47.0.277.g8800431eea-goog >
Thank you for your feedback, Namhyung! Please see my v2 patch: https://lore.kernel.org/all/20241108050806.3730811-1-ctshao@google.com/ On Thu, Nov 7, 2024 at 5:23 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Thu, Nov 07, 2024 at 09:59:20PM +0000, Chun-Tse Shao wrote: > > Same as > > https://lore.kernel.org/all/20240514180050.182454-1-namhyung@kernel.org/, > > we should skip `for_each_sibling_event()` for group leader since it > > doesn't have the ctx yet. > > > > Fixes: f3c0eba28704 ("perf: Add a few assertions") > > Reported-by: Greg Thelen <gthelen@google.com> > > Cc: Namhyung Kim <namhyung@kernel.org> > > Cc: Robin Murphy <robin.murphy@arm.com> > > Cc: Tuan Phan <tuanphan@os.amperecomputing.com> > > Signed-off-by: Chun-Tse Shao <ctshao@google.com> > > --- > > drivers/perf/arm_smmuv3_pmu.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > > index d5fa92ba8373..09789599fccc 100644 > > --- a/drivers/perf/arm_smmuv3_pmu.c > > +++ b/drivers/perf/arm_smmuv3_pmu.c > > @@ -431,6 +431,11 @@ static int smmu_pmu_event_init(struct perf_event *event) > > return -EINVAL; > > } > > > > + hwc->idx = -1; > > I think you should update the event->cpu as well. > > Thanks, > Namhyung > > > + > > + if (event->group_leader == event) > > + return 0; > > + > > for_each_sibling_event(sibling, event->group_leader) { > > if (is_software_event(sibling)) > > continue; > > @@ -442,8 +447,6 @@ static int smmu_pmu_event_init(struct perf_event *event) > > return -EINVAL; > > } > > > > - hwc->idx = -1; > > - > > /* > > * Ensure all events are on the same cpu so all events are in the > > * same cpu context, to avoid races on pmu_enable etc. > > -- > > 2.47.0.277.g8800431eea-goog > >
diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c index d5fa92ba8373..09789599fccc 100644 --- a/drivers/perf/arm_smmuv3_pmu.c +++ b/drivers/perf/arm_smmuv3_pmu.c @@ -431,6 +431,11 @@ static int smmu_pmu_event_init(struct perf_event *event) return -EINVAL; } + hwc->idx = -1; + + if (event->group_leader == event) + return 0; + for_each_sibling_event(sibling, event->group_leader) { if (is_software_event(sibling)) continue; @@ -442,8 +447,6 @@ static int smmu_pmu_event_init(struct perf_event *event) return -EINVAL; } - hwc->idx = -1; - /* * Ensure all events are on the same cpu so all events are in the * same cpu context, to avoid races on pmu_enable etc.
Same as https://lore.kernel.org/all/20240514180050.182454-1-namhyung@kernel.org/, we should skip `for_each_sibling_event()` for group leader since it doesn't have the ctx yet. Fixes: f3c0eba28704 ("perf: Add a few assertions") Reported-by: Greg Thelen <gthelen@google.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Robin Murphy <robin.murphy@arm.com> Cc: Tuan Phan <tuanphan@os.amperecomputing.com> Signed-off-by: Chun-Tse Shao <ctshao@google.com> --- drivers/perf/arm_smmuv3_pmu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)