| Message ID | 1500931022-21000-1-git-send-email-nleeder@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 24, 2017 at 05:17:02PM -0400, Neil Leeder wrote: > The check for column exclusion did not verify that the event being > checked was an L2 event, and not a software event. > Software events should not be checked for column exclusion. > This resulted in a group with both software and L2 events sometimes > incorrectly rejecting the L2 event for column exclusion and > not counting it. > > Add a check for PMU type before applying column exclusion logic. > > Signed-off-by: Neil Leeder <nleeder@codeaurora.org> This looks correct, so: Acked-by: Mark Rutland <mark.rutland@arm.com> Should this have: Fixes: 21bdbb7102edeaeb ("perf: add qcom l2 cache perf events driver") ... ? > --- > drivers/perf/qcom_l2_pmu.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/perf/qcom_l2_pmu.c b/drivers/perf/qcom_l2_pmu.c > index c259848..b242cce 100644 > --- a/drivers/perf/qcom_l2_pmu.c > +++ b/drivers/perf/qcom_l2_pmu.c > @@ -546,6 +546,7 @@ static int l2_cache_event_init(struct perf_event *event) > } > > if ((event != event->group_leader) && > + !is_software_event(event->group_leader) && > (L2_EVT_GROUP(event->group_leader->attr.config) == > L2_EVT_GROUP(event->attr.config))) { > dev_dbg_ratelimited(&l2cache_pmu->pdev->dev, > @@ -558,6 +559,7 @@ static int l2_cache_event_init(struct perf_event *event) > list_for_each_entry(sibling, &event->group_leader->sibling_list, > group_entry) { > if ((sibling != event) && > + !is_software_event(sibling) && > (L2_EVT_GROUP(sibling->attr.config) == > L2_EVT_GROUP(event->attr.config))) { > dev_dbg_ratelimited(&l2cache_pmu->pdev->dev, It's unfortunate that we duplicate the checks for the leader and siblings, but that's not a new problem, and we can fix that in a follow-up patch. Thanks, Mark.
On 7/25/2017 1:01 PM, Mark Rutland wrote: > On Mon, Jul 24, 2017 at 05:17:02PM -0400, Neil Leeder wrote: >> The check for column exclusion did not verify that the event being >> checked was an L2 event, and not a software event. >> Software events should not be checked for column exclusion. >> This resulted in a group with both software and L2 events sometimes >> incorrectly rejecting the L2 event for column exclusion and >> not counting it. >> >> Add a check for PMU type before applying column exclusion logic. >> >> Signed-off-by: Neil Leeder <nleeder@codeaurora.org> > > This looks correct, so: > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > Should this have: > > Fixes: 21bdbb7102edeaeb ("perf: add qcom l2 cache perf events driver") > > ... ? > Thanks. I'll re-post with the Fixes tag. Neil
On Tue, Jul 25, 2017 at 03:43:54PM -0400, Leeder, Neil wrote: > On 7/25/2017 1:01 PM, Mark Rutland wrote: > > On Mon, Jul 24, 2017 at 05:17:02PM -0400, Neil Leeder wrote: > >> The check for column exclusion did not verify that the event being > >> checked was an L2 event, and not a software event. > >> Software events should not be checked for column exclusion. > >> This resulted in a group with both software and L2 events sometimes > >> incorrectly rejecting the L2 event for column exclusion and > >> not counting it. > >> > >> Add a check for PMU type before applying column exclusion logic. > >> > >> Signed-off-by: Neil Leeder <nleeder@codeaurora.org> > > > > This looks correct, so: > > > > Acked-by: Mark Rutland <mark.rutland@arm.com> > > > > Should this have: > > > > Fixes: 21bdbb7102edeaeb ("perf: add qcom l2 cache perf events driver") > > > > ... ? > > > Thanks. I'll re-post with the Fixes tag. No need, I'll pick it up and add it when I apply. Cheers, Will
diff --git a/drivers/perf/qcom_l2_pmu.c b/drivers/perf/qcom_l2_pmu.c index c259848..b242cce 100644 --- a/drivers/perf/qcom_l2_pmu.c +++ b/drivers/perf/qcom_l2_pmu.c @@ -546,6 +546,7 @@ static int l2_cache_event_init(struct perf_event *event) } if ((event != event->group_leader) && + !is_software_event(event->group_leader) && (L2_EVT_GROUP(event->group_leader->attr.config) == L2_EVT_GROUP(event->attr.config))) { dev_dbg_ratelimited(&l2cache_pmu->pdev->dev, @@ -558,6 +559,7 @@ static int l2_cache_event_init(struct perf_event *event) list_for_each_entry(sibling, &event->group_leader->sibling_list, group_entry) { if ((sibling != event) && + !is_software_event(sibling) && (L2_EVT_GROUP(sibling->attr.config) == L2_EVT_GROUP(event->attr.config))) { dev_dbg_ratelimited(&l2cache_pmu->pdev->dev,
The check for column exclusion did not verify that the event being checked was an L2 event, and not a software event. Software events should not be checked for column exclusion. This resulted in a group with both software and L2 events sometimes incorrectly rejecting the L2 event for column exclusion and not counting it. Add a check for PMU type before applying column exclusion logic. Signed-off-by: Neil Leeder <nleeder@codeaurora.org> --- drivers/perf/qcom_l2_pmu.c | 2 ++ 1 file changed, 2 insertions(+)