| Message ID | 20230601030144.3458136-3-ilkka@os.amperecomputing.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: ampere: Add support for Ampere SoC PMUs | expand |
On 2023-06-01 04:01, Ilkka Koskinen wrote: > Some of the PMUs may share the interrupt. Support them by > setting IRQF_SHARED This has the usual problem of allowing any PMU instance to move the IRQ affinity to a different CPU without also migrating all the other PMU contexts, and thus breaking perf core's assumptions of mutual exclusion. Thanks, Robin. > Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> > --- > drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c > index 88547a2b73e6..cc5204d1b5fb 100644 > --- a/drivers/perf/arm_cspmu/arm_cspmu.c > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c > @@ -1067,8 +1067,8 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu) > return irq; > > ret = devm_request_irq(dev, irq, arm_cspmu_handle_irq, > - IRQF_NOBALANCING | IRQF_NO_THREAD, dev_name(dev), > - cspmu); > + IRQF_NOBALANCING | IRQF_NO_THREAD | IRQF_SHARED, > + dev_name(dev), cspmu); > if (ret) { > dev_err(dev, "Could not request IRQ %d\n", irq); > return ret;
Hi Robin, On Thu, 1 Jun 2023, Robin Murphy wrote: > On 2023-06-01 04:01, Ilkka Koskinen wrote: >> Some of the PMUs may share the interrupt. Support them by >> setting IRQF_SHARED > > This has the usual problem of allowing any PMU instance to move the IRQ > affinity to a different CPU without also migrating all the other PMU > contexts, and thus breaking perf core's assumptions of mutual exclusion. I see, I wasn't aware of such an assumption. Sounds like there isn't necessarily an easy and clean solution for the shared interrupt case. I drop the patch and get back on the issue if we come up with something reasonable later. Cheers, Ilkka > > Thanks, > Robin. > >> Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> >> --- >> drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c >> b/drivers/perf/arm_cspmu/arm_cspmu.c >> index 88547a2b73e6..cc5204d1b5fb 100644 >> --- a/drivers/perf/arm_cspmu/arm_cspmu.c >> +++ b/drivers/perf/arm_cspmu/arm_cspmu.c >> @@ -1067,8 +1067,8 @@ static int arm_cspmu_request_irq(struct arm_cspmu >> *cspmu) >> return irq; >> ret = devm_request_irq(dev, irq, arm_cspmu_handle_irq, >> - IRQF_NOBALANCING | IRQF_NO_THREAD, >> dev_name(dev), >> - cspmu); >> + IRQF_NOBALANCING | IRQF_NO_THREAD | >> IRQF_SHARED, >> + dev_name(dev), cspmu); >> if (ret) { >> dev_err(dev, "Could not request IRQ %d\n", irq); >> return ret; >
On 2023-06-02 08:04, Ilkka Koskinen wrote: > > Hi Robin, > > On Thu, 1 Jun 2023, Robin Murphy wrote: >> On 2023-06-01 04:01, Ilkka Koskinen wrote: >>> Some of the PMUs may share the interrupt. Support them by >>> setting IRQF_SHARED >> >> This has the usual problem of allowing any PMU instance to move the >> IRQ affinity to a different CPU without also migrating all the other >> PMU contexts, and thus breaking perf core's assumptions of mutual >> exclusion. > > I see, I wasn't aware of such an assumption. Sounds like there isn't > necessarily an easy and clean solution for the shared interrupt case. I > drop the patch and get back on the issue if we come up with something > reasonable later. What comes to mind is factoring out the explicit interrupt-sharing machinery that we wrote to solve this problem in arm_dmc620_pmu, or possibly trying to do something with IRQ affinity notifiers (however, I recall looking into that a while ago and it didn't seem like they actually interact with CPU hotplug in the way we'd want). Thanks, Robin.
diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c index 88547a2b73e6..cc5204d1b5fb 100644 --- a/drivers/perf/arm_cspmu/arm_cspmu.c +++ b/drivers/perf/arm_cspmu/arm_cspmu.c @@ -1067,8 +1067,8 @@ static int arm_cspmu_request_irq(struct arm_cspmu *cspmu) return irq; ret = devm_request_irq(dev, irq, arm_cspmu_handle_irq, - IRQF_NOBALANCING | IRQF_NO_THREAD, dev_name(dev), - cspmu); + IRQF_NOBALANCING | IRQF_NO_THREAD | IRQF_SHARED, + dev_name(dev), cspmu); if (ret) { dev_err(dev, "Could not request IRQ %d\n", irq); return ret;
Some of the PMUs may share the interrupt. Support them by setting IRQF_SHARED Signed-off-by: Ilkka Koskinen <ilkka@os.amperecomputing.com> --- drivers/perf/arm_cspmu/arm_cspmu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)