From: Thadeu Lima de Souza Cascardo <hidden> Date: 2014-03-03 14:33:28
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 <redacted>
---
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?
---
drivers/vfio/pci/vfio_pci.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
From: Alex Williamson <hidden> Date: 2014-03-03 15:09:37
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 <redacted>
---
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?
From: Thadeu Lima de Souza Cascardo <hidden> Date: 2014-03-03 15:29:04
On Mon, Mar 03, 2014 at 08:09:22AM -0700, Alex Williamson wrote:
On Mon, 2014-03-03 at 11:33 -0300, Thadeu Lima de Souza Cascardo wrote:
quoted
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 <redacted>
---
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?
Sorry I missed it. On the rush to report and fix it, I looked only on my
local branch. Should we backport those two patches to long term stable
3.10? I can reproduce the issue there.
Thanks.
Cascardo.
From: Alex Williamson <hidden> Date: 2014-03-03 15:47:20
On Mon, 2014-03-03 at 12:28 -0300, Thadeu Lima de Souza Cascardo wrote:
On Mon, Mar 03, 2014 at 08:09:22AM -0700, Alex Williamson wrote:
quoted
On Mon, 2014-03-03 at 11:33 -0300, Thadeu Lima de Souza Cascardo wrote:
quoted
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 <redacted>
---
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?
Sorry I missed it. On the rush to report and fix it, I looked only on my
local branch. Should we backport those two patches to long term stable
3.10? I can reproduce the issue there.
Sure, if you're trying to exercise this path, it's easy to reproduce.
It's also relatively easy to avoid once you know it's there. It's not
obvious to me that this fix meets the stable patch rules though, it's
bigger than suggested, I'm not sure it really bothers people outside of
QA testing, it does cause a hang, but not a system hang. I'd certainly
suggest any downstream based on 3.10 that cares about vfio to pick it
up, does that make it sufficient for upstream stable? Thanks,
Alex
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html