Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1695106 > unrolled thread
| Started by | Neil Leeder <nleeder@codeaurora.org> |
|---|---|
| First post | 2017-07-24 23:20 +0200 |
| Last post | 2017-07-26 10:30 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] perf: qcom_l2: fix column exclusion check Neil Leeder <nleeder@codeaurora.org> - 2017-07-24 23:20 +0200
Re: [PATCH] perf: qcom_l2: fix column exclusion check Mark Rutland <mark.rutland@arm.com> - 2017-07-25 19:10 +0200
Re: [PATCH] perf: qcom_l2: fix column exclusion check "Leeder, Neil" <nleeder@codeaurora.org> - 2017-07-25 21:50 +0200
Re: [PATCH] perf: qcom_l2: fix column exclusion check Will Deacon <will.deacon@arm.com> - 2017-07-26 10:30 +0200
| From | Neil Leeder <nleeder@codeaurora.org> |
|---|---|
| Date | 2017-07-24 23:20 +0200 |
| Subject | [PATCH] perf: qcom_l2: fix column exclusion check |
| Message-ID | <u6SVH-227-1@gated-at.bofh.it> |
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(+)
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,
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
[toc] | [next] | [standalone]
| From | Mark Rutland <mark.rutland@arm.com> |
|---|---|
| Date | 2017-07-25 19:10 +0200 |
| Message-ID | <u7bvk-5B8-23@gated-at.bofh.it> |
| In reply to | #1695106 |
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.
[toc] | [prev] | [next] | [standalone]
| From | "Leeder, Neil" <nleeder@codeaurora.org> |
|---|---|
| Date | 2017-07-25 21:50 +0200 |
| Message-ID | <u7e0b-701-37@gated-at.bofh.it> |
| In reply to | #1695955 |
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
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
[toc] | [prev] | [next] | [standalone]
| From | Will Deacon <will.deacon@arm.com> |
|---|---|
| Date | 2017-07-26 10:30 +0200 |
| Message-ID | <u7pRF-6lO-25@gated-at.bofh.it> |
| In reply to | #1696167 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web