On Mon, 2014-03-03 at 11:33 -0300, Thadeu Lima de Souza Cascardo wrote: > When we unbind vfio-pci from a device, while running a guest, we might > have a deadlock when such a guest reboots. > > Unbind takes device_lock at device_release_driver, and waits for > release_q at vfio_del_group_dev. > > release_q will only be woken up when all references to vfio_device are > gone, and that includes open file descriptors, like the ones a guest > on qemu will hold. > > If you try to reboot the guest, it will call VFIO_DEVICE_RESET, which > calls pci_reset_function, which now grabs the device_lock, and we are > deadlocked. > > Using device_trylock allow us to handle the case when the lock is > already taken, and avoid this situation. > > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxxxxxxxxxx> > --- > > Not tested yet, but I would like some comments now, like would it be > better to have a pci_try_reset_function, or do trylock on > pci_reset_function itself? We already have it: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=61cf16d8bd38c3dc52033ea75d5b1f8368514a17 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=890ed578df82f5b7b5a874f9f2fa4f117305df5f Is there something insufficient about these or are you testing on and older kernel? Thanks, Alex > --- > drivers/vfio/pci/vfio_pci.c | 14 ++++++++++++-- > 1 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c > index 3b76dc8..d1d2242 100644 > --- a/drivers/vfio/pci/vfio_pci.c > +++ b/drivers/vfio/pci/vfio_pci.c > @@ -513,8 +513,18 @@ static long vfio_pci_ioctl(void *device_data, > return ret; > > } else if (cmd == VFIO_DEVICE_RESET) { > - return vdev->reset_works ? > - pci_reset_function(vdev->pdev) : -EINVAL; > + struct pci_dev *pdev = vdev->pdev; > + int ret = -EBUSY; > + if (!vdev->reset_works) > + return -EINVAL; > + if (pci_cfg_access_trylock(pdev)) { > + if (device_trylock(&pdev->dev)) { > + ret = __pci_reset_function_locked(pdev); > + device_unlock(&pdev->dev); > + } > + pci_cfg_access_unlock(pdev); > + } > + return ret; > > } else if (cmd == VFIO_DEVICE_GET_PCI_HOT_RESET_INFO) { > struct vfio_pci_hot_reset_info hdr; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
