[irqchip:,irq/irqchip-next] irqdomain: Protect the linear revmap with RCU
diff mbox series

Message ID 162341967699.19906.3242958007782554792.tip-bot2@tip-bot2
State Accepted
Commit d4a45c68dc81f9117ceaff9f058d5fae674181b9
Headers show
Series
  • [irqchip:,irq/irqchip-next] irqdomain: Protect the linear revmap with RCU
Related show

Commit Message

irqchip-bot for Andy Shevchenko June 11, 2021, 1:54 p.m. UTC
The following commit has been merged into the irq/irqchip-next branch of irqchip:

Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
Author:        Marc Zyngier <maz@kernel.org>
AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
Committer:     Marc Zyngier <maz@kernel.org>
CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00

irqdomain: Protect the linear revmap with RCU

It is pretty odd that the radix tree uses RCU while the linear
portion doesn't, leading to potential surprises for the users,
depending on how the irqdomain has been created.

Fix this by moving the update of the linear revmap under
the mutex, and the lookup under the RCU read-side lock.

The mutex name is updated to reflect that it doesn't only
cover the radix-tree anymore.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 include/linux/irqdomain.h |  5 ++--
 kernel/irq/irqdomain.c    | 49 +++++++++++++++++---------------------
 2 files changed, 26 insertions(+), 28 deletions(-)

Comments

Guenter Roeck July 5, 2021, 5:23 p.m. UTC | #1
Hi,

On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote:
> The following commit has been merged into the irq/irqchip-next branch of irqchip:
> 
> Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
> Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
> Author:        Marc Zyngier <maz@kernel.org>
> AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
> Committer:     Marc Zyngier <maz@kernel.org>
> CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00
> 
> irqdomain: Protect the linear revmap with RCU
> 
> It is pretty odd that the radix tree uses RCU while the linear
> portion doesn't, leading to potential surprises for the users,
> depending on how the irqdomain has been created.
> 
> Fix this by moving the update of the linear revmap under
> the mutex, and the lookup under the RCU read-side lock.
> 
> The mutex name is updated to reflect that it doesn't only
> cover the radix-tree anymore.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>

This patch results in various RCU warnings when booting mipsel images
in qemu. I can not revert the patch due to subsequent changes, so I
don't know if a simple revert fixes the problem. Log messages and
bisect log see below.

Guenter

---

...
=============================
WARNING: suspicious RCU usage
5.13.0-09882-ga180bd1d7e16 #1 Not tainted
-----------------------------
include/trace/events/lock.h:13 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
RCU used illegally from extended quiescent state!
no locks held by swapper/0/0.

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
Stack : 00000000 00000000 00000047 801b2c84 80cd0000 00000004 00000000 00000000
        8209fe9c 3bdd5c9d 80fb0000 810f9930 80fb0000 00000001 8209fe38 820fcec0
        00000000 00000000 80ecfc64 8209fcc0 fffffbff 00000148 00000000 00000000
        00000000 00000064 00000002 80fb0000 80fb0000 00000001 80ecfc64 00000000
        80fb0000 80faea08 00000000 80fb1858 00000000 00000000 00000000 81100000
        ...
Call Trace:
[<8010a2ec>] show_stack+0x38/0x118
[<80c490ac>] dump_stack_lvl+0xac/0x104
[<801a4164>] lock_acquire+0x418/0x498
[<801c0d80>] __irq_resolve_mapping+0x60/0x1f0
[<807275a8>] plat_irq_dispatch+0x84/0x114
[<801038c0>] handle_int+0x160/0x16c
[<80103720>] __r4k_wait+0x20/0x40
[<80c600a4>] default_idle_call+0x9c/0x2e0
[<80173024>] do_idle+0x10c/0x1cc
[<801734b8>] cpu_startup_entry+0x24/0x3c
[<8106cf54>] start_kernel+0x790/0x7d8
[<80c55f50>] kernel_entry+0x0/0xa8


=============================
WARNING: suspicious RCU usage
5.13.0-09882-ga180bd1d7e16 #1 Not tainted
-----------------------------
include/linux/rcupdate.h:688 rcu_read_lock() used illegally while idle!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
RCU used illegally from extended quiescent state!
1 lock held by swapper/0/0:
 #0: 80fd43cc (rcu_read_lock){....}-{1:2}, at: __irq_resolve_mapping+0x28/0x1f0

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
Stack : 00000080 00000000 00000047 801b2c84 80cd0000 00000004 00000000 00000000
        8209ff1c 3bdd5c9d 80fb0000 810f9930 80fb0000 00000001 8209feb8 820fcec0
        00000000 00000000 80ecfc64 8209fd40 fffffbff 00000169 00000000 00000000
        81349a50 00000064 00000002 80fb0000 80fb0000 00000001 80ecfc64 80fd0000
        81730000 80fb0000 00000080 80fb1858 00000000 00000000 00000000 81100000
        ...
