[PATCH net] net/smc: Fix hung_task when removing SMC-R devices

[PATCH net] net/smc: Fix hung_task when removing SMC-R devices

[Wen Gu]

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(-)

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

Re: [PATCH net] net/smc: Fix hung_task when removing SMC-R devices

[dust.li]

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

Re: [PATCH net] net/smc: Fix hung_task when removing SMC-R devices

[Wen Gu]

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>
>