All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf: qcom_l2: fix column exclusion check
@ 2017-07-24 21:17 ` Neil Leeder
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Leeder @ 2017-07-24 21:17 UTC (permalink / raw)
  To: Will Deacon, Mark Rutland
  Cc: linux-kernel, linux-arm-kernel, Mark Langsdorf, Mark Salter,
	Jon Masters, Timur Tabi, Mark Brown, nleeder

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.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] perf: qcom_l2: fix column exclusion check
@ 2017-07-24 21:17 ` Neil Leeder
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Leeder @ 2017-07-24 21:17 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] perf: qcom_l2: fix column exclusion check
  2017-07-24 21:17 ` Neil Leeder
@ 2017-07-25 17:01   ` Mark Rutland
  -1 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2017-07-25 17:01 UTC (permalink / raw)
  To: Neil Leeder
  Cc: Will Deacon, linux-kernel, linux-arm-kernel, Mark Langsdorf,
	Mark Salter, Jon Masters, Timur Tabi, Mark Brown

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] perf: qcom_l2: fix column exclusion check
@ 2017-07-25 17:01   ` Mark Rutland
  0 siblings, 0 replies; 8+ messages in thread
From: Mark Rutland @ 2017-07-25 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] perf: qcom_l2: fix column exclusion check
  2017-07-25 17:01   ` Mark Rutland
@ 2017-07-25 19:43     ` Leeder, Neil
  -1 siblings, 0 replies; 8+ messages in thread
From: Leeder, Neil @ 2017-07-25 19:43 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Will Deacon, linux-kernel, linux-arm-kernel, Mark Langsdorf,
	Mark Salter, Jon Masters, Timur Tabi, Mark Brown, nleeder

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] perf: qcom_l2: fix column exclusion check
@ 2017-07-25 19:43     ` Leeder, Neil
  0 siblings, 0 replies; 8+ messages in thread
From: Leeder, Neil @ 2017-07-25 19:43 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] perf: qcom_l2: fix column exclusion check
  2017-07-25 19:43     ` Leeder, Neil
@ 2017-07-26  8:27       ` Will Deacon
  -1 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2017-07-26  8:27 UTC (permalink / raw)
  To: Leeder, Neil
  Cc: Mark Rutland, linux-kernel, linux-arm-kernel, Mark Langsdorf,
	Mark Salter, Jon Masters, Timur Tabi, Mark Brown

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] perf: qcom_l2: fix column exclusion check
@ 2017-07-26  8:27       ` Will Deacon
  0 siblings, 0 replies; 8+ messages in thread
From: Will Deacon @ 2017-07-26  8:27 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-07-26  8:27 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-24 21:17 [PATCH] perf: qcom_l2: fix column exclusion check Neil Leeder
2017-07-24 21:17 ` Neil Leeder
2017-07-25 17:01 ` Mark Rutland
2017-07-25 17:01   ` Mark Rutland
2017-07-25 19:43   ` Leeder, Neil
2017-07-25 19:43     ` Leeder, Neil
2017-07-26  8:27     ` Will Deacon
2017-07-26  8:27       ` Will Deacon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.