[PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Tuan Phan]

Add IRQF_SHARED flag when register IRQ such that multiple dsu
devices can share same IRQ.

Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
---
 drivers/perf/arm_dsu_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
index 70968c8..2622900 100644
--- a/drivers/perf/arm_dsu_pmu.c
+++ b/drivers/perf/arm_dsu_pmu.c
@@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
 	if (!name)
 		return -ENOMEM;
 	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
-			      IRQF_NOBALANCING, name, dsu_pmu);
+			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
 	if (rc) {
 		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
 		return rc;
-- 
2.7.4

[PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Tuan Phan]

Add IRQF_SHARED flag when register IRQ such that multiple dsu
devices can share same IRQ.

Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
---
 drivers/perf/arm_dsu_pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
index 70968c8..2622900 100644
--- a/drivers/perf/arm_dsu_pmu.c
+++ b/drivers/perf/arm_dsu_pmu.c
@@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
 	if (!name)
 		return -ENOMEM;
 	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
-			      IRQF_NOBALANCING, name, dsu_pmu);
+			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
 	if (rc) {
 		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
 		return rc;
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Will Deacon]

[+Suzuki, since he wrote the driver]

On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> Add IRQF_SHARED flag when register IRQ such that multiple dsu
> devices can share same IRQ.
> 
> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
> ---
>  drivers/perf/arm_dsu_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index 70968c8..2622900 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
>  	if (!name)
>  		return -ENOMEM;
>  	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
> -			      IRQF_NOBALANCING, name, dsu_pmu);
> +			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
>  	if (rc) {
>  		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
>  		return rc;
> -- 
> 2.7.4
> 

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Will Deacon]

[+Suzuki, since he wrote the driver]

On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> Add IRQF_SHARED flag when register IRQ such that multiple dsu
> devices can share same IRQ.
> 
> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
> ---
>  drivers/perf/arm_dsu_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index 70968c8..2622900 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
>  	if (!name)
>  		return -ENOMEM;
>  	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
> -			      IRQF_NOBALANCING, name, dsu_pmu);
> +			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
>  	if (rc) {
>  		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
>  		return rc;
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Mark Rutland]

On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> Add IRQF_SHARED flag when register IRQ such that multiple dsu
> devices can share same IRQ.
> 
> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>

I don't think that this makes sense; further I think that this
highlights that the current driver doesn't support such a configuration
for other reasons.

A DSU instance can only be accessed from a CPU associated with it, since
it's accessed via sysregs. The IRQ handler must run on one of those
CPUs.

To handle that, the DSU PMU driver will need to gain an understanding of
which CPUs are associated with the instance. As it stands the driver
seems to assume that there's a single DSU instance, and that all CPUs
are affine to that same instance.

So NAK to this patch, given the above.

Thanks,
Mark.

> ---
>  drivers/perf/arm_dsu_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index 70968c8..2622900 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
>  	if (!name)
>  		return -ENOMEM;
>  	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
> -			      IRQF_NOBALANCING, name, dsu_pmu);
> +			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
>  	if (rc) {
>  		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
>  		return rc;
> -- 
> 2.7.4
> 

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Mark Rutland]

On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> Add IRQF_SHARED flag when register IRQ such that multiple dsu
> devices can share same IRQ.
> 
> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>

I don't think that this makes sense; further I think that this
highlights that the current driver doesn't support such a configuration
for other reasons.

A DSU instance can only be accessed from a CPU associated with it, since
it's accessed via sysregs. The IRQ handler must run on one of those
CPUs.

To handle that, the DSU PMU driver will need to gain an understanding of
which CPUs are associated with the instance. As it stands the driver
seems to assume that there's a single DSU instance, and that all CPUs
are affine to that same instance.

So NAK to this patch, given the above.

Thanks,
Mark.

> ---
>  drivers/perf/arm_dsu_pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/arm_dsu_pmu.c b/drivers/perf/arm_dsu_pmu.c
> index 70968c8..2622900 100644
> --- a/drivers/perf/arm_dsu_pmu.c
> +++ b/drivers/perf/arm_dsu_pmu.c
> @@ -700,7 +700,7 @@ static int dsu_pmu_device_probe(struct platform_device *pdev)
>  	if (!name)
>  		return -ENOMEM;
>  	rc = devm_request_irq(&pdev->dev, irq, dsu_pmu_handle_irq,
> -			      IRQF_NOBALANCING, name, dsu_pmu);
> +			      IRQF_NOBALANCING | IRQF_SHARED, name, dsu_pmu);
>  	if (rc) {
>  		dev_warn(&pdev->dev, "Failed to request IRQ %d\n", irq);
>  		return rc;
> -- 
> 2.7.4
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Mark Rutland]

