From: "Michael S. Tsirkin" <mst@redhat.com>
To: gongwei <gongwei833x@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio_pci: don't hang when vf dev is disabled
Date: Mon, 10 Oct 2022 13:13:02 -0400 [thread overview]
Message-ID: <20221009151308-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20221008143656.154616-1-gongwei833x@gmail.com>
On Sat, Oct 08, 2022 at 02:36:56PM +0000, gongwei wrote:
> according to sriov's protocol specification vendor_id and
> device_id field in all VFs return FFFFh when read
> so when vf devs is in the pci_device_is_present,it will be
> misjudged as surprise removeal
>
> when io is issued on the vf, normally disable virtio_blk vf
> devs,at this time the disable opration will hang. and virtio
> blk dev io hang.
>
> task:bash state:D stack: 0 pid: 1773 ppid: 1241 flags:0x00004002
> Call Trace:
> <TASK>
> __schedule+0x2ee/0x900
> schedule+0x4f/0xc0
> blk_mq_freeze_queue_wait+0x69/0xa0
> ? wait_woken+0x80/0x80
> blk_mq_freeze_queue+0x1b/0x20
> blk_cleanup_queue+0x3d/0xd0
> virtblk_remove+0x3c/0xb0 [virtio_blk]
> virtio_dev_remove+0x4b/0x80
> device_release_driver_internal+0x103/0x1d0
> device_release_driver+0x12/0x20
> bus_remove_device+0xe1/0x150
> device_del+0x192/0x3f0
> device_unregister+0x1b/0x60
> unregister_virtio_device+0x18/0x30
> virtio_pci_remove+0x41/0x80
> pci_device_remove+0x3e/0xb0
> device_release_driver_internal+0x103/0x1d0
> device_release_driver+0x12/0x20
> pci_stop_bus_device+0x79/0xa0
> pci_stop_and_remove_bus_device+0x13/0x20
> pci_iov_remove_virtfn+0xc5/0x130
> ? pci_get_device+0x4a/0x60
> sriov_disable+0x33/0xf0
> pci_disable_sriov+0x26/0x30
> virtio_pci_sriov_configure+0x6f/0xa0
> sriov_numvfs_store+0x104/0x140
> dev_attr_store+0x17/0x30
> sysfs_kf_write+0x3e/0x50
> kernfs_fop_write_iter+0x138/0x1c0
> new_sync_write+0x117/0x1b0
> vfs_write+0x185/0x250
> ksys_write+0x67/0xe0
> __x64_sys_write+0x1a/0x20
> do_syscall_64+0x61/0xb0
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7f21bd1f3ba4
> RSP: 002b:00007ffd34a24188 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f21bd1f3ba4
> RDX: 0000000000000002 RSI: 0000560305040800 RDI: 0000000000000001
> RBP: 0000560305040800 R08: 000056030503fd50 R09: 0000000000000073
> R10: 00000000ffffffff R11: 0000000000000202 R12: 0000000000000002
> R13: 00007f21bd2de760 R14: 00007f21bd2da5e0 R15: 00007f21bd2d99e0
>
> when virtio_blk is performing io, as long as there two stages of:
> 1. dispatch io. queue_usage_counter++;
> 2. io is completed after receiving the interrupt. queue_usage_counter--;
>
> disable virtio_blk vfs:
> if(!pci_device_is_present(pci_dev))
> virtio_break_device(&vp_dev->vdev);
> virtqueue for vf devs will be marked broken.
> the interrupt notification io is end. Since it is judged that the
> virtqueue has been marked as broken, the completed io will not be
> performed.
> So queue_usage_counter will not be cleared.
> when the disk is removed at the same time, the queue will be frozen,
> and you must wait for the queue_usage_counter to be cleared.
> Therefore, it leads to the removeal of hang.
>
> Signed-off-by: gongwei <gongwei833x@gmail.com>
Hmm I think this is wrong. First is_physfn is only set for SRIOV.
Second it seems to me the bug is really in the pci core.
I just sent a patch for that - can you help test please?
Third can you pls sign patches with your full name?
Forth - I could not figure out if there is still a bug
in virtio besides that. Does surprise removal work ok for you?
> ---
> drivers/virtio/virtio_pci_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index fdbde1db5ec5..8cbc529fb582 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -616,7 +616,7 @@ static void virtio_pci_remove(struct pci_dev *pci_dev)
> * Device is marked broken on surprise removal so that virtio upper
> * layers can abort any ongoing operation.
> */
> - if (!pci_device_is_present(pci_dev))
> + if (!pci_device_is_present(pci_dev) && pci_dev->is_physfn)
> virtio_break_device(&vp_dev->vdev);
>
> pci_disable_sriov(pci_dev);
> --
> 2.25.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
parent reply other threads:[~2022-10-10 17:13 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20221008143656.154616-1-gongwei833x@gmail.com>]
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=20221009151308-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=gongwei833x@gmail.com \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).