Call Trace:
[<8010a2ec>] show_stack+0x38/0x118
[<80c490ac>] dump_stack_lvl+0xac/0x104
[<801c0ee8>] __irq_resolve_mapping+0x1c8/0x1f0
[<807275a8>] plat_irq_dispatch+0x84/0x114
[<801038c0>] handle_int+0x160/0x16c
[<80103720>] __r4k_wait+0x20/0x40
[<80c600a4>] default_idle_call+0x9c/0x2e0
[<80173024>] do_idle+0x10c/0x1cc
[<801734b8>] cpu_startup_entry+0x24/0x3c
[<8106cf54>] start_kernel+0x790/0x7d8
[<80c55f50>] kernel_entry+0x0/0xa8


=============================
WARNING: suspicious RCU usage
5.13.0-09882-ga180bd1d7e16 #1 Not tainted
-----------------------------
kernel/irq/irqdomain.c:920 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
RCU used illegally from extended quiescent state!
1 lock held by swapper/0/0:
 #0: 80fd43cc (rcu_read_lock){....}-{1:2}, at: __irq_resolve_mapping+0x28/0x1f0

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
Stack : 00000080 00000000 00000047 801b2c84 80cd0000 00000004 00000000 00000000
        8209ff1c 3bdd5c9d 80fb0000 810f9930 80fb0000 00000001 8209feb8 820fcec0
        00000000 00000000 80ecfc64 8209fd40 fffffbff 00000189 00000000 00000000
        81349a50 00000064 00000002 80fb0000 80fb0000 00000001 80ecfc64 80fd0000
        81730000 80fb0000 00000080 80fb1858 00000000 00000000 00000000 81100000
        ...
Call Trace:
[<8010a2ec>] show_stack+0x38/0x118
[<80c490ac>] dump_stack_lvl+0xac/0x104
[<801c0e68>] __irq_resolve_mapping+0x148/0x1f0
[<807275a8>] plat_irq_dispatch+0x84/0x114
[<801038c0>] handle_int+0x160/0x16c
[<80103720>] __r4k_wait+0x20/0x40
[<80c600a4>] default_idle_call+0x9c/0x2e0
[<80173024>] do_idle+0x10c/0x1cc
[<801734b8>] cpu_startup_entry+0x24/0x3c
[<8106cf54>] start_kernel+0x790/0x7d8
[<80c55f50>] kernel_entry+0x0/0xa8


=============================
WARNING: suspicious RCU usage
5.13.0-09882-ga180bd1d7e16 #1 Not tainted
-----------------------------
include/linux/rcupdate.h:716 rcu_read_unlock() used illegally while idle!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
RCU used illegally from extended quiescent state!
1 lock held by swapper/0/0:
 #0: 80fd43cc (rcu_read_lock){....}-{1:2}, at: __irq_resolve_mapping+0x28/0x1f0

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
Stack : 00000080 00000000 00000047 801b2c84 80cd0000 00000004 00000000 00000000
        8209ff1c 3bdd5c9d 80fb0000 810f9930 80fb0000 00000001 8209feb8 820fcec0
        00000000 00000000 80ecfc64 8209fd40 fffffbff 000001a9 00000000 00000000
        81349a50 00000064 00000002 80fb0000 80fb0000 00000001 80ecfc64 80fd0000
        81730000 80fb0000 00000080 80fb1858 00000000 00000000 00000000 81100000
        ...
Call Trace:
[<8010a2ec>] show_stack+0x38/0x118
[<80c490ac>] dump_stack_lvl+0xac/0x104
[<801c0ea8>] __irq_resolve_mapping+0x188/0x1f0
[<807275a8>] plat_irq_dispatch+0x84/0x114
[<801038c0>] handle_int+0x160/0x16c
[<80103720>] __r4k_wait+0x20/0x40
[<80c600a4>] default_idle_call+0x9c/0x2e0
[<80173024>] do_idle+0x10c/0x1cc
[<801734b8>] cpu_startup_entry+0x24/0x3c
[<8106cf54>] start_kernel+0x790/0x7d8
[<80c55f50>] kernel_entry+0x0/0xa8


=============================
WARNING: suspicious RCU usage
5.13.0-09882-ga180bd1d7e16 #1 Not tainted
-----------------------------
include/trace/events/lock.h:58 suspicious rcu_dereference_check() usage!

other info that might help us debug this:


