[v3,00/19] irqdomain: fix mapping race and clean up locking

Message ID 20221209140150.1453-1-johan+linaro@kernel.org
Headers
Series irqdomain: fix mapping race and clean up locking |

Message

Johan Hovold Dec. 9, 2022, 2:01 p.m. UTC
  Parallel probing (e.g. due to asynchronous probing) of devices that
share interrupts can currently result in two mappings for the same
hardware interrupt to be created.

This series fixes this mapping race and clean up the irqdomain locking
so that in the end the global irq_domain_mutex is only used for managing
the likewise global irq_domain_list, while domain operations (e.g.
IRQ allocations) use per-domain (hierarchy) locking.

Johan


Changes in v2
 - split out redundant-lookup cleanup (1/4)
 - use a per-domain mutex to address mapping race (2/4)
 - move kernel-doc to exported function (2/4)
 - fix association race (3/4, new)
 - use per-domain mutex for associations (4/4, new)

Changes in v3
 - drop dead and bogus code (1--3/19, new)
 - fix racy mapcount accesses (5/19, new)
 - drop revmap mutex (6/19, new)
 - use irq_domain_mutex to address mapping race (9/19)
 - clean up irq_domain_push/pop_irq() (10/19, new)
 - use irq_domain_create_hierarchy() to construct hierarchies
   (11--18/19, new)
 - switch to per-domain locking (19/19, new)


Johan Hovold (19):
  irqdomain: Drop bogus fwspec-mapping error handling
  irqdomain: Drop dead domain-name assignment
  irqdomain: Drop leftover brackets
  irqdomain: Fix association race
  irqdomain: Fix disassociation race
  irqdomain: Drop revmap mutex
  irqdomain: Look for existing mapping only once
  irqdomain: Refactor __irq_domain_alloc_irqs()
  irqdomain: Fix mapping-creation race
  irqdomain: Clean up irq_domain_push/pop_irq()
  x86/ioapic: Use irq_domain_create_hierarchy()
  x86/apic: Use irq_domain_create_hierarchy()
  irqchip/alpine-msi: Use irq_domain_add_hierarchy()
  irqchip/gic-v2m: Use irq_domain_create_hierarchy()
  irqchip/gic-v3-its: Use irq_domain_create_hierarchy()
  irqchip/gic-v3-mbi: Use irq_domain_create_hierarchy()
  irqchip/loongson-pch-msi: Use irq_domain_create_hierarchy()
  irqchip/mvebu-odmi: Use irq_domain_create_hierarchy()
  irqdomain: Switch to per-domain locking

 arch/x86/kernel/apic/io_apic.c         |   8 +-
 arch/x86/platform/uv/uv_irq.c          |   7 +-
 drivers/irqchip/irq-alpine-msi.c       |   8 +-
 drivers/irqchip/irq-gic-v2m.c          |   5 +-
 drivers/irqchip/irq-gic-v3-its.c       |  13 +-
 drivers/irqchip/irq-gic-v3-mbi.c       |   5 +-
 drivers/irqchip/irq-loongson-pch-msi.c |   9 +-
 drivers/irqchip/irq-mvebu-odmi.c       |  13 +-
 include/linux/irqdomain.h              |   6 +-
 kernel/irq/irqdomain.c                 | 328 ++++++++++++++-----------
 10 files changed, 220 insertions(+), 182 deletions(-)
  

Comments

Thomas Gleixner Dec. 9, 2022, 3:51 p.m. UTC | #1
Johan!

On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> Parallel probing (e.g. due to asynchronous probing) of devices that
> share interrupts can currently result in two mappings for the same
> hardware interrupt to be created.
>
> This series fixes this mapping race and clean up the irqdomain locking
> so that in the end the global irq_domain_mutex is only used for managing
> the likewise global irq_domain_list, while domain operations (e.g.
> IRQ allocations) use per-domain (hierarchy) locking.

Can you please rebase that on top of:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

Thanks,

        tglx
  
Johan Hovold Dec. 9, 2022, 4:16 p.m. UTC | #2
Hi Thomas,

On Fri, Dec 09, 2022 at 04:51:00PM +0100, Thomas Gleixner wrote:
> Johan!
> 
> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> > Parallel probing (e.g. due to asynchronous probing) of devices that
> > share interrupts can currently result in two mappings for the same
> > hardware interrupt to be created.
> >
> > This series fixes this mapping race and clean up the irqdomain locking
> > so that in the end the global irq_domain_mutex is only used for managing
> > the likewise global irq_domain_list, while domain operations (e.g.
> > IRQ allocations) use per-domain (hierarchy) locking.
> 
> Can you please rebase that on top of:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core

The series is based on next-20221208 which should contain that branch in
its current state if I'm not mistaken.

I just tried applying it on top of irq/core and did not notice any
problems.

Johan
  
Thomas Gleixner Dec. 9, 2022, 7:53 p.m. UTC | #3
On Fri, Dec 09 2022 at 17:16, Johan Hovold wrote:
> On Fri, Dec 09, 2022 at 04:51:00PM +0100, Thomas Gleixner wrote:
>> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
>> > Parallel probing (e.g. due to asynchronous probing) of devices that
>> > share interrupts can currently result in two mappings for the same
>> > hardware interrupt to be created.
>> >
>> > This series fixes this mapping race and clean up the irqdomain locking
>> > so that in the end the global irq_domain_mutex is only used for managing
>> > the likewise global irq_domain_list, while domain operations (e.g.
>> > IRQ allocations) use per-domain (hierarchy) locking.
>> 
>> Can you please rebase that on top of:
>> 
>>   git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
>
> The series is based on next-20221208 which should contain that branch in
> its current state if I'm not mistaken.
>
> I just tried applying it on top of irq/core and did not notice any
> problems.

