diff mbox series

[for-rc] RDMA/hns: Fix the double unlock problem of poll_sem

Message ID 1627887374-20019-1-git-send-email-liangwenpeng@huawei.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series [for-rc] RDMA/hns: Fix the double unlock problem of poll_sem | expand

Commit Message

Wenpeng Liang Aug. 2, 2021, 6:56 a.m. UTC
From: Yangyang Li <liyangyang20@huawei.com>

If hns_roce_cmd_use_events() fails then it means that the poll_sem is not
obtained, but the poll_sem is released in hns_roce_cmd_use_polling(), this
will cause an unlock problem.

This is the static checker warning:
	drivers/infiniband/hw/hns/hns_roce_main.c:926 hns_roce_init()
	error: double unlocked '&hr_dev->cmd.poll_sem' (orig line 879)

Event mode and polling mode are mutually exclusive and resources are
separated, so there is no need to process polling mode resources in
event mode.

The initial mode of cmd is polling mode, so even if cmd fails to switch to
event mode, it is not necessary to switch to polling mode.

Fixes: a389d016c030 ("RDMA/hns: Enable all CMDQ context")
Fixes: 3d50503b3b33 ("RDMA/hns: Optimize cmd init and mode selection for hip08")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Yangyang Li <liyangyang20@huawei.com>
Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_cmd.c  | 7 +++----
 drivers/infiniband/hw/hns/hns_roce_main.c | 4 +---
 2 files changed, 4 insertions(+), 7 deletions(-)

--
2.8.1

Comments

Leon Romanovsky Aug. 2, 2021, 10:46 a.m. UTC | #1
On Mon, Aug 02, 2021 at 02:56:14PM +0800, Wenpeng Liang wrote:
> From: Yangyang Li <liyangyang20@huawei.com>
> 
> If hns_roce_cmd_use_events() fails then it means that the poll_sem is not
> obtained, but the poll_sem is released in hns_roce_cmd_use_polling(), this
> will cause an unlock problem.
> 
> This is the static checker warning:
> 	drivers/infiniband/hw/hns/hns_roce_main.c:926 hns_roce_init()
> 	error: double unlocked '&hr_dev->cmd.poll_sem' (orig line 879)
> 
> Event mode and polling mode are mutually exclusive and resources are
> separated, so there is no need to process polling mode resources in
> event mode.
> 
> The initial mode of cmd is polling mode, so even if cmd fails to switch to
> event mode, it is not necessary to switch to polling mode.
> 
> Fixes: a389d016c030 ("RDMA/hns: Enable all CMDQ context")
> Fixes: 3d50503b3b33 ("RDMA/hns: Optimize cmd init and mode selection for hip08")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Yangyang Li <liyangyang20@huawei.com>
> Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_cmd.c  | 7 +++----
>  drivers/infiniband/hw/hns/hns_roce_main.c | 4 +---
>  2 files changed, 4 insertions(+), 7 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Jason Gunthorpe Aug. 3, 2021, 4:43 p.m. UTC | #2
On Mon, Aug 02, 2021 at 02:56:14PM +0800, Wenpeng Liang wrote:
> From: Yangyang Li <liyangyang20@huawei.com>
> 
> If hns_roce_cmd_use_events() fails then it means that the poll_sem is not
> obtained, but the poll_sem is released in hns_roce_cmd_use_polling(), this
> will cause an unlock problem.
> 
> This is the static checker warning:
> 	drivers/infiniband/hw/hns/hns_roce_main.c:926 hns_roce_init()
> 	error: double unlocked '&hr_dev->cmd.poll_sem' (orig line 879)
> 
> Event mode and polling mode are mutually exclusive and resources are
> separated, so there is no need to process polling mode resources in
> event mode.
> 
> The initial mode of cmd is polling mode, so even if cmd fails to switch to
> event mode, it is not necessary to switch to polling mode.
> 
> Fixes: a389d016c030 ("RDMA/hns: Enable all CMDQ context")
> Fixes: 3d50503b3b33 ("RDMA/hns: Optimize cmd init and mode selection for hip08")
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Yangyang Li <liyangyang20@huawei.com>
> Signed-off-by: Wenpeng Liang <liangwenpeng@huawei.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  drivers/infiniband/hw/hns/hns_roce_cmd.c  | 7 +++----
>  drivers/infiniband/hw/hns/hns_roce_main.c | 4 +---
>  2 files changed, 4 insertions(+), 7 deletions(-)

Applied to for-rc, thanks

Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_cmd.c b/drivers/infiniband/hw/hns/hns_roce_cmd.c
index 8f68cc3..84f3f2b 100644
--- a/drivers/infiniband/hw/hns/hns_roce_cmd.c
+++ b/drivers/infiniband/hw/hns/hns_roce_cmd.c
@@ -213,8 +213,10 @@  int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)

 	hr_cmd->context =
 		kcalloc(hr_cmd->max_cmds, sizeof(*hr_cmd->context), GFP_KERNEL);
-	if (!hr_cmd->context)
+	if (!hr_cmd->context) {
+		hr_dev->cmd_mod = 0;
 		return -ENOMEM;
+	}

 	for (i = 0; i < hr_cmd->max_cmds; ++i) {
 		hr_cmd->context[i].token = i;
@@ -228,7 +230,6 @@  int hns_roce_cmd_use_events(struct hns_roce_dev *hr_dev)
 	spin_lock_init(&hr_cmd->context_lock);

 	hr_cmd->use_events = 1;
-	down(&hr_cmd->poll_sem);

 	return 0;
 }
@@ -239,8 +240,6 @@  void hns_roce_cmd_use_polling(struct hns_roce_dev *hr_dev)

 	kfree(hr_cmd->context);
 	hr_cmd->use_events = 0;
-
-	up(&hr_cmd->poll_sem);
 }

 struct hns_roce_cmd_mailbox *
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index 078a971..cc6eab1 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -873,11 +873,9 @@  int hns_roce_init(struct hns_roce_dev *hr_dev)

 	if (hr_dev->cmd_mod) {
 		ret = hns_roce_cmd_use_events(hr_dev);
-		if (ret) {
+		if (ret)
 			dev_warn(dev,
 				 "Cmd event  mode failed, set back to poll!\n");
-			hns_roce_cmd_use_polling(hr_dev);
-		}
 	}

 	ret = hns_roce_init_hem(hr_dev);