| Message ID | 20230916060619.3744220-1-rex.zhang@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c0409dd3d151f661e7e57b901a81a02565df163c |
| Headers | show |
| Series | dmaengine: idxd: use spin_lock_irqsave before wait_event_lock_irq | expand |
On Sat, 16 Sep 2023 14:06:19 +0800, Rex Zhang wrote: > In idxd_cmd_exec(), wait_event_lock_irq() explicitly calls > spin_unlock_irq()/spin_lock_irq(). If the interrupt is on before entering > wait_event_lock_irq(), it will become off status after > wait_event_lock_irq() is called. Later, wait_for_completion() may go to > sleep but irq is disabled. The scenario is warned in might_sleep(). > > Fix it by using spin_lock_irqsave() instead of the primitive spin_lock() > to save the irq status before entering wait_event_lock_irq() and using > spin_unlock_irqrestore() instead of the primitive spin_unlock() to restore > the irq status before entering wait_for_completion(). > > [...] Applied, thanks! [1/1] dmaengine: idxd: use spin_lock_irqsave before wait_event_lock_irq commit: c0409dd3d151f661e7e57b901a81a02565df163c Best regards,
diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c index 22d6f4e455b7..8f754f922217 100644 --- a/drivers/dma/idxd/device.c +++ b/drivers/dma/idxd/device.c @@ -477,6 +477,7 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand, union idxd_command_reg cmd; DECLARE_COMPLETION_ONSTACK(done); u32 stat; + unsigned long flags; if (idxd_device_is_halted(idxd)) { dev_warn(&idxd->pdev->dev, "Device is HALTED!\n"); @@ -490,7 +491,7 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand, cmd.operand = operand; cmd.int_req = 1; - spin_lock(&idxd->cmd_lock); + spin_lock_irqsave(&idxd->cmd_lock, flags); wait_event_lock_irq(idxd->cmd_waitq, !test_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags), idxd->cmd_lock); @@ -507,7 +508,7 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand, * After command submitted, release lock and go to sleep until * the command completes via interrupt. */ - spin_unlock(&idxd->cmd_lock); + spin_unlock_irqrestore(&idxd->cmd_lock, flags); wait_for_completion(&done); stat = ioread32(idxd->reg_base + IDXD_CMDSTS_OFFSET); spin_lock(&idxd->cmd_lock);
In idxd_cmd_exec(), wait_event_lock_irq() explicitly calls spin_unlock_irq()/spin_lock_irq(). If the interrupt is on before entering wait_event_lock_irq(), it will become off status after wait_event_lock_irq() is called. Later, wait_for_completion() may go to sleep but irq is disabled. The scenario is warned in might_sleep(). Fix it by using spin_lock_irqsave() instead of the primitive spin_lock() to save the irq status before entering wait_event_lock_irq() and using spin_unlock_irqrestore() instead of the primitive spin_unlock() to restore the irq status before entering wait_for_completion(). Before the change: idxd_cmd_exec() { interrupt is on spin_lock() // interrupt is on wait_event_lock_irq() spin_unlock_irq() // interrupt is enabled ... spin_lock_irq() // interrupt is disabled spin_unlock() // interrupt is still disabled wait_for_completion() // report "BUG: sleeping function // called from invalid context... // in_atomic() irqs_disabled()" } After applying spin_lock_irqsave(): idxd_cmd_exec() { interrupt is on spin_lock_irqsave() // save the on state // interrupt is disabled wait_event_lock_irq() spin_unlock_irq() // interrupt is enabled ... spin_lock_irq() // interrupt is disabled spin_unlock_irqrestore() // interrupt is restored to on wait_for_completion() // No Call trace } Fixes: f9f4082dbc56 ("dmaengine: idxd: remove interrupt disable for cmd_lock") Signed-off-by: Rex Zhang <rex.zhang@intel.com> Signed-off-by: Lijun Pan <lijun.pan@intel.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Reviewed-by: Fenghua Yu <fenghua.yu@intel.com> --- drivers/dma/idxd/device.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)