rcu_scheduler_active = 2, debug_locks = 1
RCU used illegally from extended quiescent state!
1 lock held by swapper/0/0:
 #0: 80fd3c4c ((console_sem).lock){-...}-{2:2}, at: down_trylock+0x18/0x4c

stack backtrace:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-09882-ga180bd1d7e16 #1
Stack : 817924dc 8209fcfc 817924f0 00001fec 80ee11b8 80fb0000 81061e40 8071e768
        00000000 3bdd5c9d 80fb0000 810f9930 000001ec 00000001 8209fc98 820fcec0
        00000000 00000000 80ecfc64 00000001 0000000a 00000003 00000001 6d6f4320
        00000000 817926e9 817944dc 00000001 80fb0000 00000001 80ecfc64 81061e20
        80faea08 80e92708 00000000 80fb0000 00000000 00000000 00000000 81100000
        ...
Call Trace:
[<8010a2ec>] show_stack+0x38/0x118
[<80c490ac>] dump_stack_lvl+0xac/0x104
[<801a834c>] lock_release+0x3d0/0x46c
[<80c6080c>] _raw_spin_unlock_irqrestore+0x24/0xa0
[<80198854>] down_trylock+0x34/0x4c
[<801aef18>] __down_trylock_console_sem+0x44/0x120
[<801af370>] console_trylock+0x1c/0x88
[<801b2ccc>] vprintk_emit+0x124/0x404
[<801b2fcc>] vprintk_default+0x20/0x2c
[<80c3e6ac>] printk+0x34/0x58
[<80c3d884>] lockdep_rcu_suspicious+0x40/0x114
[<801a4164>] lock_acquire+0x418/0x498
[<801c0d80>] __irq_resolve_mapping+0x60/0x1f0
[<807275a8>] plat_irq_dispatch+0x84/0x114
[<801038c0>] handle_int+0x160/0x16c
[<80103720>] __r4k_wait+0x20/0x40
[<80c600a4>] default_idle_call+0x9c/0x2e0
[<80173024>] do_idle+0x10c/0x1cc
[<801734b8>] cpu_startup_entry+0x24/0x3c
[<8106cf54>] start_kernel+0x790/0x7d8
[<80c55f50>] kernel_entry+0x0/0xa8

---
bisect log:

# bad: [007b350a58754a93ca9fe50c498cc27780171153] Merge tag 'dlm-5.14' of git://git.kernel.org/pub/scm/linux/kernel/git/teigland/linux-dlm
# good: [62fb9874f5da54fdb243003b386128037319b219] Linux 5.13
git bisect start '007b350a5875' '62fb9874f5da'
# good: [36824f198c621cebeb22966b5e244378fa341295] Merge tag 'for-linus' of git://git.kernel.org/pub/scm/virt/kvm/kvm
git bisect good 36824f198c621cebeb22966b5e244378fa341295
# good: [122fa8c588316aacafe7e5a393bb3e875eaf5b25] Merge tag 'for-5.14-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
git bisect good 122fa8c588316aacafe7e5a393bb3e875eaf5b25
# bad: [3563f55ce65462063543dfa6a8d8c7fbfb9d7772] Merge tag 'pm-5.14-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect bad 3563f55ce65462063543dfa6a8d8c7fbfb9d7772
# good: [c54b245d011855ea91c5beff07f1db74143ce614] Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace
git bisect good c54b245d011855ea91c5beff07f1db74143ce614
# bad: [21edf50948728f55b685ad95f196ba46196eb767] Merge tag 'irq-core-2021-06-29' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 21edf50948728f55b685ad95f196ba46196eb767
# bad: [c64638d5091a5630d0f5f5ab7001bdee1ad194f2] Merge branch irq/generic_handle_domain_irq-core into irq/irqchip-next
git bisect bad c64638d5091a5630d0f5f5ab7001bdee1ad194f2
# bad: [d4a45c68dc81f9117ceaff9f058d5fae674181b9] irqdomain: Protect the linear revmap with RCU
git bisect bad d4a45c68dc81f9117ceaff9f058d5fae674181b9
# good: [13a9a5d17d07cec8181ea0843674ce48c191628e] powerpc: Add missing linux/{of.h,irqdomain.h} include directives
git bisect good 13a9a5d17d07cec8181ea0843674ce48c191628e
# good: [1da027362a7db422243601e895e6f8288389f435] irqdomain: Reimplement irq_linear_revmap() with irq_find_mapping()
git bisect good 1da027362a7db422243601e895e6f8288389f435
# good: [4f86a06e2d6ece5316e4c42fbf946ee22acb30f3] irqdomain: Make normal and nomap irqdomains exclusive
git bisect good 4f86a06e2d6ece5316e4c42fbf946ee22acb30f3
# good: [48b15a7921d60680babe59f64e127816585a585c] irqdomain: Cache irq_data instead of a virq number in the revmap
git bisect good 48b15a7921d60680babe59f64e127816585a585c
# first bad commit: [d4a45c68dc81f9117ceaff9f058d5fae674181b9] irqdomain: Protect the linear revmap with RCU
Marc Zyngier July 5, 2021, 6:01 p.m. UTC | #2
Hi Guenter,

