Skip to contentSkip to conversation

nvme-core: Use nvme_wq to queue async events and fw activation

PATCH New Sagi Grimberg 3 participants 8 messages Sep 20, 2017 NVMe

lore
Share & tools
lore

Sagi Grimberg

author maintainer
#0 lore raw

PATCH 1/2 nvme-core: Use nvme_wq to queue async events and fw activation

async_event_work might race as it is executed from two different workqueues at the moment.

Signed-off-by Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +2 -2
index 5589f67d2cd8..bb2aad078637 100644
@@ -2676,7 +2676,7 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
 	case NVME_SC_ABORT_REQ:
 		++ctrl->event_limit;
 		if (ctrl->state == NVME_CTRL_LIVE)
-			schedule_work(&ctrl->async_event_work);
+			queue_work(nvme_wq, &ctrl->async_event_work);
 		break;
 	default:
 		break;

@@ -2691,7 +2691,7 @@ void nvme_complete_async_event(struct nvme_ctrl *ctrl, __le16 status,
 		nvme_queue_scan(ctrl);
 		break;
 	case NVME_AER_NOTICE_FW_ACT_STARTING:
-		schedule_work(&ctrl->fw_act_work);
+		queue_work(nvme_wq, &ctrl->fw_act_work);
 		break;
 	default:
 		dev_warn(ctrl->device, "async event result %08x\n", result);
Signature · 1 line
2.7.4
author maintainer
#1 lore raw

PATCH 2/2 nvme-rdma: give up reconnect if state change fails

If we failed to transition to state LIVE after a successful reconnect, then controller deletion already started. In this case there is no point moving forward with reconnect.

Signed-off-by Sagi Grimberg <sagi at grimberg.me>
---
 drivers/nvme/host/rdma.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/nvme/host/rdma.c +5 -1
index 58983000964b..4460ec3a2c0f 100644
@@ -942,7 +942,11 @@ static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work)
 	}
 
 	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
-	WARN_ON_ONCE(!changed);
+	if (!changed) {
+		/* We are probably in DELETING state */
+		return;
+	}
+
 	ctrl->ctrl.nr_reconnects = 0;
 
 	nvme_start_ctrl(&ctrl->ctrl);
Signature · 1 line
2.7.4
#2 lore raw

PATCH 2/2 nvme-rdma: give up reconnect if state change fails

drivers/nvme/host/rdma.c static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work) · line 946

Quoted 4 lines · Sagi Grimberg
drivers/nvme/host/rdma.c +2 -1
 	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
-	WARN_ON_ONCE(!changed);
+	if (!changed) {
+		/* We are probably in DELETING state */

Shouldn't we check if we're really in DELETING instead of just speculating we are?

Byte,
Johannes

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
author maintainer
#3 lore raw

PATCH 2/2 nvme-rdma: give up reconnect if state change fails

Quoted 8 lines

drivers/nvme/host/rdma.c static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work) · line 946

Quoted 5 lines
  	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
drivers/nvme/host/rdma.c +2 -1
-	WARN_ON_ONCE(!changed);
+	if (!changed) {
+		/* We are probably in DELETING state */

Shouldn't we check if we're really in DELETING instead of just speculating we are?

I can add a WARN_ONCE.

#4 lore raw

PATCH 2/2 nvme-rdma: give up reconnect if state change fails

Quoted 10 lines · Sagi Grimberg
Quoted 8 lines

drivers/nvme/host/rdma.c static void nvme_rdma_reconnect_ctrl_work(struct work_struct *work) · line 946

Quoted 5 lines
  	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
drivers/nvme/host/rdma.c +2 -1
-	WARN_ON_ONCE(!changed);
+	if (!changed) {
+		/* We are probably in DELETING state */

Shouldn't we check if we're really in DELETING instead of just speculating we are?

I can add a WARN_ONCE.

Works for me.

With that change,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
#6 lore raw

PATCH 1/2 nvme-core: Use nvme_wq to queue async events and fw activation

In reply to Sagi Grimberg

Looks good,
Reviewed-by: Johannes Thumshirn <jthumshirn at suse.de>

Signature · 6 lines
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850