Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1654311 > unrolled thread

[PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc

Back to article view | Back to linux.kernel

Contents

  [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc Jia-Ju Bai <baijiaju1990@163.com> - 2017-05-31 16:30 +0200
    RE: [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc "Mintz, Yuval" <Yuval.Mintz@cavium.com> - 2017-05-31 20:10 +0200

#1654311 — [PATCH] qed: Fix a sleep-in-interrupt bug in qed_int_sp_dpc

The driver may sleep in interrupt handling, and the function call path is:
qed_int_sp_dpc (tasklet_init indicates it handles interrupt)
  qed_int_attentions
    qed_mcp_handle_events
      qed_mcp_handle_link_change
        qed_link_update
          qed_fill_link
            qed_mcp_get_media_type
              qed_ptt_acquire
                usleep_range --> may sleep

To fix it, the "usleep_range" is replaced with "udelay".

Signed-off-by: Jia-Ju Bai <baijiaju1990@163.com>
---
 drivers/net/ethernet/qlogic/qed/qed_hw.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_hw.c b/drivers/net/ethernet/qlogic/qed/qed_hw.c
index a05feb3..3250cc4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hw.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_hw.c
@@ -131,7 +131,7 @@ struct qed_ptt *qed_ptt_acquire(struct qed_hwfn *p_hwfn)
 		}
 
 		spin_unlock_bh(&p_hwfn->p_ptt_pool->lock);
-		usleep_range(1000, 2000);
+		udelay(1500);
 	}
 
 	DP_NOTICE(p_hwfn, "PTT acquire timeout - failed to allocate PTT\n");
-- 
1.7.9.5

[toc] | [next] | [standalone]

#1654516

> The driver may sleep in interrupt handling, and the function call path is:
> qed_int_sp_dpc (tasklet_init indicates it handles interrupt)
>   qed_int_attentions
>     qed_mcp_handle_events
>       qed_mcp_handle_link_change
>         qed_link_update
>           qed_fill_link
>             qed_mcp_get_media_type
>               qed_ptt_acquire
>                 usleep_range --> may sleep
> 
> To fix it, the "usleep_range" is replaced with "udelay".

Thanks for pointing this out, but I believe the fix isn't the right one.

The better option would be to propagate the PTT from the outside
instead of acquiring a new one - in the call stack you've shared
qed_mcp_link_change() held a PTT entry and we could have propagated
it all the way through.

I'll try sending a fix for this in the next couple of days.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web