On Mon, 05 Jul 2021 18:23:52 +0100,
Guenter Roeck <linux@roeck-us.net> wrote:
> 
> Hi,
> 
> On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote:
> > The following commit has been merged into the irq/irqchip-next branch of irqchip:
> > 
> > Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
> > Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
> > Author:        Marc Zyngier <maz@kernel.org>
> > AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
> > Committer:     Marc Zyngier <maz@kernel.org>
> > CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00
> > 
> > irqdomain: Protect the linear revmap with RCU
> > 
> > It is pretty odd that the radix tree uses RCU while the linear
> > portion doesn't, leading to potential surprises for the users,
> > depending on how the irqdomain has been created.
> > 
> > Fix this by moving the update of the linear revmap under
> > the mutex, and the lookup under the RCU read-side lock.
> > 
> > The mutex name is updated to reflect that it doesn't only
> > cover the radix-tree anymore.
> > 
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> 
> This patch results in various RCU warnings when booting mipsel images
> in qemu. I can not revert the patch due to subsequent changes, so I
> don't know if a simple revert fixes the problem. Log messages and
> bisect log see below.

Thanks for the heads up. Do you have a config file I can use to
reproduce this? The QEMU invocation runes would certainly help too.

It strikes me that in drivers/irqchip/irq-mips-cpu.c,
plat_irq_dispatch() now uses the irqdomain resolution before
irq_enter() took place. That's certainly a latent bug. I'll fix that
regardless, but I'd like to make sure this is what you are seeing too.

Thanks,

	M.
Guenter Roeck July 5, 2021, 6:23 p.m. UTC | #3
Hi Marc,

On 7/5/21 11:01 AM, Marc Zyngier wrote:
> Hi Guenter,
> 
> On Mon, 05 Jul 2021 18:23:52 +0100,
> Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Hi,
>>
>> On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote:
>>> The following commit has been merged into the irq/irqchip-next branch of irqchip:
>>>
>>> Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
>>> Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
>>> Author:        Marc Zyngier <maz@kernel.org>
>>> AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
>>> Committer:     Marc Zyngier <maz@kernel.org>
>>> CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00
>>>
>>> irqdomain: Protect the linear revmap with RCU
>>>
>>> It is pretty odd that the radix tree uses RCU while the linear
>>> portion doesn't, leading to potential surprises for the users,
>>> depending on how the irqdomain has been created.
>>>
>>> Fix this by moving the update of the linear revmap under
>>> the mutex, and the lookup under the RCU read-side lock.
>>>
>>> The mutex name is updated to reflect that it doesn't only
>>> cover the radix-tree anymore.
>>>
>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>
>> This patch results in various RCU warnings when booting mipsel images
>> in qemu. I can not revert the patch due to subsequent changes, so I
>> don't know if a simple revert fixes the problem. Log messages and
>> bisect log see below.
> 
> Thanks for the heads up. Do you have a config file I can use to
> reproduce this? The QEMU invocation runes would certainly help too.
> 
> It strikes me that in drivers/irqchip/irq-mips-cpu.c,
> plat_irq_dispatch() now uses the irqdomain resolution before
> irq_enter() took place. That's certainly a latent bug. I'll fix that
> regardless, but I'd like to make sure this is what you are seeing too.
> 

See http://server.roeck-us.net/qemu/mipsel/

config		Complete configuration file
defconfig	Shortened configuration file
rootfs.cpio	root file system (initrd)
run.sh		qemu run script (tested with qemu 4.2.1 and 6.0.0)
vmlinux		Kernel image experiencing the problem (v5.13-9883-gaf9efb8b661)