Sorry for the noise. Pilot error. -ETOOMANYBRANCHES :)
  
Hsin-Yi Wang Dec. 15, 2022, 9:31 a.m. UTC | #4
On Thu, Dec 15, 2022 at 5:22 PM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Parallel probing (e.g. due to asynchronous probing) of devices that
> share interrupts can currently result in two mappings for the same
> hardware interrupt to be created.
>
> This series fixes this mapping race and clean up the irqdomain locking
> so that in the end the global irq_domain_mutex is only used for managing
> the likewise global irq_domain_list, while domain operations (e.g.
> IRQ allocations) use per-domain (hierarchy) locking.
>
> Johan
>
>
> Changes in v2
>  - split out redundant-lookup cleanup (1/4)
>  - use a per-domain mutex to address mapping race (2/4)
>  - move kernel-doc to exported function (2/4)
>  - fix association race (3/4, new)
>  - use per-domain mutex for associations (4/4, new)
>
> Changes in v3
>  - drop dead and bogus code (1--3/19, new)
>  - fix racy mapcount accesses (5/19, new)
>  - drop revmap mutex (6/19, new)
>  - use irq_domain_mutex to address mapping race (9/19)
>  - clean up irq_domain_push/pop_irq() (10/19, new)
>  - use irq_domain_create_hierarchy() to construct hierarchies
>    (11--18/19, new)
>  - switch to per-domain locking (19/19, new)
>
>
> Johan Hovold (19):
>   irqdomain: Drop bogus fwspec-mapping error handling
>   irqdomain: Drop dead domain-name assignment
>   irqdomain: Drop leftover brackets
>   irqdomain: Fix association race
>   irqdomain: Fix disassociation race
>   irqdomain: Drop revmap mutex
>   irqdomain: Look for existing mapping only once
>   irqdomain: Refactor __irq_domain_alloc_irqs()
>   irqdomain: Fix mapping-creation race
>   irqdomain: Clean up irq_domain_push/pop_irq()
>   x86/ioapic: Use irq_domain_create_hierarchy()
>   x86/apic: Use irq_domain_create_hierarchy()
>   irqchip/alpine-msi: Use irq_domain_add_hierarchy()
>   irqchip/gic-v2m: Use irq_domain_create_hierarchy()
>   irqchip/gic-v3-its: Use irq_domain_create_hierarchy()
>   irqchip/gic-v3-mbi: Use irq_domain_create_hierarchy()
>   irqchip/loongson-pch-msi: Use irq_domain_create_hierarchy()
>   irqchip/mvebu-odmi: Use irq_domain_create_hierarchy()
>   irqdomain: Switch to per-domain locking
>
>  arch/x86/kernel/apic/io_apic.c         |   8 +-
>  arch/x86/platform/uv/uv_irq.c          |   7 +-
>  drivers/irqchip/irq-alpine-msi.c       |   8 +-
>  drivers/irqchip/irq-gic-v2m.c          |   5 +-
>  drivers/irqchip/irq-gic-v3-its.c       |  13 +-
>  drivers/irqchip/irq-gic-v3-mbi.c       |   5 +-
>  drivers/irqchip/irq-loongson-pch-msi.c |   9 +-
>  drivers/irqchip/irq-mvebu-odmi.c       |  13 +-
>  include/linux/irqdomain.h              |   6 +-
>  kernel/irq/irqdomain.c                 | 328 ++++++++++++++-----------
>  10 files changed, 220 insertions(+), 182 deletions(-)
>
> --

Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>

The series solves a race issue when having non-populated 2nd source
components that share the same irq on ARM devices:
Previously we would see
[    0.476357] irq: type mismatch, failed to map hwirq-11 for pinctrl@10005000!
and the component failed to probe.


> 2.37.4
>
  
Johan Hovold Jan. 16, 2023, 1:53 p.m. UTC | #5
On Thu, Dec 15, 2022 at 05:31:24PM +0800, Hsin-Yi Wang wrote:
> On Thu, Dec 15, 2022 at 5:22 PM Johan Hovold <johan+linaro@kernel.org> wrote:
> >
> > Parallel probing (e.g. due to asynchronous probing) of devices that
> > share interrupts can currently result in two mappings for the same
> > hardware interrupt to be created.
> >
> > This series fixes this mapping race and clean up the irqdomain locking
> > so that in the end the global irq_domain_mutex is only used for managing
> > the likewise global irq_domain_list, while domain operations (e.g.
> > IRQ allocations) use per-domain (hierarchy) locking.

> Tested-by: Hsin-Yi Wang <hsinyi@chromium.org>
> 
> The series solves a race issue when having non-populated 2nd source
> components that share the same irq on ARM devices:
> Previously we would see
> [    0.476357] irq: type mismatch, failed to map hwirq-11 for pinctrl@10005000!
> and the component failed to probe.

Thanks again for testing. I just sent a v4 adding a couple of clarifying
comments to the final patch.

Johan