[cc += Parav Pandit, author of 43bb40c5b926] On Sun, Nov 13, 2022 at 03:46:06AM -0500, Michael S. Tsirkin wrote: > On Fri, Nov 11, 2022 at 05:42:19PM -0600, Bjorn Helgaas wrote: > > On Thu, Nov 10, 2022 at 03:15:55PM -0500, Michael S. Tsirkin wrote: > > > On Thu, Nov 10, 2022 at 01:35:47PM -0600, Bjorn Helgaas wrote: > > > > 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? > > Just making sure - pci_device_is_present *is* the suggested way > to distinguish between graceful and surprise removal, isn't it? No, it's not. Instead of !pci_device_is_present() you really want to call pci_dev_is_disconnected() instead. While the fix Bjorn applied for v6.2 may solve the issue and may make sense on it's own, it's not the solution you're looking for. You want to swap the call to !pci_device_is_present() with pci_dev_is_disconnected(), move pci_dev_is_disconnected() from drivers/pci/pci.h to include/linux/pci.h and add a Fixes tag referencing 43bb40c5b926. If you don't want to move pci_dev_is_disconnected(), you can alternatively check for "pdev->error_state == pci_channel_io_perm_failure" or call pci_channel_offline(). The latter will also return true though on transient inaccessibility of the device (e.g. if it's being reset). The theory of operation is as follows: The PCI layer does indeed know whether the device was surprise removed or gracefully removed and that information is passed in the "presence" flag to pciehp_unconfigure_device() (in drivers/pci/hotplug/pciehp_pci.c). That function does the following: if (!presence) pci_walk_bus(parent, pci_dev_set_disconnected, NULL); In other words, pdev->error_state is set to pci_channel_io_perm_failure on the entire hierarchy below the hotplug port. And pci_dev_is_disconnected() simply checks whether that's the device's error_state. pci_dev_is_disconnected() makes sense if you definitely know the device is gone and want to skip certain steps or delays on device teardown. However be aware that the device may be hot-removed after graceful removal was initiated. In such a situation, pci_dev_is_disconnected() may return false and you'll try to access the device as normal, even though it was yanked from the slot after the pci_dev_is_disconnected() call was performed. Ideally you should be able to cope with such scenarios as well. For some more background info, refer to this LWN article (scroll down to the "Surprise removal" section): https://lwn.net/Articles/767885/ Thanks, Lukas