Hope this helps,
Guenter
Marc Zyngier July 5, 2021, 6:43 p.m. UTC | #4
On Mon, 05 Jul 2021 19:23:27 +0100,
Guenter Roeck <linux@roeck-us.net> wrote:
> 
> Hi Marc,
> 
> On 7/5/21 11:01 AM, Marc Zyngier wrote:
> > Hi Guenter,
> > 
> > On Mon, 05 Jul 2021 18:23:52 +0100,
> > Guenter Roeck <linux@roeck-us.net> wrote:
> >> 
> >> Hi,
> >> 
> >> On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote:
> >>> The following commit has been merged into the irq/irqchip-next branch of irqchip:
> >>> 
> >>> Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
> >>> Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
> >>> Author:        Marc Zyngier <maz@kernel.org>
> >>> AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
> >>> Committer:     Marc Zyngier <maz@kernel.org>
> >>> CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00
> >>> 
> >>> irqdomain: Protect the linear revmap with RCU
> >>> 
> >>> It is pretty odd that the radix tree uses RCU while the linear
> >>> portion doesn't, leading to potential surprises for the users,
> >>> depending on how the irqdomain has been created.
> >>> 
> >>> Fix this by moving the update of the linear revmap under
> >>> the mutex, and the lookup under the RCU read-side lock.
> >>> 
> >>> The mutex name is updated to reflect that it doesn't only
> >>> cover the radix-tree anymore.
> >>> 
> >>> Signed-off-by: Marc Zyngier <maz@kernel.org>
> >> 
> >> This patch results in various RCU warnings when booting mipsel images
> >> in qemu. I can not revert the patch due to subsequent changes, so I
> >> don't know if a simple revert fixes the problem. Log messages and
> >> bisect log see below.
> > 
> > Thanks for the heads up. Do you have a config file I can use to
> > reproduce this? The QEMU invocation runes would certainly help too.
> > 
> > It strikes me that in drivers/irqchip/irq-mips-cpu.c,
> > plat_irq_dispatch() now uses the irqdomain resolution before
> > irq_enter() took place. That's certainly a latent bug. I'll fix that
> > regardless, but I'd like to make sure this is what you are seeing too.
> > 
> 
> See http://server.roeck-us.net/qemu/mipsel/
> 
> config		Complete configuration file
> defconfig	Shortened configuration file
> rootfs.cpio	root file system (initrd)
> run.sh		qemu run script (tested with qemu 4.2.1 and 6.0.0)
> vmlinux		Kernel image experiencing the problem (v5.13-9883-gaf9efb8b661)
> 
> Hope this helps,

It definitely helps, and confirms my hunch. With the patch below, I'm
not getting the warnings anymore. I'm pretty sure a number of other
MIPS systems suffer from similar issues, which I'll address similarly.

Please let me know if that addresses the issue on your end.

Thanks,

	M.

diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
index d1477ecb1af9..57561e0e6e8d 100644
--- a/arch/mips/include/asm/irq.h
+++ b/arch/mips/include/asm/irq.h
@@ -57,6 +57,9 @@ asmlinkage void plat_irq_dispatch(void);
 
 extern void do_IRQ(unsigned int irq);
 
+struct irq_domain;
+extern void do_domain_IRQ(struct irq_domain *domain, unsigned int irq);
+
 extern void arch_init_irq(void);
 extern void spurious_interrupt(void);
 
diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
index 85b6c60f285d..c76005cd3b79 100644
--- a/arch/mips/kernel/irq.c
+++ b/arch/mips/kernel/irq.c
@@ -21,6 +21,7 @@
 #include <linux/kallsyms.h>
 #include <linux/kgdb.h>
 #include <linux/ftrace.h>
+#include <linux/irqdomain.h>
 
 #include <linux/atomic.h>
 #include <linux/uaccess.h>
@@ -107,3 +108,16 @@ void __irq_entry do_IRQ(unsigned int irq)
 	irq_exit();
 }
 
