15 Jul
2021
15 Jul
'21
10:11 p.m.
On Thu, Jul 15, 2021 at 03:00:55PM -0600, Alex Williamson wrote:
On Wed, 14 Jul 2021 21:20:38 -0300 Jason Gunthorpe jgg@nvidia.com wrote:
+/*
- We need to get memory_lock for each device, but devices can share mmap_lock,
- therefore we need to zap and hold the vma_lock for each device, and only then
- get each memory_lock.
- */
+static int vfio_hot_reset_device_set(struct vfio_pci_device *vdev,
struct vfio_pci_group_info *groups)+{
- struct vfio_device_set *dev_set = vdev->vdev.dev_set;
- struct vfio_pci_device *cur_mem =
list_first_entry(&dev_set->device_list, struct vfio_pci_device,vdev.dev_set_list);We shouldn't be looking at the list outside of the lock, if the first entry got removed we'd break our unwind code.
- struct vfio_pci_device *cur_vma;
- struct vfio_pci_device *cur;
- bool is_mem = true;
- int ret;
- if (pci_dev_driver(pdev) != &vfio_pci_driver) {
vfio_device_put(device);return -EBUSY;
- mutex_lock(&dev_set->lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^
Oh, righto, this is an oopsie!
- list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list)
up_write(&cur->memory_lock);- mutex_unlock(&dev_set->lock);
- return ret;
Isn't the above section actually redundant to below, ie. we could just fall through after the pci_reset_bus()? Thanks,
It could, but I thought it was less confusing this way due to how oddball the below is:
+err_undo:
- list_for_each_entry(cur, &dev_set->device_list, vdev.dev_set_list) {
if (cur == cur_mem)is_mem = false;if (cur == cur_vma)break;if (is_mem)up_write(&cur->memory_lock);elsemutex_unlock(&cur->vma_lock);- }
But either works, do want it switch in v2?
Thanks, Jason