From: Wei Gong <gongwei833x@gmail.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
linux-kernel@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org
Subject: Re: [PATCH v2] pci: fix device presence detection for VFs
Date: Fri, 11 Nov 2022 04:00:54 +0000 [thread overview]
Message-ID: <20221111035851.GA40662@zander> (raw)
In-Reply-To: <20221110193547.GA631336@bhelgaas>
On Thu, Nov 10, 2022 at 01:35:47PM -0600, Bjorn Helgaas wrote:
> Hi Wei,
>
> I can't quite parse this. Is the problem that you had some virtio I/O
> in progress, you wrote "0" to /sys/.../sriov_numvfs, and the virtio
> I/O operation hangs?
It is, and has the same effect when unbind driver.
>
> Is there any indication to the user, e.g., softlockup oops?
There is no more user output than the Trace stack listed below.
>
> More questions below.
>
> On Tue, Nov 08, 2022 at 04:52:16AM +0000, Wei Gong 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.
>
> I want to follow along in the code, but I need some hints.
>
> "queue_usage_counter" looks like it's supposed to be a symbol, but I
> can't find it.
>
> Where (which function) is the I/O dispatched and queue_usage_counter
> incremented? Where is queue_usage_counter decremented?
queue_usage_counter is blk core q->q_usage_counter.
In blk_mq:
submit_bio
-> submit_bio_noacct
-> __submit_bio_noacct_mq
-> bio_queue_enter
bio_queue_enter: q_usage_counter++
Complete io:
vp_vring_interrupt
-> vring_interrupt ( if (vq->broken) { return; } )
-> virtblk_done
-> blk_mq_complete_request
-> virtblk_request_done
-> blk_mq_end_request
-> blk_mq_free_request
-> blk_queue_exit
blk_queue_exit: q_usage_counter--
>
> Prior to this change pci_device_is_present(VF) returned "false"
> (because the VF Vendor ID is 0xffff); after the change it will return
> "true" (because it will look at the PF Vendor ID instead).
>
> Previously virtio_pci_remove() called virtio_break_device(). I guess
> that meant the virtio I/O operation will never be completed?
>
> But if we don't call virtio_break_device(), the virtio I/O operation
> *will* be completed?
>
> Bjorn
I think so.
In virtblk I think I don't want virtio_break_device() to be called
because it need to complete, but it would be well if it could be
called at the right time.
The problem new is that it's being called in a situation where it
shouldn't, and that's because pci_device_is_present() is giving
incorrect return.
Wei
next prev parent reply other threads:[~2022-11-11 4:03 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 6:11 [PATCH v2] pci: fix device presence detection for VFs Michael S. Tsirkin
2022-10-26 13:46 ` Wei Gong
2022-11-08 4:52 ` Wei Gong
2022-11-08 4:58 ` Michael S. Tsirkin
2022-11-08 5:06 ` Bjorn Helgaas
2022-11-10 19:35 ` Bjorn Helgaas
2022-11-10 20:15 ` Michael S. Tsirkin
2022-11-11 23:42 ` Bjorn Helgaas
2022-11-13 8:46 ` Michael S. Tsirkin
2022-11-15 16:24 ` Bjorn Helgaas
2022-11-16 11:16 ` Lukas Wunner
2022-11-17 5:36 ` Parav Pandit
2022-12-19 5:56 ` Michael S. Tsirkin
2022-12-19 8:22 ` Lukas Wunner
2022-11-11 4:00 ` Wei Gong [this message]
2022-11-08 14:53 ` Bjorn Helgaas
2022-11-08 15:02 ` Bjorn Helgaas
2022-11-08 15:19 ` Michael S. Tsirkin
2022-11-08 17:58 ` Bjorn Helgaas
2022-11-08 18:02 ` Michael S. Tsirkin
2022-11-09 4:36 ` Wei Gong
2022-11-09 5:12 ` Bjorn Helgaas
2022-11-09 7:00 ` Wei Gong
2022-11-09 7:10 ` Michael S. Tsirkin
2022-11-09 17:30 ` Bjorn Helgaas
2022-11-09 17:49 ` Michael S. Tsirkin
2022-11-11 23:39 ` Bjorn Helgaas
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=20221111035851.GA40662@zander \
--to=gongwei833x@gmail.com \
--cc=bhelgaas@google.com \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mst@redhat.com \
/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).