+void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
+{
+	struct irq_desc *desc;
+
+	irq_enter();
+	check_stack_overflow();
+
+	desc = irq_resolve_mapping(domain, hwirq);
+	if (likely(desc))
+		handle_irq_desc(desc);
+
+	irq_exit();
+}
diff --git a/drivers/irqchip/irq-mips-cpu.c b/drivers/irqchip/irq-mips-cpu.c
index 0bbb0b2d0dd5..0c7ae71a0af0 100644
--- a/drivers/irqchip/irq-mips-cpu.c
+++ b/drivers/irqchip/irq-mips-cpu.c
@@ -127,7 +127,6 @@ static struct irq_chip mips_mt_cpu_irq_controller = {
 asmlinkage void __weak plat_irq_dispatch(void)
 {
 	unsigned long pending = read_c0_cause() & read_c0_status() & ST0_IM;
-	unsigned int virq;
 	int irq;
 
 	if (!pending) {
@@ -137,12 +136,15 @@ asmlinkage void __weak plat_irq_dispatch(void)
 
 	pending >>= CAUSEB_IP;
 	while (pending) {
+		struct irq_domain *d;
+
 		irq = fls(pending) - 1;
 		if (IS_ENABLED(CONFIG_GENERIC_IRQ_IPI) && irq < 2)
-			virq = irq_linear_revmap(ipi_domain, irq);
+			d = ipi_domain;
 		else
-			virq = irq_linear_revmap(irq_domain, irq);
-		do_IRQ(virq);
+			d = irq_domain;
+
+		do_domain_IRQ(d, irq);
 		pending &= ~BIT(irq);
 	}
 }
Guenter Roeck July 5, 2021, 8:36 p.m. UTC | #5
On 7/5/21 11:43 AM, Marc Zyngier wrote:
> On Mon, 05 Jul 2021 19:23:27 +0100,
> Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> Hi Marc,
>>
>> On 7/5/21 11:01 AM, Marc Zyngier wrote:
>>> Hi Guenter,
>>>
>>> On Mon, 05 Jul 2021 18:23:52 +0100,
>>> Guenter Roeck <linux@roeck-us.net> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Fri, Jun 11, 2021 at 01:54:36PM -0000, irqchip-bot for Marc Zyngier wrote:
>>>>> The following commit has been merged into the irq/irqchip-next branch of irqchip:
>>>>>
>>>>> Commit-ID:     d4a45c68dc81f9117ceaff9f058d5fae674181b9
>>>>> Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms/d4a45c68dc81f9117ceaff9f058d5fae674181b9
>>>>> Author:        Marc Zyngier <maz@kernel.org>
>>>>> AuthorDate:    Mon, 05 Apr 2021 12:57:27 +01:00
>>>>> Committer:     Marc Zyngier <maz@kernel.org>
>>>>> CommitterDate: Thu, 10 Jun 2021 13:09:18 +01:00
>>>>>
>>>>> irqdomain: Protect the linear revmap with RCU
>>>>>
>>>>> It is pretty odd that the radix tree uses RCU while the linear
>>>>> portion doesn't, leading to potential surprises for the users,
>>>>> depending on how the irqdomain has been created.
>>>>>
>>>>> Fix this by moving the update of the linear revmap under
>>>>> the mutex, and the lookup under the RCU read-side lock.
>>>>>
>>>>> The mutex name is updated to reflect that it doesn't only
>>>>> cover the radix-tree anymore.
>>>>>
>>>>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>>>>
>>>> This patch results in various RCU warnings when booting mipsel images
>>>> in qemu. I can not revert the patch due to subsequent changes, so I
>>>> don't know if a simple revert fixes the problem. Log messages and
>>>> bisect log see below.
>>>
>>> Thanks for the heads up. Do you have a config file I can use to
>>> reproduce this? The QEMU invocation runes would certainly help too.
>>>
>>> It strikes me that in drivers/irqchip/irq-mips-cpu.c,
>>> plat_irq_dispatch() now uses the irqdomain resolution before
>>> irq_enter() took place. That's certainly a latent bug. I'll fix that
>>> regardless, but I'd like to make sure this is what you are seeing too.
>>>
>>
>> See http://server.roeck-us.net/qemu/mipsel/
>>
>> config		Complete configuration file
>> defconfig	Shortened configuration file
>> rootfs.cpio	root file system (initrd)
>> run.sh		qemu run script (tested with qemu 4.2.1 and 6.0.0)
>> vmlinux		Kernel image experiencing the problem (v5.13-9883-gaf9efb8b661)
>>
>> Hope this helps,
> 
> It definitely helps, and confirms my hunch. With the patch below, I'm
> not getting the warnings anymore. I'm pretty sure a number of other
> MIPS systems suffer from similar issues, which I'll address similarly.
> 
> Please let me know if that addresses the issue on your end.
> 

Yes, it does. Feel free to add

Tested-by: Guenter Roeck <linux@roeck-us.net>

to the real patch.

Now the big question: Why does this only affect 32-bit little endian
mips images, but not the matching big endian images, nor 64-bit images ?

Thanks,
Guenter

> Thanks,
> 
> 	M.
> 
> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
> index d1477ecb1af9..57561e0e6e8d 100644
> --- a/arch/mips/include/asm/irq.h
> +++ b/arch/mips/include/asm/irq.h
> @@ -57,6 +57,9 @@ asmlinkage void plat_irq_dispatch(void);
>   
>   extern void do_IRQ(unsigned int irq);
>   
> +struct irq_domain;
> +extern void do_domain_IRQ(struct irq_domain *domain, unsigned int irq);
> +
>   extern void arch_init_irq(void);
>   extern void spurious_interrupt(void);
>   
> diff --git a/arch/mips/kernel/irq.c b/arch/mips/kernel/irq.c
> index 85b6c60f285d..c76005cd3b79 100644
> --- a/arch/mips/kernel/irq.c
> +++ b/arch/mips/kernel/irq.c
> @@ -21,6 +21,7 @@
>   #include <linux/kallsyms.h>
>   #include <linux/kgdb.h>
>   #include <linux/ftrace.h>
> +#include <linux/irqdomain.h>
>   
>   #include <linux/atomic.h>
>   #include <linux/uaccess.h>
> @@ -107,3 +108,16 @@ void __irq_entry do_IRQ(unsigned int irq)
>   	irq_exit();
>   }
>   
> +void __irq_entry do_domain_IRQ(struct irq_domain *domain, unsigned int hwirq)
> +{
> +	struct irq_desc *desc;
> +
> +	irq_enter();
> +	check_stack_overflow();
> +
> +	desc = irq_resolve_mapping(domain, hwirq);
> +	if (likely(desc))
> +		handle_irq_desc(desc);
> +
> +	irq_exit();
> +}
> diff --git a/drivers/irqchip/irq-mips-cpu.c b/drivers/irqchip/irq-mips-cpu.c
> index 0bbb0b2d0dd5..0c7ae71a0af0 100644
> --- a/drivers/irqchip/irq-mips-cpu.c
> +++ b/drivers/irqchip/irq-mips-cpu.c
> @@ -127,7 +127,6 @@ static struct irq_chip mips_mt_cpu_irq_controller = {
>   asmlinkage void __weak plat_irq_dispatch(void)
>   {
>   	unsigned long pending = read_c0_cause() & read_c0_status() & ST0_IM;
> -	unsigned int virq;
>   	int irq;
>   
>   	if (!pending) {
> @@ -137,12 +136,15 @@ asmlinkage void __weak plat_irq_dispatch(void)
>   
>   	pending >>= CAUSEB_IP;
>   	while (pending) {
> +		struct irq_domain *d;
> +
>   		irq = fls(pending) - 1;
>   		if (IS_ENABLED(CONFIG_GENERIC_IRQ_IPI) && irq < 2)
> -			virq = irq_linear_revmap(ipi_domain, irq);
> +			d = ipi_domain;
>   		else
> -			virq = irq_linear_revmap(irq_domain, irq);
> -		do_IRQ(virq);
> +			d = irq_domain;
> +
> +		do_domain_IRQ(d, irq);
>   		pending &= ~BIT(irq);
>   	}
>   }
>
Marc Zyngier July 6, 2021, 9:24 a.m. UTC | #6
On Mon, 05 Jul 2021 21:36:36 +0100,
Guenter Roeck <linux@roeck-us.net> wrote:
> 
> On 7/5/21 11:43 AM, Marc Zyngier wrote:

> > It definitely helps, and confirms my hunch. With the patch below, I'm
> > not getting the warnings anymore. I'm pretty sure a number of other
> > MIPS systems suffer from similar issues, which I'll address similarly.
> > 
> > Please let me know if that addresses the issue on your end.
> > 
> 
> Yes, it does. Feel free to add
> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> to the real patch.

Thanks.

> Now the big question: Why does this only affect 32-bit little endian
> mips images, but not the matching big endian images, nor 64-bit images ?

Are you sure these images are using the exact same HW? A bunch (most?)
of MIPS systems do not use irqdomains in their root interrupt
handling, so this issue wouldn't be visible (irq_enter() will already
have been done for the chained interrupt handling).

FWIW, I can reproduce the problem by switching your mipsel config to
BE, and adding this patch fixes it.

Thanks,

	M.
Guenter Roeck July 6, 2021, 2:23 p.m. UTC | #7
On 7/6/21 2:24 AM, Marc Zyngier wrote:
> On Mon, 05 Jul 2021 21:36:36 +0100,
> Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On 7/5/21 11:43 AM, Marc Zyngier wrote:
> 
>>> It definitely helps, and confirms my hunch. With the patch below, I'm
>>> not getting the warnings anymore. I'm pretty sure a number of other
>>> MIPS systems suffer from similar issues, which I'll address similarly.
>>>
>>> Please let me know if that addresses the issue on your end.
>>>
>>
>> Yes, it does. Feel free to add
>>
>> Tested-by: Guenter Roeck <linux@roeck-us.net>
>>
>> to the real patch.
> 
> Thanks.
> 
>> Now the big question: Why does this only affect 32-bit little endian
>> mips images, but not the matching big endian images, nor 64-bit images ?
> 

Actually they do.

> Are you sure these images are using the exact same HW? A bunch (most?)
> of MIPS systems do not use irqdomains in their root interrupt
> handling, so this issue wouldn't be visible (irq_enter() will already
> have been done for the chained interrupt handling).
> 
> FWIW, I can reproduce the problem by switching your mipsel config to
> BE, and adding this patch fixes it.
> 

Yes, turns out I did not have the necessary debugging options enabled
in my other mips tests.

Thanks,
Guenter

Patch
diff mbox series

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 340cc04..2b696c9 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -151,6 +151,7 @@  struct irq_domain_chip_generic;
  * Revmap data, used internally by irq_domain
  * @revmap_size: Size of the linear map table @revmap[]
  * @revmap_tree: Radix map tree for hwirqs that don't fit in the linear map
+ * @revmap_mutex: Lock for the revmap
  * @revmap: Linear table of irq_data pointers
  */
 struct irq_domain {
@@ -173,8 +174,8 @@  struct irq_domain {
 	irq_hw_number_t hwirq_max;
 	unsigned int revmap_size;
 	struct radix_tree_root revmap_tree;
-	struct mutex revmap_tree_mutex;
-	struct irq_data *revmap[];
+	struct mutex revmap_mutex;
+	struct irq_data __rcu *revmap[];
 };
 
 /* Irq domain flags */
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 7a4e388..8fbadee 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -213,7 +213,7 @@  struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, int size,
 
 	/* Fill structure */
 	INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
-	mutex_init(&domain->revmap_tree_mutex);
+	mutex_init(&domain->revmap_mutex);
 	domain->ops = ops;
 	domain->host_data = host_data;
 	domain->hwirq_max = hwirq_max;
@@ -504,13 +504,12 @@  static void irq_domain_clear_mapping(struct irq_domain *domain,
 	if (irq_domain_is_nomap(domain))
 		return;
 
-	if (hwirq < domain->revmap_size) {
-		domain->revmap[hwirq] = NULL;
-	} else {
-		mutex_lock(&domain->revmap_tree_mutex);
+	mutex_lock(&domain->revmap_mutex);
+	if (hwirq < domain->revmap_size)
+		rcu_assign_pointer(domain->revmap[hwirq], NULL);
+	else
 		radix_tree_delete(&domain->revmap_tree, hwirq);
-		mutex_unlock(&domain->revmap_tree_mutex);
-	}
+	mutex_unlock(&domain->revmap_mutex);
 }
 
 static void irq_domain_set_mapping(struct irq_domain *domain,
@@ -520,13 +519,12 @@  static void irq_domain_set_mapping(struct irq_domain *domain,
 	if (irq_domain_is_nomap(domain))
 		return;
 
-	if (hwirq < domain->revmap_size) {
-		domain->revmap[hwirq] = irq_data;
-	} else {
-		mutex_lock(&domain->revmap_tree_mutex);
+	mutex_lock(&domain->revmap_mutex);
+	if (hwirq < domain->revmap_size)
+		rcu_assign_pointer(domain->revmap[hwirq], irq_data);
+	else
 		radix_tree_insert(&domain->revmap_tree, hwirq, irq_data);
-		mutex_unlock(&domain->revmap_tree_mutex);
-	}
+	mutex_unlock(&domain->revmap_mutex);
 }
 
 static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
@@ -911,12 +909,12 @@  unsigned int irq_find_mapping(struct irq_domain *domain,
 		return 0;
 	}
 
+	rcu_read_lock();
 	/* Check if the hwirq is in the linear revmap. */
 	if (hwirq < domain->revmap_size)
-		return domain->revmap[hwirq]->irq;
-
-	rcu_read_lock();
-	data = radix_tree_lookup(&domain->revmap_tree, hwirq);
+		data = rcu_dereference(domain->revmap[hwirq]);
+	else
+		data = radix_tree_lookup(&domain->revmap_tree, hwirq);
 	rcu_read_unlock();
 	return data ? data->irq : 0;
 }
@@ -1499,18 +1497,17 @@  static void irq_domain_fix_revmap(struct irq_data *d)
 	if (irq_domain_is_nomap(d->domain))
 		return;
 
+	/* Fix up the revmap. */
+	mutex_lock(&d->domain->revmap_mutex);
 	if (d->hwirq < d->domain->revmap_size) {
 		/* Not using radix tree */
-		d->domain->revmap[d->hwirq] = d;
-		return;
+		rcu_assign_pointer(d->domain->revmap[d->hwirq], d);
+	} else {
+		slot = radix_tree_lookup_slot(&d->domain->revmap_tree, d->hwirq);
+		if (slot)
+			radix_tree_replace_slot(&d->domain->revmap_tree, slot, d);
 	}
-
-	/* Fix up the revmap. */
-	mutex_lock(&d->domain->revmap_tree_mutex);
-	slot = radix_tree_lookup_slot(&d->domain->revmap_tree, d->hwirq);
-	if (slot)
-		radix_tree_replace_slot(&d->domain->revmap_tree, slot, d);
-	mutex_unlock(&d->domain->revmap_tree_mutex);
+	mutex_unlock(&d->domain->revmap_mutex);
 }
 
 /**