On Thu, Mar 19, 2020 at 02:32:51PM +0000, Mark Rutland wrote:
> On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> > Add IRQF_SHARED flag when register IRQ such that multiple dsu
> > devices can share same IRQ.
> > 
> > Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
> 
> I don't think that this makes sense; further I think that this
> highlights that the current driver doesn't support such a configuration
> for other reasons.
> 
> A DSU instance can only be accessed from a CPU associated with it, since
> it's accessed via sysregs. The IRQ handler must run on one of those
> CPUs.
> 
> To handle that, the DSU PMU driver will need to gain an understanding of
> which CPUs are associated with the instance. As it stands the driver
> seems to assume that there's a single DSU instance, and that all CPUs
> are affine to that same instance.

Sorry, I misread dsu_pmu_get_online_cpu_any_but(), multiple instances
are handled already.

> So NAK to this patch, given the above.

Regardless, this NAK stands.

Thanks,
Mark.

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Mark Rutland]

On Thu, Mar 19, 2020 at 02:32:51PM +0000, Mark Rutland wrote:
> On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
> > Add IRQF_SHARED flag when register IRQ such that multiple dsu
> > devices can share same IRQ.
> > 
> > Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
> 
> I don't think that this makes sense; further I think that this
> highlights that the current driver doesn't support such a configuration
> for other reasons.
> 
> A DSU instance can only be accessed from a CPU associated with it, since
> it's accessed via sysregs. The IRQ handler must run on one of those
> CPUs.
> 
> To handle that, the DSU PMU driver will need to gain an understanding of
> which CPUs are associated with the instance. As it stands the driver
> seems to assume that there's a single DSU instance, and that all CPUs
> are affine to that same instance.

Sorry, I misread dsu_pmu_get_online_cpu_any_but(), multiple instances
are handled already.

> So NAK to this patch, given the above.

Regardless, this NAK stands.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Robin Murphy]

On 2020-03-19 2:35 pm, Mark Rutland wrote:
> On Thu, Mar 19, 2020 at 02:32:51PM +0000, Mark Rutland wrote:
>> On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
>>> Add IRQF_SHARED flag when register IRQ such that multiple dsu
>>> devices can share same IRQ.
>>>
>>> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
>>
>> I don't think that this makes sense; further I think that this
>> highlights that the current driver doesn't support such a configuration
>> for other reasons.
>>
>> A DSU instance can only be accessed from a CPU associated with it, since
>> it's accessed via sysregs. The IRQ handler must run on one of those
>> CPUs.
>>
>> To handle that, the DSU PMU driver will need to gain an understanding of
>> which CPUs are associated with the instance. As it stands the driver
>> seems to assume that there's a single DSU instance, and that all CPUs
>> are affine to that same instance.
> 
> Sorry, I misread dsu_pmu_get_online_cpu_any_but(), multiple instances
> are handled already.

Oh, so either way it's effectively a rerun of the U8500 problem of 
having no guarantee that the interrupt will be taken on an appropriate 
CPU, and losing genuine events as apparently spurious if it isn't. Yeah, 
that's really really bad... :(

>> So NAK to this patch, given the above.
> 
> Regardless, this NAK stands.

Agreed, pretending that this might work without significantly more 
invasive workarounds does more harm than good.

Robin.

Re: [PATCH 1/2] perf: dsu: Allow multiple devices share same IRQ.

[Robin Murphy]

On 2020-03-19 2:35 pm, Mark Rutland wrote:
> On Thu, Mar 19, 2020 at 02:32:51PM +0000, Mark Rutland wrote:
>> On Tue, Mar 17, 2020 at 05:26:15PM -0700, Tuan Phan wrote:
>>> Add IRQF_SHARED flag when register IRQ such that multiple dsu
>>> devices can share same IRQ.
>>>
>>> Signed-off-by: Tuan Phan <tuanphan@os.amperecomputing.com>
>>
>> I don't think that this makes sense; further I think that this
>> highlights that the current driver doesn't support such a configuration
>> for other reasons.
>>
>> A DSU instance can only be accessed from a CPU associated with it, since
>> it's accessed via sysregs. The IRQ handler must run on one of those
>> CPUs.
>>
>> To handle that, the DSU PMU driver will need to gain an understanding of
>> which CPUs are associated with the instance. As it stands the driver
>> seems to assume that there's a single DSU instance, and that all CPUs
>> are affine to that same instance.
> 
> Sorry, I misread dsu_pmu_get_online_cpu_any_but(), multiple instances
> are handled already.

Oh, so either way it's effectively a rerun of the U8500 problem of 
having no guarantee that the interrupt will be taken on an appropriate 
CPU, and losing genuine events as apparently spurious if it isn't. Yeah, 
that's really really bad... :(

>> So NAK to this patch, given the above.
> 
> Regardless, this NAK stands.

Agreed, pretending that this might work without significantly more 
invasive workarounds does more harm than good.

Robin.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel