| Message ID | 1642167444-107744-1-git-send-email-guwen@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net/smc: Fix hung_task when removing SMC-R devices | expand |
On Fri, Jan 14, 2022 at 09:37:24PM +0800, Wen Gu wrote: >A hung_task is observed when removing SMC-R devices. Suppose that >a link group has two active links(lnk_A, lnk_B) associated with two >different SMC-R devices(dev_A, dev_B). When dev_A is removed, the >link group will be removed from smc_lgr_list and added into >lgr_linkdown_list. lnk_A will be cleared and smcibdev(A)->lnk_cnt >will reach to zero. However, when dev_B is removed then, the link >group can't be found in smc_lgr_list and lnk_B won't be cleared, >making smcibdev->lnk_cnt never reaches zero, which causes a hung_task. > >This patch fixes this issue by restoring the implementation of >smc_smcr_terminate_all() to what it was before commit 349d43127dac >("net/smc: fix kernel panic caused by race of smc_sock"). The original >implementation also satisfies the intention that make sure QP destroy >earlier than CQ destroy because we will always wait for smcibdev->lnk_cnt >reaches zero, which guarantees QP has been destroyed. Good catch, thank you ! Update the comments of smc_smcr_terminate_all as well ? > >Fixes: 349d43127dac ("net/smc: fix kernel panic caused by race of smc_sock") >Signed-off-by: Wen Gu <guwen@linux.alibaba.com> Reviewed-by: Dust Li <dust.li@linux.alibaba.com> >--- > net/smc/smc_core.c | 13 +------------ > 1 file changed, 1 insertion(+), 12 deletions(-) > >diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c >index b19c0aa..1124594 100644 >--- a/net/smc/smc_core.c >+++ b/net/smc/smc_core.c >@@ -1533,7 +1533,6 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) > { > struct smc_link_group *lgr, *lg; > LIST_HEAD(lgr_free_list); >- LIST_HEAD(lgr_linkdown_list); > int i; > > spin_lock_bh(&smc_lgr_list.lock); >@@ -1545,7 +1544,7 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) > list_for_each_entry_safe(lgr, lg, &smc_lgr_list.list, list) { > for (i = 0; i < SMC_LINKS_PER_LGR_MAX; i++) { > if (lgr->lnk[i].smcibdev == smcibdev) >- list_move_tail(&lgr->list, &lgr_linkdown_list); >+ smcr_link_down_cond_sched(&lgr->lnk[i]); > } > } > } >@@ -1557,16 +1556,6 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) > __smc_lgr_terminate(lgr, false); > } > >- list_for_each_entry_safe(lgr, lg, &lgr_linkdown_list, list) { >- for (i = 0; i < SMC_LINKS_PER_LGR_MAX; i++) { >- if (lgr->lnk[i].smcibdev == smcibdev) { >- mutex_lock(&lgr->llc_conf_mutex); >- smcr_link_down_cond(&lgr->lnk[i]); >- mutex_unlock(&lgr->llc_conf_mutex); >- } >- } >- } >- > if (smcibdev) { > if (atomic_read(&smcibdev->lnk_cnt)) > wait_event(smcibdev->lnks_deleted, >-- >1.8.3.1
On 2022/1/15 6:29 pm, dust.li wrote: > On Fri, Jan 14, 2022 at 09:37:24PM +0800, Wen Gu wrote: >> A hung_task is observed when removing SMC-R devices. > > Good catch, thank you ! > > Update the comments of smc_smcr_terminate_all as well ? > OK, will do. Thank you! Thanks, Wen Gu >> >> Fixes: 349d43127dac ("net/smc: fix kernel panic caused by race of smc_sock") >> Signed-off-by: Wen Gu <guwen@linux.alibaba.com> > > Reviewed-by: Dust Li <dust.li@linux.alibaba.com> >
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c index b19c0aa..1124594 100644 --- a/net/smc/smc_core.c +++ b/net/smc/smc_core.c @@ -1533,7 +1533,6 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) { struct smc_link_group *lgr, *lg; LIST_HEAD(lgr_free_list); - LIST_HEAD(lgr_linkdown_list); int i; spin_lock_bh(&smc_lgr_list.lock); @@ -1545,7 +1544,7 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) list_for_each_entry_safe(lgr, lg, &smc_lgr_list.list, list) { for (i = 0; i < SMC_LINKS_PER_LGR_MAX; i++) { if (lgr->lnk[i].smcibdev == smcibdev) - list_move_tail(&lgr->list, &lgr_linkdown_list); + smcr_link_down_cond_sched(&lgr->lnk[i]); } } } @@ -1557,16 +1556,6 @@ void smc_smcr_terminate_all(struct smc_ib_device *smcibdev) __smc_lgr_terminate(lgr, false); } - list_for_each_entry_safe(lgr, lg, &lgr_linkdown_list, list) { - for (i = 0; i < SMC_LINKS_PER_LGR_MAX; i++) { - if (lgr->lnk[i].smcibdev == smcibdev) { - mutex_lock(&lgr->llc_conf_mutex); - smcr_link_down_cond(&lgr->lnk[i]); - mutex_unlock(&lgr->llc_conf_mutex); - } - } - } - if (smcibdev) { if (atomic_read(&smcibdev->lnk_cnt)) wait_event(smcibdev->lnks_deleted,
A hung_task is observed when removing SMC-R devices. Suppose that a link group has two active links(lnk_A, lnk_B) associated with two different SMC-R devices(dev_A, dev_B). When dev_A is removed, the link group will be removed from smc_lgr_list and added into lgr_linkdown_list. lnk_A will be cleared and smcibdev(A)->lnk_cnt will reach to zero. However, when dev_B is removed then, the link group can't be found in smc_lgr_list and lnk_B won't be cleared, making smcibdev->lnk_cnt never reaches zero, which causes a hung_task. This patch fixes this issue by restoring the implementation of smc_smcr_terminate_all() to what it was before commit 349d43127dac ("net/smc: fix kernel panic caused by race of smc_sock"). The original implementation also satisfies the intention that make sure QP destroy earlier than CQ destroy because we will always wait for smcibdev->lnk_cnt reaches zero, which guarantees QP has been destroyed. Fixes: 349d43127dac ("net/smc: fix kernel panic caused by race of smc_sock") Signed-off-by: Wen Gu <guwen@linux.alibaba.com> --- net/smc/smc_core.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)