* [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
@ 2023-09-26 10:11 Chengfeng Ye
2023-10-23 16:01 ` Jason Gunthorpe
2023-10-25 8:13 ` Leon Romanovsky
0 siblings, 2 replies; 4+ messages in thread
From: Chengfeng Ye @ 2023-09-26 10:11 UTC (permalink / raw)
To: dennis.dalessandro, jgg, leon, dean.luick
Cc: linux-rdma, linux-kernel, Chengfeng Ye
handle_receive_interrupt_napi_sp() running inside interrupt handler
could introduce inverse lock ordering between &dd->irq_src_lock
and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
[CPU0] | [CPU1]
hfi1_ipoib_dev_open() |
--> hfi1_netdev_enable_queues() |
--> enable_queues(rx) |
--> hfi1_rcvctrl() |
--> set_intr_bits() |
--> read_mod_write() |
--> spin_lock(&dd->irq_src_lock) |
| hfi1_poll()
| --> poll_next()
| --> spin_lock_irq(&dd->uctxt_lock)
|
| --> hfi1_rcvctrl()
| --> set_intr_bits()
| --> read_mod_write()
| --> spin_lock(&dd->irq_src_lock)
<interrupt> |
--> handle_receive_interrupt_napi_sp() |
--> set_all_fastpath() |
--> hfi1_rcd_get_by_index() |
--> spin_lock_irqsave(&dd->uctxt_lock) |
This flaw was found by an experimental static analysis tool I am
developing for irq-related deadlock.
To prevent the potential deadlock, the patch use spin_lock_irqsave()
on &dd->irq_src_lock inside read_mod_write() to prevent the possible
deadlock scenario.
Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
---
drivers/infiniband/hw/hfi1/chip.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index 0814291a0412..9b542f7c6c11 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -13185,15 +13185,16 @@ static void read_mod_write(struct hfi1_devdata *dd, u16 src, u64 bits,
{
u64 reg;
u16 idx = src / BITS_PER_REGISTER;
+ unsigned long flags;
- spin_lock(&dd->irq_src_lock);
+ spin_lock_irqsave(&dd->irq_src_lock, flags);
reg = read_csr(dd, CCE_INT_MASK + (8 * idx));
if (set)
reg |= bits;
else
reg &= ~bits;
write_csr(dd, CCE_INT_MASK + (8 * idx), reg);
- spin_unlock(&dd->irq_src_lock);
+ spin_unlock_irqrestore(&dd->irq_src_lock, flags);
}
/**
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
2023-09-26 10:11 [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock Chengfeng Ye
@ 2023-10-23 16:01 ` Jason Gunthorpe
2023-10-24 15:26 ` Dennis Dalessandro
2023-10-25 8:13 ` Leon Romanovsky
1 sibling, 1 reply; 4+ messages in thread
From: Jason Gunthorpe @ 2023-10-23 16:01 UTC (permalink / raw)
To: Chengfeng Ye, dennis.dalessandro
Cc: leon, dean.luick, linux-rdma, linux-kernel
On Tue, Sep 26, 2023 at 10:11:16AM +0000, Chengfeng Ye wrote:
> handle_receive_interrupt_napi_sp() running inside interrupt handler
> could introduce inverse lock ordering between &dd->irq_src_lock
> and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
>
> [CPU0] | [CPU1]
> hfi1_ipoib_dev_open() |
> --> hfi1_netdev_enable_queues() |
> --> enable_queues(rx) |
> --> hfi1_rcvctrl() |
> --> set_intr_bits() |
> --> read_mod_write() |
> --> spin_lock(&dd->irq_src_lock) |
> | hfi1_poll()
> | --> poll_next()
> | --> spin_lock_irq(&dd->uctxt_lock)
> |
> | --> hfi1_rcvctrl()
> | --> set_intr_bits()
> | --> read_mod_write()
> | --> spin_lock(&dd->irq_src_lock)
> <interrupt> |
> --> handle_receive_interrupt_napi_sp() |
> --> set_all_fastpath() |
> --> hfi1_rcd_get_by_index() |
> --> spin_lock_irqsave(&dd->uctxt_lock) |
>
> This flaw was found by an experimental static analysis tool I am
> developing for irq-related deadlock.
>
> To prevent the potential deadlock, the patch use spin_lock_irqsave()
> on &dd->irq_src_lock inside read_mod_write() to prevent the possible
> deadlock scenario.
>
> Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
> ---
> drivers/infiniband/hw/hfi1/chip.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Dennis? This needs your ack/nack
Thanks,
Jason
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
2023-10-23 16:01 ` Jason Gunthorpe
@ 2023-10-24 15:26 ` Dennis Dalessandro
0 siblings, 0 replies; 4+ messages in thread
From: Dennis Dalessandro @ 2023-10-24 15:26 UTC (permalink / raw)
To: Jason Gunthorpe, Chengfeng Ye; +Cc: leon, dean.luick, linux-rdma, linux-kernel
On 10/23/23 12:01 PM, Jason Gunthorpe wrote:
> On Tue, Sep 26, 2023 at 10:11:16AM +0000, Chengfeng Ye wrote:
>> handle_receive_interrupt_napi_sp() running inside interrupt handler
>> could introduce inverse lock ordering between &dd->irq_src_lock
>> and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
>>
>> [CPU0] | [CPU1]
>> hfi1_ipoib_dev_open() |
>> --> hfi1_netdev_enable_queues() |
>> --> enable_queues(rx) |
>> --> hfi1_rcvctrl() |
>> --> set_intr_bits() |
>> --> read_mod_write() |
>> --> spin_lock(&dd->irq_src_lock) |
>> | hfi1_poll()
>> | --> poll_next()
>> | --> spin_lock_irq(&dd->uctxt_lock)
>> |
>> | --> hfi1_rcvctrl()
>> | --> set_intr_bits()
>> | --> read_mod_write()
>> | --> spin_lock(&dd->irq_src_lock)
>> <interrupt> |
>> --> handle_receive_interrupt_napi_sp() |
>> --> set_all_fastpath() |
>> --> hfi1_rcd_get_by_index() |
>> --> spin_lock_irqsave(&dd->uctxt_lock) |
>>
>> This flaw was found by an experimental static analysis tool I am
>> developing for irq-related deadlock.
>>
>> To prevent the potential deadlock, the patch use spin_lock_irqsave()
>> on &dd->irq_src_lock inside read_mod_write() to prevent the possible
>> deadlock scenario.
>>
>> Signed-off-by: Chengfeng Ye <dg573847474@gmail.com>
>> ---
>> drivers/infiniband/hw/hfi1/chip.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> Dennis? This needs your ack/nack
Looks like we need to disable the interrupt. Sorry for the delay.
Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
2023-09-26 10:11 [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock Chengfeng Ye
2023-10-23 16:01 ` Jason Gunthorpe
@ 2023-10-25 8:13 ` Leon Romanovsky
1 sibling, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2023-10-25 8:13 UTC (permalink / raw)
To: dennis.dalessandro, jgg, dean.luick, Chengfeng Ye
Cc: linux-rdma, linux-kernel
On Tue, 26 Sep 2023 10:11:16 +0000, Chengfeng Ye wrote:
> handle_receive_interrupt_napi_sp() running inside interrupt handler
> could introduce inverse lock ordering between &dd->irq_src_lock
> and &dd->uctxt_lock, if read_mod_write() is preempted by the isr.
>
> [CPU0] | [CPU1]
> hfi1_ipoib_dev_open() |
> --> hfi1_netdev_enable_queues() |
> --> enable_queues(rx) |
> --> hfi1_rcvctrl() |
> --> set_intr_bits() |
> --> read_mod_write() |
> --> spin_lock(&dd->irq_src_lock) |
> | hfi1_poll()
> | --> poll_next()
> | --> spin_lock_irq(&dd->uctxt_lock)
> |
> | --> hfi1_rcvctrl()
> | --> set_intr_bits()
> | --> read_mod_write()
> | --> spin_lock(&dd->irq_src_lock)
> <interrupt> |
> --> handle_receive_interrupt_napi_sp() |
> --> set_all_fastpath() |
> --> hfi1_rcd_get_by_index() |
> --> spin_lock_irqsave(&dd->uctxt_lock) |
>
> [...]
Applied, thanks!
[1/1] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock
https://git.kernel.org/rdma/rdma/c/2f19c4b8395ccb
Best regards,
--
Leon Romanovsky <leon@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-10-25 8:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-26 10:11 [PATCH] IB/hfi1: Fix potential deadlock on &irq_src_lock and &dd->uctxt_lock Chengfeng Ye
2023-10-23 16:01 ` Jason Gunthorpe
2023-10-24 15:26 ` Dennis Dalessandro
2023-10-25 8:13 ` Leon Romanovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox