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
