On Mon, 23 Aug 2021 18:28:49 +0300
Max Gurtovoy [off-list ref] wrote:

On 8/23/2021 6:16 PM, Alex Williamson wrote:

quoted

On Sun, 22 Aug 2021 17:35:55 +0300
Yishai Hadas [off-list ref] wrote:  

quoted

diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
new file mode 100644
index 000000000000..15474ebadd98
--- /dev/null
+++ b/drivers/vfio/pci/vfio_pci.c  

...  

quoted

+static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
+{
+	might_sleep();
+
+	if (!enable_sriov)
+		return -ENOENT;
+
+	return vfio_pci_core_sriov_configure(pdev, nr_virtfn);
+}  

As noted in previous version, why do we need the might_sleep() above
when the core code below includes it and there's nothing above that
might sleep before that?  Thanks,  

This is used to mention vfio_pci_core_sriov_configure might sleep.

If this is redundant, can you please remove this one line upon merge ?

I guess I'm not sure how far up we need to, or should, percolate
might_sleep() annotations.  vfio_pci_core_sriov_configure() calls
vfio_device_get_from_dev() which makes use of mutexes, which I think is
the original reason for the annotation there ahead of those in the PCI
iov code.  But is the annotation through mutex_lock() enough on its own,
ie. should we remove all of our gratuitous annotations in the vfio part
of the code path?  Thanks,

Alex

quoted

quoted

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 94f062818e0c..87d1960d0d61 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c  

...  

quoted

-static int vfio_pci_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
+int vfio_pci_core_sriov_configure(struct pci_dev *pdev, int nr_virtfn)
  {
  	struct vfio_device *device;
  	int ret = 0;
  
  	might_sleep();
  
-	if (!enable_sriov)
-		return -ENOENT;
-
  	device = vfio_device_get_from_dev(&pdev->dev);
  	if (!device)
  		return -ENODEV;