Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

dmaengine: idxd: Convert spinlock to mutex to lock evl workqueue

drain_workqueue() cannot be called safely in a spinlocked context due to
possible task rescheduling. In the multi-task scenario, calling
queue_work() while drain_workqueue() will lead to a Call Trace as
pushing a work on a draining workqueue is not permitted in spinlocked
context.
Call Trace:
<TASK>
? __warn+0x7d/0x140
? __queue_work+0x2b2/0x440
? report_bug+0x1f8/0x200
? handle_bug+0x3c/0x70
? exc_invalid_op+0x18/0x70
? asm_exc_invalid_op+0x1a/0x20
? __queue_work+0x2b2/0x440
queue_work_on+0x28/0x30
idxd_misc_thread+0x303/0x5a0 [idxd]
? __schedule+0x369/0xb40
? __pfx_irq_thread_fn+0x10/0x10
? irq_thread+0xbc/0x1b0
irq_thread_fn+0x21/0x70
irq_thread+0x102/0x1b0
? preempt_count_add+0x74/0xa0
? __pfx_irq_thread_dtor+0x10/0x10
? __pfx_irq_thread+0x10/0x10
kthread+0x103/0x140
? __pfx_kthread+0x10/0x10
ret_from_fork+0x31/0x50
? __pfx_kthread+0x10/0x10
ret_from_fork_asm+0x1b/0x30
</TASK>

The current implementation uses a spinlock to protect event log workqueue
and will lead to the Call Trace due to potential task rescheduling.

To address the locking issue, convert the spinlock to mutex, allowing
the drain_workqueue() to be called in a safe mutex-locked context.

This change ensures proper synchronization when accessing the event log
workqueue, preventing potential Call Trace and improving the overall
robustness of the code.

Fixes: c40bd7d9737b ("dmaengine: idxd: process user page faults for completion record")
Signed-off-by: Rex Zhang <rex.zhang@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Lijun Pan <lijun.pan@intel.com>
Link: https://lore.kernel.org/r/20240404223949.2885604-1-fenghua.yu@intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

authored by

Rex Zhang and committed by
Vinod Koul
d5638de8 9140ce47

+12 -13
+2 -3
drivers/dma/idxd/cdev.c
··· 342 342 if (!evl) 343 343 return; 344 344 345 - spin_lock(&evl->lock); 345 + mutex_lock(&evl->lock); 346 346 status.bits = ioread64(idxd->reg_base + IDXD_EVLSTATUS_OFFSET); 347 347 t = status.tail; 348 348 h = status.head; ··· 354 354 set_bit(h, evl->bmap); 355 355 h = (h + 1) % size; 356 356 } 357 - spin_unlock(&evl->lock); 358 - 359 357 drain_workqueue(wq->wq); 358 + mutex_unlock(&evl->lock); 360 359 } 361 360 362 361 static int idxd_cdev_release(struct inode *node, struct file *filep)
+2 -2
drivers/dma/idxd/debugfs.c
··· 66 66 if (!evl || !evl->log) 67 67 return 0; 68 68 69 - spin_lock(&evl->lock); 69 + mutex_lock(&evl->lock); 70 70 71 71 evl_status.bits = ioread64(idxd->reg_base + IDXD_EVLSTATUS_OFFSET); 72 72 t = evl_status.tail; ··· 87 87 dump_event_entry(idxd, s, i, &count, processed); 88 88 } 89 89 90 - spin_unlock(&evl->lock); 90 + mutex_unlock(&evl->lock); 91 91 return 0; 92 92 } 93 93
+4 -4
drivers/dma/idxd/device.c
··· 775 775 goto err_alloc; 776 776 } 777 777 778 - spin_lock(&evl->lock); 778 + mutex_lock(&evl->lock); 779 779 evl->log = addr; 780 780 evl->dma = dma_addr; 781 781 evl->log_size = size; ··· 796 796 gencfg.evl_en = 1; 797 797 iowrite32(gencfg.bits, idxd->reg_base + IDXD_GENCFG_OFFSET); 798 798 799 - spin_unlock(&evl->lock); 799 + mutex_unlock(&evl->lock); 800 800 return 0; 801 801 802 802 err_alloc: ··· 819 819 if (!gencfg.evl_en) 820 820 return; 821 821 822 - spin_lock(&evl->lock); 822 + mutex_lock(&evl->lock); 823 823 gencfg.evl_en = 0; 824 824 iowrite32(gencfg.bits, idxd->reg_base + IDXD_GENCFG_OFFSET); 825 825 ··· 836 836 evl_dma = evl->dma; 837 837 evl->log = NULL; 838 838 evl->size = IDXD_EVL_SIZE_MIN; 839 - spin_unlock(&evl->lock); 839 + mutex_unlock(&evl->lock); 840 840 841 841 dma_free_coherent(dev, evl_log_size, evl_log, evl_dma); 842 842 }
+1 -1
drivers/dma/idxd/idxd.h
··· 293 293 294 294 struct idxd_evl { 295 295 /* Lock to protect event log access. */ 296 - spinlock_t lock; 296 + struct mutex lock; 297 297 void *log; 298 298 dma_addr_t dma; 299 299 /* Total size of event log = number of entries * entry size. */
+1 -1
drivers/dma/idxd/init.c
··· 354 354 if (!evl) 355 355 return -ENOMEM; 356 356 357 - spin_lock_init(&evl->lock); 357 + mutex_init(&evl->lock); 358 358 evl->size = IDXD_EVL_SIZE_MIN; 359 359 360 360 idxd_name = dev_name(idxd_confdev(idxd));
+2 -2
drivers/dma/idxd/irq.c
··· 363 363 evl_status.bits = 0; 364 364 evl_status.int_pending = 1; 365 365 366 - spin_lock(&evl->lock); 366 + mutex_lock(&evl->lock); 367 367 /* Clear interrupt pending bit */ 368 368 iowrite32(evl_status.bits_upper32, 369 369 idxd->reg_base + IDXD_EVLSTATUS_OFFSET + sizeof(u32)); ··· 380 380 381 381 evl_status.head = h; 382 382 iowrite32(evl_status.bits_lower32, idxd->reg_base + IDXD_EVLSTATUS_OFFSET); 383 - spin_unlock(&evl->lock); 383 + mutex_unlock(&evl->lock); 384 384 } 385 385 386 386 irqreturn_t idxd_misc_thread(int vec, void *data)