| Message ID | 20210518091725.046774792@linutronix.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | genirq, perf: Cleanup the abuse of irq_set_affinity_hint() | expand |
Hi Thomas, On Tue, May 18, 2021 at 11:17:25AM +0200, Thomas Gleixner wrote: > The modular PMU drivers use irq_set_affinity_hint() to set the affinity > for the PMU interrupts, which relies on the undocumented side effect that > this function actually sets the affinity under the hood. > > Setting an hint is clearly not a guarantee and for these PMU interrupts an > affinity hint, which is supposed to guide userspace for setting affinity, > is beyond pointless, because the affinity of these interrupts cannot be > modified from user space. > > Aside of that the error checks are bogus because the only error which is > returned from irq_set_affinity_hint() is when there is no irq descriptor > for the interrupt number, but not when the affinity set fails. That's on > purpose because the hint can point to an offline CPU. > > Sigh, if people would at least talk if something is missing... > > Clean up the mess by exposing irq_set_affinity() and converting the drivers > over to that. Sorry about this, and thanks for cleaning this up. For the series: Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > > Thanks, > > tglx > --- > drivers/perf/arm-ccn.c | 6 +--- > drivers/perf/arm-cmn.c | 9 +----- > drivers/perf/arm_dmc620_pmu.c | 5 +-- > drivers/perf/arm_dsu_pmu.c | 8 +---- > drivers/perf/arm_smmuv3_pmu.c | 10 ++----- > drivers/perf/fsl_imx8_ddr_perf.c | 5 +-- > drivers/perf/hisilicon/hisi_uncore_ddrc_pmu.c | 3 -- > drivers/perf/hisilicon/hisi_uncore_hha_pmu.c | 3 -- > drivers/perf/hisilicon/hisi_uncore_l3c_pmu.c | 3 -- > drivers/perf/hisilicon/hisi_uncore_pa_pmu.c | 3 -- > drivers/perf/hisilicon/hisi_uncore_pmu.c | 4 +- > drivers/perf/hisilicon/hisi_uncore_sllc_pmu.c | 3 -- > include/linux/interrupt.h | 35 +------------------------- > kernel/irq/manage.c | 33 +++++++++++++++++++++++- > 14 files changed, 49 insertions(+), 81 deletions(-) > >
Hi Thomas, On Tue, May 18, 2021 at 11:17:25AM +0200, Thomas Gleixner wrote: > The modular PMU drivers use irq_set_affinity_hint() to set the affinity > for the PMU interrupts, which relies on the undocumented side effect that > this function actually sets the affinity under the hood. > > Setting an hint is clearly not a guarantee and for these PMU interrupts an > affinity hint, which is supposed to guide userspace for setting affinity, > is beyond pointless, because the affinity of these interrupts cannot be > modified from user space. > > Aside of that the error checks are bogus because the only error which is > returned from irq_set_affinity_hint() is when there is no irq descriptor > for the interrupt number, but not when the affinity set fails. That's on > purpose because the hint can point to an offline CPU. > > Sigh, if people would at least talk if something is missing... > > Clean up the mess by exposing irq_set_affinity() and converting the drivers > over to that. This all looks good to me, thanks. Given the number of PMU drivers it touches, it may well conflict with driver work for 5.14. If you put the IRQ core stuff on a stable branch, then I could pull that into the Arm perf tree and stick all the driver changes on top. That also means any new drivers that come in can use irq_set_affinity() right away. Does that work for you? Cheers, Will
On Tue, May 18 2021 at 11:48, Will Deacon wrote: > On Tue, May 18, 2021 at 11:17:25AM +0200, Thomas Gleixner wrote: >> The modular PMU drivers use irq_set_affinity_hint() to set the affinity >> for the PMU interrupts, which relies on the undocumented side effect that >> this function actually sets the affinity under the hood. >> >> Setting an hint is clearly not a guarantee and for these PMU interrupts an >> affinity hint, which is supposed to guide userspace for setting affinity, >> is beyond pointless, because the affinity of these interrupts cannot be >> modified from user space. >> >> Aside of that the error checks are bogus because the only error which is >> returned from irq_set_affinity_hint() is when there is no irq descriptor >> for the interrupt number, but not when the affinity set fails. That's on >> purpose because the hint can point to an offline CPU. >> >> Sigh, if people would at least talk if something is missing... >> >> Clean up the mess by exposing irq_set_affinity() and converting the drivers >> over to that. > > This all looks good to me, thanks. Given the number of PMU drivers it > touches, it may well conflict with driver work for 5.14. If you put the > IRQ core stuff on a stable branch, then I could pull that into the Arm > perf tree and stick all the driver changes on top. That also means any > new drivers that come in can use irq_set_affinity() right away. > > Does that work for you? Sure. I'll send you a tag to pull ... Thanks, tglx
On Tue, May 18 2021 at 17:51, Thomas Gleixner wrote:
> Sure. I'll send you a tag to pull ...
Here you go:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-export-set-affinity
Thanks,
tglx
On Wed, May 19, 2021 at 11:08:30AM +0200, Thomas Gleixner wrote: > On Tue, May 18 2021 at 17:51, Thomas Gleixner wrote: > > Sure. I'll send you a tag to pull ... > > Here you go: > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq-export-set-affinity Cheers, I've pulled that in and stuck the driver patches on top. Will