On Fri, Jul 23, 2021 at 09:47:49AM +0200, Christoph Hellwig wrote:

...

@@ -2020,12 +2004,17 @@ static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) INIT_LIST_HEAD(&vdev->vma_list); init_rwsem(&vdev->memory_lock);

• ret = vfio_pci_reflck_attach(vdev);

• if (pci_is_root_bus(pdev->bus))

• ret = vfio_assign_device_set(&vdev->vdev, vdev);
  

• else if (!pci_probe_reset_slot(pdev->slot))

• ret = vfio_assign_device_set(&vdev->vdev, pdev->slot);
  

• else

• ret = vfio_assign_device_set(&vdev->vdev, pdev->bus);
  

Maybe invert this and add a comment:

if (pci_is_root_bus(pdev->bus)) ret = vfio_assign_device_set(&vdev->vdev, vdev); /* * If there is no slot reset support for this device, the whole * bus needs to be grouped together to support bus-wide resets. */

I like the comment

else if (pci_probe_reset_slot(pdev->slot) < 0) ret = vfio_assign_device_set(&vdev->vdev, pdev->bus); else ret = vfio_assign_device_set(&vdev->vdev, pdev->slot);

The consensus idiom here is the !:

drivers/pci/pci.c: return (!pci_probe_reset_slot(pdev->slot)) ? drivers/vfio/pci/vfio_pci.c: if (!pci_probe_reset_slot(vdev->pdev->slot)) drivers/vfio/pci/vfio_pci.c: if (!pci_probe_reset_slot(vdev->pdev->slot)) drivers/vfio/pci/vfio_pci.c: bool slot = !pci_probe_reset_slot(vdev->pdev->slot); drivers/vfio/pci/vfio_pci.c: if (!pci_probe_reset_slot(vdev->pdev->slot))

It reads quite poorly either way, IMHO, I'd rather switch it to a readable bool, but not for this series

Thanks, Jason