> Subject: [PATCH V5] drivers: infiniband: hw: Fix deadlock in > irdma_cleanup_cm_core() > > There is a deadlock in irdma_cleanup_cm_core(), which is shown > below: > > (Thread 1) | (Thread 2) > | irdma_schedule_cm_timer() > irdma_cleanup_cm_core() | add_timer() > spin_lock_irqsave() //(1) | (wait a time) > ... | irdma_cm_timer_tick() > del_timer_sync() | spin_lock_irqsave() //(2) > (wait timer to stop) | ... > > We hold cm_core->ht_lock in position (1) of thread 1 and use del_timer_sync() > to wait timer to stop, but timer handler also need cm_core->ht_lock in position (2) > of thread 2. > As a result, irdma_cleanup_cm_core() will block forever. > > This patch removes the check of timer_pending() in irdma_cleanup_cm_core(), > because the del_timer_sync() function will just return directly if there isn't a > pending timer. As a result, the lock is redundant, because there is no resource it > could protect. > > Signed-off-by: Duoming Zhou <duoming@xxxxxxxxxx> > --- > Changes in V5: > - Remove mod_timer() in irdma_schedule_cm_timer and irdma_cm_timer_tick. > > drivers/infiniband/hw/irdma/cm.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/infiniband/hw/irdma/cm.c b/drivers/infiniband/hw/irdma/cm.c > index dedb3b7edd8..4b6b1065f85 100644 > --- a/drivers/infiniband/hw/irdma/cm.c > +++ b/drivers/infiniband/hw/irdma/cm.c > @@ -3251,10 +3251,7 @@ void irdma_cleanup_cm_core(struct > irdma_cm_core *cm_core) > if (!cm_core) > return; > > - spin_lock_irqsave(&cm_core->ht_lock, flags); > - if (timer_pending(&cm_core->tcp_timer)) > - del_timer_sync(&cm_core->tcp_timer); > - spin_unlock_irqrestore(&cm_core->ht_lock, flags); > + del_timer_sync(&cm_core->tcp_timer); > > destroy_workqueue(cm_core->event_wq); > cm_core->dev->ws_reset(&cm_core->iwdev->vsi); > -- I am not sure the deadlock is possible practically since all CM nodes should be culled by the time we get to irdma_cleanup_cm_core. However, timer_pending check and locks are redundant and should be removed. The subject line for patches to our driver are typically prefixed with "RDMA/irdma: " Reviewed-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx>
