From: Cornelia Huck <cohuck@redhat.com>
To: Jason Wang <jasowang@redhat.com>,
jasowang@redhat.com, mst@redhat.com,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Cc: lulu@redhat.com, paulmck@kernel.org, peterz@infradead.org,
maz@kernel.org, pasic@linux.ibm.com, eperezma@redhat.com,
tglx@linutronix.de
Subject: Re: [PATCH V4 0/9] rework on the IRQ hardening of virtio
Date: Tue, 10 May 2022 11:29:30 +0200 [thread overview]
Message-ID: <875ymd3fd1.fsf@redhat.com> (raw)
In-Reply-To: <20220507071954.14455-1-jasowang@redhat.com>
On Sat, May 07 2022, Jason Wang <jasowang@redhat.com> wrote:
> Hi All:
>
> This is a rework on the IRQ hardening for virtio which is done
> previously by the following commits are reverted:
>
> 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
> 080cd7c3ac87 ("virtio-pci: harden INTX interrupts")
>
> The reason is that it depends on the IRQF_NO_AUTOEN which may conflict
> with the assumption of the affinity managed IRQ that is used by some
> virtio drivers. And what's more, it is only done for virtio-pci but
> not other transports.
>
> In this rework, I try to implement a general virtio solution which
> borrows the idea of the INTX hardening by re-using per virtqueue
> boolean vq->broken and toggle it in virtio_device_ready() and
> virtio_reset_device(). Then we can simply reuse the existing checks in
> the vring_interrupt() and return early if the driver is not ready.
>
> Note that, I only did compile test on ccw and MMIO transport.
Lockdep is unhappy with the ccw parts:
================================
WARNING: inconsistent lock state
5.18.0-rc6+ #191 Not tainted
--------------------------------
inconsistent {IN-HARDIRQ-R} -> {HARDIRQ-ON-W} usage.
kworker/u4:0/9 [HC0[0]:SC0[0]:HE1:SE1] takes:
00000000058e9618 (&vcdev->irq_lock){+-..}-{2:2}, at: virtio_ccw_synchronize_cbs+0x4e/0x60
{IN-HARDIRQ-R} state was registered at:
__lock_acquire+0x442/0xc20
lock_acquire.part.0+0xdc/0x228
lock_acquire+0xa6/0x1b0
_raw_read_lock_irqsave+0x72/0x100
virtio_ccw_int_handler+0x84/0x238
ccw_device_call_handler+0x72/0xd0
ccw_device_irq+0x7a/0x198
do_cio_interrupt+0x11c/0x1d0
__handle_irq_event_percpu+0xc2/0x318
handle_irq_event_percpu+0x26/0x68
handle_percpu_irq+0x64/0x88
generic_handle_irq+0x40/0x58
do_irq_async+0x56/0xb0
do_io_irq+0x82/0x160
io_int_handler+0xe6/0x120
rcu_read_lock_sched_held+0x3e/0xb0
lock_acquired+0x12e/0x208
new_inode+0x3e/0xd0
debugfs_get_inode+0x22/0x68
__debugfs_create_file+0x78/0x1c0
debugfs_create_file_unsafe+0x36/0x58
debugfs_create_u32+0x38/0x68
sched_init_debug+0xb0/0x1c0
do_one_initcall+0x108/0x280
do_initcalls+0x124/0x148
kernel_init_freeable+0x242/0x280
kernel_init+0x2e/0x158
__ret_from_fork+0x3c/0x50
ret_from_fork+0xa/0x40
irq event stamp: 539789
hardirqs last enabled at (539789): [<0000000000d9c632>] _raw_spin_unlock_irqrestore+0x72/0x88
hardirqs last disabled at (539788): [<0000000000d9c2b6>] _raw_spin_lock_irqsave+0x96/0xd0
softirqs last enabled at (539568): [<0000000000d9e0d4>] __do_softirq+0x434/0x588
softirqs last disabled at (539503): [<000000000018cd66>] __irq_exit_rcu+0x146/0x170
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&vcdev->irq_lock);
<Interrupt>
lock(&vcdev->irq_lock);
*** DEADLOCK ***
2 locks held by kworker/u4:0/9:
#0: 000000000288d948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x1ea/0x658
#1: 000003800004bdc8 ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work+0x1ea/0x658
stack backtrace:
CPU: 1 PID: 9 Comm: kworker/u4:0 Not tainted 5.18.0-rc6+ #191
Hardware name: QEMU 8561 QEMU (KVM/Linux)
Workqueue: events_unbound async_run_entry_fn
Call Trace:
[<0000000000d8af22>] dump_stack_lvl+0x92/0xd0
[<00000000002032ac>] mark_lock_irq+0x864/0x968
[<0000000000203670>] mark_lock.part.0+0x2c0/0x790
[<0000000000203cea>] mark_usage+0x10a/0x178
[<000000000020692a>] __lock_acquire+0x442/0xc20
[<0000000000207cc4>] lock_acquire.part.0+0xdc/0x228
[<0000000000207eb6>] lock_acquire+0xa6/0x1b0
[<0000000000d9c774>] _raw_write_lock+0x54/0xa8
[<0000000000d5a1f6>] virtio_ccw_synchronize_cbs+0x4e/0x60
[<00000000008eec04>] register_virtio_device+0xdc/0x1b0
[<0000000000d5aabe>] virtio_ccw_online+0x246/0x2e8
[<0000000000c9fecc>] ccw_device_set_online+0x1c4/0x540
[<0000000000d5a05e>] virtio_ccw_auto_online+0x26/0x50
[<00000000001ba2b0>] async_run_entry_fn+0x40/0x108
[<00000000001ab9b4>] process_one_work+0x2a4/0x658
[<00000000001abdd0>] worker_thread+0x68/0x440
[<00000000001b4668>] kthread+0x128/0x130
[<0000000000102fac>] __ret_from_fork+0x3c/0x50
[<0000000000d9d3aa>] ret_from_fork+0xa/0x40
INFO: lockdep is turned off.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
WARNING: multiple messages have this Message-ID (diff)
From: Cornelia Huck <cohuck@redhat.com>
To: Jason Wang <jasowang@redhat.com>,
jasowang@redhat.com, mst@redhat.com,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Cc: tglx@linutronix.de, peterz@infradead.org, paulmck@kernel.org,
maz@kernel.org, pasic@linux.ibm.com, eperezma@redhat.com,
lulu@redhat.com, sgarzare@redhat.com, xuanzhuo@linux.alibaba.com
Subject: Re: [PATCH V4 0/9] rework on the IRQ hardening of virtio
Date: Tue, 10 May 2022 11:29:30 +0200 [thread overview]
Message-ID: <875ymd3fd1.fsf@redhat.com> (raw)
In-Reply-To: <20220507071954.14455-1-jasowang@redhat.com>
On Sat, May 07 2022, Jason Wang <jasowang@redhat.com> wrote:
> Hi All:
>
> This is a rework on the IRQ hardening for virtio which is done
> previously by the following commits are reverted:
>
> 9e35276a5344 ("virtio_pci: harden MSI-X interrupts")
> 080cd7c3ac87 ("virtio-pci: harden INTX interrupts")
>
> The reason is that it depends on the IRQF_NO_AUTOEN which may conflict
> with the assumption of the affinity managed IRQ that is used by some
> virtio drivers. And what's more, it is only done for virtio-pci but
> not other transports.
>
> In this rework, I try to implement a general virtio solution which
> borrows the idea of the INTX hardening by re-using per virtqueue
> boolean vq->broken and toggle it in virtio_device_ready() and
> virtio_reset_device(). Then we can simply reuse the existing checks in
> the vring_interrupt() and return early if the driver is not ready.
>
> Note that, I only did compile test on ccw and MMIO transport.
Lockdep is unhappy with the ccw parts:
================================
WARNING: inconsistent lock state
5.18.0-rc6+ #191 Not tainted
--------------------------------
inconsistent {IN-HARDIRQ-R} -> {HARDIRQ-ON-W} usage.
kworker/u4:0/9 [HC0[0]:SC0[0]:HE1:SE1] takes:
00000000058e9618 (&vcdev->irq_lock){+-..}-{2:2}, at: virtio_ccw_synchronize_cbs+0x4e/0x60
{IN-HARDIRQ-R} state was registered at:
__lock_acquire+0x442/0xc20
lock_acquire.part.0+0xdc/0x228
lock_acquire+0xa6/0x1b0
_raw_read_lock_irqsave+0x72/0x100
virtio_ccw_int_handler+0x84/0x238
ccw_device_call_handler+0x72/0xd0
ccw_device_irq+0x7a/0x198
do_cio_interrupt+0x11c/0x1d0
__handle_irq_event_percpu+0xc2/0x318
handle_irq_event_percpu+0x26/0x68
handle_percpu_irq+0x64/0x88
generic_handle_irq+0x40/0x58
do_irq_async+0x56/0xb0
do_io_irq+0x82/0x160
io_int_handler+0xe6/0x120
rcu_read_lock_sched_held+0x3e/0xb0
lock_acquired+0x12e/0x208
new_inode+0x3e/0xd0
debugfs_get_inode+0x22/0x68
__debugfs_create_file+0x78/0x1c0
debugfs_create_file_unsafe+0x36/0x58
debugfs_create_u32+0x38/0x68
sched_init_debug+0xb0/0x1c0
do_one_initcall+0x108/0x280
do_initcalls+0x124/0x148
kernel_init_freeable+0x242/0x280
kernel_init+0x2e/0x158
__ret_from_fork+0x3c/0x50
ret_from_fork+0xa/0x40
irq event stamp: 539789
hardirqs last enabled at (539789): [<0000000000d9c632>] _raw_spin_unlock_irqrestore+0x72/0x88
hardirqs last disabled at (539788): [<0000000000d9c2b6>] _raw_spin_lock_irqsave+0x96/0xd0
softirqs last enabled at (539568): [<0000000000d9e0d4>] __do_softirq+0x434/0x588
softirqs last disabled at (539503): [<000000000018cd66>] __irq_exit_rcu+0x146/0x170
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&vcdev->irq_lock);
<Interrupt>
lock(&vcdev->irq_lock);
*** DEADLOCK ***
2 locks held by kworker/u4:0/9:
#0: 000000000288d948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x1ea/0x658
#1: 000003800004bdc8 ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work+0x1ea/0x658
stack backtrace:
CPU: 1 PID: 9 Comm: kworker/u4:0 Not tainted 5.18.0-rc6+ #191
Hardware name: QEMU 8561 QEMU (KVM/Linux)
Workqueue: events_unbound async_run_entry_fn
Call Trace:
[<0000000000d8af22>] dump_stack_lvl+0x92/0xd0
[<00000000002032ac>] mark_lock_irq+0x864/0x968
[<0000000000203670>] mark_lock.part.0+0x2c0/0x790
[<0000000000203cea>] mark_usage+0x10a/0x178
[<000000000020692a>] __lock_acquire+0x442/0xc20
[<0000000000207cc4>] lock_acquire.part.0+0xdc/0x228
[<0000000000207eb6>] lock_acquire+0xa6/0x1b0
[<0000000000d9c774>] _raw_write_lock+0x54/0xa8
[<0000000000d5a1f6>] virtio_ccw_synchronize_cbs+0x4e/0x60
[<00000000008eec04>] register_virtio_device+0xdc/0x1b0
[<0000000000d5aabe>] virtio_ccw_online+0x246/0x2e8
[<0000000000c9fecc>] ccw_device_set_online+0x1c4/0x540
[<0000000000d5a05e>] virtio_ccw_auto_online+0x26/0x50
[<00000000001ba2b0>] async_run_entry_fn+0x40/0x108
[<00000000001ab9b4>] process_one_work+0x2a4/0x658
[<00000000001abdd0>] worker_thread+0x68/0x440
[<00000000001b4668>] kthread+0x128/0x130
[<0000000000102fac>] __ret_from_fork+0x3c/0x50
[<0000000000d9d3aa>] ret_from_fork+0xa/0x40
INFO: lockdep is turned off.
next prev parent reply other threads:[~2022-05-10 9:29 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-07 7:19 [PATCH V4 0/9] rework on the IRQ hardening of virtio Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 1/9] virtio: use virtio_device_ready() in virtio_device_restore() Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-09 15:22 ` Cornelia Huck
2022-05-09 15:22 ` Cornelia Huck
2022-05-10 1:50 ` Jason Wang
2022-05-10 1:50 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 2/9] virtio: use virtio_reset_device() when possible Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 3/9] virtio: introduce config op to synchronize vring callbacks Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-09 15:24 ` Cornelia Huck
2022-05-09 15:24 ` Cornelia Huck
2022-05-07 7:19 ` [PATCH V4 4/9] virtio-pci: implement synchronize_cbs() Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-09 15:26 ` Cornelia Huck
2022-05-09 15:26 ` Cornelia Huck
2022-05-07 7:19 ` [PATCH V4 5/9] virtio-mmio: " Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-09 15:34 ` Cornelia Huck
2022-05-09 15:34 ` Cornelia Huck
2022-05-07 7:19 ` [PATCH V4 6/9] virtio-ccw: " Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-10 11:27 ` Michael S. Tsirkin
2022-05-10 11:27 ` Michael S. Tsirkin
2022-05-11 2:41 ` Jason Wang
2022-05-11 2:41 ` Jason Wang
2022-05-11 8:17 ` Cornelia Huck
2022-05-11 8:17 ` Cornelia Huck
2022-05-11 8:58 ` Jason Wang
2022-05-11 8:58 ` Jason Wang
2022-05-11 9:13 ` Cornelia Huck
2022-05-11 9:13 ` Cornelia Huck
2022-05-11 9:28 ` Jason Wang
2022-05-11 9:28 ` Jason Wang
2022-05-11 14:52 ` Vineeth Vijayan
2022-05-12 3:29 ` Jason Wang
2022-05-12 3:29 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 7/9] virtio: allow to unbreak virtqueue Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 8/9] virtio: harden vring IRQ Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-10 11:32 ` Michael S. Tsirkin
2022-05-10 11:32 ` Michael S. Tsirkin
2022-05-11 2:40 ` Jason Wang
2022-05-11 2:40 ` Jason Wang
2022-05-11 8:44 ` Cornelia Huck
2022-05-11 8:44 ` Cornelia Huck
2022-05-11 9:27 ` Jason Wang
2022-05-11 9:27 ` Jason Wang
2022-05-11 12:49 ` Halil Pasic
2022-05-11 12:49 ` Halil Pasic
2022-05-12 3:27 ` Jason Wang
2022-05-12 3:27 ` Jason Wang
2022-05-07 7:19 ` [PATCH V4 9/9] virtio: use WARN_ON() to warning illegal status value Jason Wang
2022-05-07 7:19 ` Jason Wang
2022-05-10 9:29 ` Cornelia Huck [this message]
2022-05-10 9:29 ` [PATCH V4 0/9] rework on the IRQ hardening of virtio Cornelia Huck
2022-05-11 2:22 ` Jason Wang
2022-05-11 2:22 ` Jason Wang
2022-05-11 14:01 ` Halil Pasic
2022-05-11 14:01 ` Halil Pasic
2022-05-12 3:31 ` Jason Wang
2022-05-12 3:31 ` Jason Wang
2022-05-16 11:20 ` Halil Pasic
2022-05-16 11:20 ` Halil Pasic
2022-05-16 14:25 ` Michael S. Tsirkin
2022-05-16 14:25 ` Michael S. Tsirkin
2022-05-17 1:00 ` Jason Wang
2022-05-17 1:00 ` Jason Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=875ymd3fd1.fsf@redhat.com \
--to=cohuck@redhat.com \
--cc=eperezma@redhat.com \
--cc=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lulu@redhat.com \
--cc=maz@kernel.org \
--cc=mst@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=virtualization@lists.linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.