The driver could possibly sleep while in atomic context resulting in the following call trace while CONFIG_DEBUG_ATOMIC_SLEEP=y is set: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:283 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 2817, name: bash preempt_count: 1, expected: 0 RCU nest depth: 0, expected: 0 Call Trace: <TASK> dump_stack_lvl+0x36/0x50 __might_resched+0x123/0x170 mutex_lock+0x1e/0x50 pds_vfio_put_lm_file+0x1e/0xa0 [pds_vfio_pci] pds_vfio_put_save_file+0x19/0x30 [pds_vfio_pci] pds_vfio_state_mutex_unlock+0x2e/0x80 [pds_vfio_pci] pci_reset_function+0x4b/0x70 reset_store+0x5b/0xa0 kernfs_fop_write_iter+0x137/0x1d0 vfs_write+0x2de/0x410 ksys_write+0x5d/0xd0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x6e/0xd8 This can happen if pds_vfio_put_restore_file() and/or pds_vfio_put_save_file() grab the mutex_lock(&lm_file->lock) while the spin_lock(&pds_vfio->reset_lock) is held, which can happen during while calling pds_vfio_state_mutex_unlock(). Fix this by changing the reset_lock to reset_mutex so there are no such conerns. Also, make sure to destroy the reset_mutex in the driver specific VFIO device release function. This also fixes a spinlock bad magic BUG that was caused by not calling spinlock_init() on the reset_lock. Since, the lock is being changed to a mutex, make sure to call mutex_init() on it.
Unified · Split
| @@ -55,10 +55,10 @@ static void pds_vfio_recovery(struct pds_vfio_pci_device *pds_vfio) · 54 unmodified lines | ||
| 55 | 55 | * VFIO_DEVICE_STATE_RUNNING. |
| 56 | 56 | */ |
| 57 | 57 | if (deferred_reset_needed) { |
| 58 | − spin_lock(&pds_vfio->reset_lock); | |
| 58 | + mutex_lock(&pds_vfio->reset_mutex); | |
| 59 | 59 | pds_vfio->deferred_reset = true; |
| 60 | 60 | pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_ERROR; |
| 61 | − spin_unlock(&pds_vfio->reset_lock); | |
| 61 | + mutex_unlock(&pds_vfio->reset_mutex); | |
| 62 | 62 | } |
| 63 | 63 | } |
| 64 | 64 | |
| @@ -29,7 +29,7 @@ struct pds_vfio_pci_device *pds_vfio_pci_drvdata(struct pci_dev *pdev) · 28 unmodified lines | ||
| 29 | 29 | void pds_vfio_state_mutex_unlock(struct pds_vfio_pci_device *pds_vfio) |
| 30 | 30 | { |
| 31 | 31 | again: |
| 32 | − spin_lock(&pds_vfio->reset_lock); | |
| 32 | + mutex_lock(&pds_vfio->reset_mutex); | |
| 33 | 33 | if (pds_vfio->deferred_reset) { |
| 34 | 34 | pds_vfio->deferred_reset = false; |
| 35 | 35 | if (pds_vfio->state == VFIO_DEVICE_STATE_ERROR) { |
| @@ -39,23 +39,23 @@ again: · 3 unmodified lines | ||
| 39 | 39 | } |
| 40 | 40 | pds_vfio->state = pds_vfio->deferred_reset_state; |
| 41 | 41 | pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_RUNNING; |
| 42 | − spin_unlock(&pds_vfio->reset_lock); | |
| 42 | + mutex_unlock(&pds_vfio->reset_mutex); | |
| 43 | 43 | goto again; |
| 44 | 44 | } |
| 45 | 45 | mutex_unlock(&pds_vfio->state_mutex); |
| 46 | − spin_unlock(&pds_vfio->reset_lock); | |
| 46 | + mutex_unlock(&pds_vfio->reset_mutex); | |
| 47 | 47 | } |
| 48 | 48 | |
| 49 | 49 | void pds_vfio_reset(struct pds_vfio_pci_device *pds_vfio) |
| 50 | 50 | { |
| 51 | − spin_lock(&pds_vfio->reset_lock); | |
| 51 | + mutex_lock(&pds_vfio->reset_mutex); | |
| 52 | 52 | pds_vfio->deferred_reset = true; |
| 53 | 53 | pds_vfio->deferred_reset_state = VFIO_DEVICE_STATE_RUNNING; |
| 54 | 54 | if (!mutex_trylock(&pds_vfio->state_mutex)) { |
| 55 | − spin_unlock(&pds_vfio->reset_lock); | |
| 55 | + mutex_unlock(&pds_vfio->reset_mutex); | |
| 56 | 56 | return; |
| 57 | 57 | } |
| 58 | − spin_unlock(&pds_vfio->reset_lock); | |
| 58 | + mutex_unlock(&pds_vfio->reset_mutex); | |
| 59 | 59 | pds_vfio_state_mutex_unlock(pds_vfio); |
| 60 | 60 | } |
| 61 | 61 | |
| @@ -156,6 +156,7 @@ static int pds_vfio_init_device(struct vfio_device *vdev) · 94 unmodified lines | ||
| 156 | 156 | pds_vfio->vf_id = vf_id; |
| 157 | 157 | |
| 158 | 158 | mutex_init(&pds_vfio->state_mutex); |
| 159 | + mutex_init(&pds_vfio->reset_mutex); | |
| 159 | 160 | |
| 160 | 161 | vdev->migration_flags = VFIO_MIGRATION_STOP_COPY | VFIO_MIGRATION_P2P; |
| 161 | 162 | vdev->mig_ops = &pds_vfio_lm_ops; |
| @@ -177,6 +178,7 @@ static void pds_vfio_release_device(struct vfio_device *vdev) · 15 unmodified lines | ||
| 177 | 178 | vfio_coredev.vdev); |
| 178 | 179 | |
| 179 | 180 | mutex_destroy(&pds_vfio->state_mutex); |
| 181 | + mutex_destroy(&pds_vfio->reset_mutex); | |
| 180 | 182 | vfio_pci_core_release_dev(vdev); |
| 181 | 183 | } |
| 182 | 184 | |
| @@ -18,7 +18,7 @@ struct pds_vfio_pci_device { · 17 unmodified lines | ||
| 18 | 18 | struct pds_vfio_dirty dirty; |
| 19 | 19 | struct mutex state_mutex; /* protect migration state */ |
| 20 | 20 | enum vfio_device_mig_state state; |
| 21 | − spinlock_t reset_lock; /* protect reset_done flow */ | |
| 21 | + struct mutex reset_mutex; /* protect reset_done flow */ | |
| 22 | 22 | u8 deferred_reset; |
| 23 | 23 | enum vfio_device_mig_state deferred_reset_state; |
| 24 | 24 | struct notifier_block nb; |