Re: [PATCH v3 2/2] vfio/pci: Remove vfio_device_get_from_dev()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 5/5/2022 12:31 AM, Jason Gunthorpe wrote:
The last user of this function is in PCI callbacks that want to convert
their struct pci_dev to a vfio_device. Instead of searching use the
vfio_device available trivially through the drvdata.

When a callback in the device_driver is called, the caller must hold the
device_lock() on dev. The purpose of the device_lock is to prevent
remove() from being called (see __device_release_driver), and allow the
driver to safely interact with its drvdata without races.

The PCI core correctly follows this and holds the device_lock() when
calling error_detected (see report_error_detected) and
sriov_configure (see sriov_numvfs_store).

Further, since the drvdata holds a positive refcount on the vfio_device
any access of the drvdata, under the device_lock(), from a driver callback
needs no further protection or refcounting.

Thus the remark in the vfio_device_get_from_dev() comment does not apply
here, VFIO PCI drivers all call vfio_unregister_group_dev() from their
remove callbacks under the device_lock() and cannot race with the
remaining callers.

Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
Reviewed-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
Reviewed-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

My ack was on previous version where drvdata is set and used in same module.
 https://www.spinics.net/lists/kvm/msg275737.html

Its not a good practice to force vfio_pci_core vendor drivers to set a particular structure pointer in drvdata. drvdata set from a driver should be used by same driver and other driver should not assume/rely on it.

I still prefer earlier version.

Thanks,
Kirti




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux