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 <casca...@linux.vnet.ibm.com>
> ---
> 
> 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;



_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Reply via email to