[PATCH v2 0/5] Fix host side state machine

[PATCH v2 0/5] Fix host side state machine

[Max Gurtovoy]

Hi all,
this series is rebased above nvme-4.16.
Actually there is a still missing part in this tree (but I tested it on
my own "stable" mixed kernel with this patch):
"nvme-rdma: fix concurrent reset and reconnect" from Sagi.

The first motivation for this series was fixing RDMA initiator that crushes in
case we fail during initial connect and start error recovery during initial
connection establishment.
This patchset also renames NVME_CTRL_RECONNECTING to NVME_CTRL_CONNECTING as
this state doesn't represent only a reconnection flow but also used for
initialization process.

changes from V1:
 - Added FC support

Max Gurtovoy (5):
  nvme: rename NVME_CTRL_RECONNECTING state to NVME_CTRL_CONNECTING
  nvme-rdma: use NVME_CTRL_CONNECTING state to mark init process
  nvme-fc: use NVME_CTRL_CONNECTING state to mark init process
  nvme: delete NVME_CTRL_LIVE --> NVME_CTRL_CONNECTING transition
  nvme: delete NVME_CTRL_NEW --> NVME_CTRL_LIVE transition

 drivers/nvme/host/core.c    | 13 ++++++-------
 drivers/nvme/host/fabrics.h |  9 +++++----
 drivers/nvme/host/fc.c      | 30 +++++++++++++++++-------------
 drivers/nvme/host/nvme.h    |  2 +-
 drivers/nvme/host/pci.c     |  8 ++++----
 drivers/nvme/host/rdma.c    |  7 +++++--
 6 files changed, 38 insertions(+), 31 deletions(-)

-- 
1.8.3.1

[PATCH 1/5] nvme: rename NVME_CTRL_RECONNECTING state to NVME_CTRL_CONNECTING

[Max Gurtovoy]

In pci transport, this state is used to mark the initialization
process. This should be also used in other transports as well.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c    | 10 +++++-----
 drivers/nvme/host/fabrics.h |  9 +++++----
 drivers/nvme/host/fc.c      | 14 +++++++-------
 drivers/nvme/host/nvme.h    |  2 +-
 drivers/nvme/host/pci.c     |  8 ++++----
 drivers/nvme/host/rdma.c    |  4 ++--
 6 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b3af8e9..c1a9706 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -265,7 +265,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 	switch (new_state) {
 	case NVME_CTRL_ADMIN_ONLY:
 		switch (old_state) {
-		case NVME_CTRL_RECONNECTING:
+		case NVME_CTRL_CONNECTING:
 			changed = true;
 			/* FALLTHRU */
 		default:
@@ -276,7 +276,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		switch (old_state) {
 		case NVME_CTRL_NEW:
 		case NVME_CTRL_RESETTING:
-		case NVME_CTRL_RECONNECTING:
+		case NVME_CTRL_CONNECTING:
 			changed = true;
 			/* FALLTHRU */
 		default:
@@ -294,7 +294,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 			break;
 		}
 		break;
-	case NVME_CTRL_RECONNECTING:
+	case NVME_CTRL_CONNECTING:
 		switch (old_state) {
 		case NVME_CTRL_LIVE:
 		case NVME_CTRL_RESETTING:
@@ -309,7 +309,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		case NVME_CTRL_LIVE:
 		case NVME_CTRL_ADMIN_ONLY:
 		case NVME_CTRL_RESETTING:
-		case NVME_CTRL_RECONNECTING:
+		case NVME_CTRL_CONNECTING:
 			changed = true;
 			/* FALLTHRU */
 		default:
@@ -2682,7 +2682,7 @@ static ssize_t nvme_sysfs_show_state(struct device *dev,
 		[NVME_CTRL_LIVE]	= "live",
 		[NVME_CTRL_ADMIN_ONLY]	= "only-admin",
 		[NVME_CTRL_RESETTING]	= "resetting",
-		[NVME_CTRL_RECONNECTING]= "reconnecting",
+		[NVME_CTRL_CONNECTING]	= "connecting",
 		[NVME_CTRL_DELETING]	= "deleting",
 		[NVME_CTRL_DEAD]	= "dead",
 	};
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index 25b19f7..a3145d9 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -171,13 +171,14 @@ static inline blk_status_t nvmf_check_init_req(struct nvme_ctrl *ctrl,
 	    cmd->common.opcode != nvme_fabrics_command ||
 	    cmd->fabrics.fctype != nvme_fabrics_type_connect) {
 		/*
-		 * Reconnecting state means transport disruption, which can take
-		 * a long time and even might fail permanently, fail fast to
-		 * give upper layers a chance to failover.
+		 * Connecting state means transport disruption or initial
+		 * establishment, which can take a long time and even might
+		 * fail permanently, fail fast to give upper layers a chance
+		 * to failover.
 		 * Deleting state means that the ctrl will never accept commands
 		 * again, fail it permanently.
 		 */
-		if (ctrl->state == NVME_CTRL_RECONNECTING ||
+		if (ctrl->state == NVME_CTRL_CONNECTING ||
 		    ctrl->state == NVME_CTRL_DELETING) {
 			nvme_req(rq)->status = NVME_SC_ABORT_REQ;
 			return BLK_STS_IOERR;
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index b76ba46..b679971 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -534,7 +534,7 @@ static void __nvme_fc_delete_hw_queue(struct nvme_fc_ctrl *,
 {
 	switch (ctrl->ctrl.state) {
 	case NVME_CTRL_NEW:
-	case NVME_CTRL_RECONNECTING:
+	case NVME_CTRL_CONNECTING:
 		/*
 		 * As all reconnects were suppressed, schedule a
 		 * connect.
@@ -779,7 +779,7 @@ static void __nvme_fc_delete_hw_queue(struct nvme_fc_ctrl *,
 		}
 		break;
 
-	case NVME_CTRL_RECONNECTING:
+	case NVME_CTRL_CONNECTING:
 		/*
 		 * The association has already been terminated and the
 		 * controller is attempting reconnects.  No need to do anything
@@ -1724,7 +1724,7 @@ enum {
 	if (status &&
 	    (blk_queue_dying(rq->q) ||
 	     ctrl->ctrl.state == NVME_CTRL_NEW ||
-	     ctrl->ctrl.state == NVME_CTRL_RECONNECTING))
+	     ctrl->ctrl.state == NVME_CTRL_CONNECTING))
 		status |= cpu_to_le16(NVME_SC_DNR << 1);
 
 	if (__nvme_fc_fcpop_chk_teardowns(ctrl, op))
@@ -2951,7 +2951,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	unsigned long recon_delay = ctrl->ctrl.opts->reconnect_delay * HZ;
 	bool recon = true;
 
-	if (ctrl->ctrl.state != NVME_CTRL_RECONNECTING)
+	if (ctrl->ctrl.state != NVME_CTRL_CONNECTING)
 		return;
 
 	if (portptr->port_state == FC_OBJSTATE_ONLINE)
@@ -2999,10 +2999,10 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	/* will block will waiting for io to terminate */
 	nvme_fc_delete_association(ctrl);
 
-	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RECONNECTING)) {
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) {
 		dev_err(ctrl->ctrl.device,
 			"NVME-FC{%d}: error_recovery: Couldn't change state "
-			"to RECONNECTING\n", ctrl->cnum);
+			"to CONNECTING\n", ctrl->cnum);
 		return;
 	}
 
@@ -3203,7 +3203,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	 * transport errors (frame drop, LS failure) inherently must kill
 	 * the association. The transport is coded so that any command used
 	 * to create the association (prior to a LIVE state transition
-	 * while NEW or RECONNECTING) will fail if it completes in error or
+	 * while NEW or CONNECTING) will fail if it completes in error or
 	 * times out.
 	 *
 	 * As such: as the connect request was mostly likely due to a
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 8e7fc1b..91529c9 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -123,7 +123,7 @@ enum nvme_ctrl_state {
 	NVME_CTRL_LIVE,
 	NVME_CTRL_ADMIN_ONLY,    /* Only admin queue live */
 	NVME_CTRL_RESETTING,
-	NVME_CTRL_RECONNECTING,
+	NVME_CTRL_CONNECTING,
 	NVME_CTRL_DELETING,
 	NVME_CTRL_DEAD,
 };
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0bc6a9e..8060ee9 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1143,7 +1143,7 @@ static bool nvme_should_reset(struct nvme_dev *dev, u32 csts)
 	/* If there is a reset/reinit ongoing, we shouldn't reset again. */
 	switch (dev->ctrl.state) {
 	case NVME_CTRL_RESETTING:
-	case NVME_CTRL_RECONNECTING:
+	case NVME_CTRL_CONNECTING:
 		return false;
 	default:
 		break;
@@ -2290,12 +2290,12 @@ static void nvme_reset_work(struct work_struct *work)
 		nvme_dev_disable(dev, false);
 
 	/*
-	 * Introduce RECONNECTING state from nvme-fc/rdma transports to mark the
+	 * Introduce CONNECTING state from nvme-fc/rdma transports to mark the
 	 * initializing procedure here.
 	 */
-	if (!nvme_change_ctrl_state(&dev->ctrl, NVME_CTRL_RECONNECTING)) {
+	if (!nvme_change_ctrl_state(&dev->ctrl, NVME_CTRL_CONNECTING)) {
 		dev_warn(dev->ctrl.device,
-			"failed to mark controller RECONNECTING\n");
+			"failed to mark controller CONNECTING\n");
 		goto out;
 	}
 
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 6c2fdfa..219c6a0 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -887,7 +887,7 @@ static void nvme_rdma_free_ctrl(struct nvme_ctrl *nctrl)
 static void nvme_rdma_reconnect_or_remove(struct nvme_rdma_ctrl *ctrl)
 {
 	/* If we are resetting/deleting then do nothing */
-	if (ctrl->ctrl.state != NVME_CTRL_RECONNECTING) {
+	if (ctrl->ctrl.state != NVME_CTRL_CONNECTING) {
 		WARN_ON_ONCE(ctrl->ctrl.state == NVME_CTRL_NEW ||
 			ctrl->ctrl.state == NVME_CTRL_LIVE);
 		return;
@@ -978,7 +978,7 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work)
 
 static void nvme_rdma_error_recovery(struct nvme_rdma_ctrl *ctrl)
 {
-	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RECONNECTING))
+	if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING))
 		return;
 
 	queue_work(nvme_wq, &ctrl->err_work);
-- 
1.8.3.1

[PATCH 2/5] nvme-rdma: use NVME_CTRL_CONNECTING state to mark init process

[Max Gurtovoy]

In order to avoid concurrent error recovery during initialization
process (allowed by the NVME_CTRL_NEW --> NVME_CTRL_RESETTING transition)
we must mark the ctrl as CONNECTING before initial connection
establisment.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c | 1 +
 drivers/nvme/host/rdma.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c1a9706..1d7e304 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -296,6 +296,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		break;
 	case NVME_CTRL_CONNECTING:
 		switch (old_state) {
+		case NVME_CTRL_NEW:
 		case NVME_CTRL_LIVE:
 		case NVME_CTRL_RESETTING:
 			changed = true;
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 219c6a0..d86aee7 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1930,6 +1930,9 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	if (!ctrl->queues)
 		goto out_uninit_ctrl;
 
+	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING);
+	WARN_ON_ONCE(!changed);
+
 	ret = nvme_rdma_configure_admin_queue(ctrl, true);
 	if (ret)
 		goto out_kfree_queues;
-- 
1.8.3.1

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[Max Gurtovoy]

Align with RDMA and PCI transports.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/fc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index b679971..2992896 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2704,7 +2704,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
  * on the link side, recreates the controller association.
  */
 static int
-nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
+nvme_fc_create_association(struct nvme_fc_ctrl *ctrl, bool new)
 {
 	struct nvmf_ctrl_options *opts = ctrl->ctrl.opts;
 	int ret;
@@ -2735,7 +2735,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	if (ret)
 		goto out_delete_hw_queue;
 
-	if (ctrl->ctrl.state != NVME_CTRL_NEW)
+	if (!new)
 		blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
 
 	ret = nvmf_connect_admin_queue(&ctrl->ctrl);
@@ -2801,7 +2801,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	 */
 
 	if (ctrl->ctrl.queue_count > 1) {
-		if (ctrl->ctrl.state == NVME_CTRL_NEW)
+		if (new)
 			ret = nvme_fc_create_io_queues(ctrl);
 		else
 			ret = nvme_fc_reinit_io_queues(ctrl);
@@ -3007,7 +3007,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	}
 
 	if (ctrl->rport->remoteport.port_state == FC_OBJSTATE_ONLINE)
-		ret = nvme_fc_create_association(ctrl);
+		ret = nvme_fc_create_association(ctrl, false);
 	else
 		ret = -ENOTCONN;
 
@@ -3042,7 +3042,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 			container_of(to_delayed_work(work),
 				struct nvme_fc_ctrl, connect_work);
 
-	ret = nvme_fc_create_association(ctrl);
+	ret = nvme_fc_create_association(ctrl, false);
 	if (ret)
 		nvme_fc_reconnect_or_delete(ctrl, ret);
 	else
@@ -3096,6 +3096,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	struct nvme_fc_ctrl *ctrl;
 	unsigned long flags;
 	int ret, idx, retry;
+	bool changed;
 
 	if (!(rport->remoteport.port_role &
 	    (FC_PORT_ROLE_NVME_DISCOVERY | FC_PORT_ROLE_NVME_TARGET))) {
@@ -3212,8 +3213,11 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
 	 * request fails, retry the initial connection creation up to
 	 * three times before giving up and declaring failure.
 	 */
+	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING);
+	WARN_ON_ONCE(!changed);
+
 	for (retry = 0; retry < 3; retry++) {
-		ret = nvme_fc_create_association(ctrl);
+		ret = nvme_fc_create_association(ctrl, true);
 		if (!ret)
 			break;
 	}
-- 
1.8.3.1

[PATCH 4/5] nvme: delete NVME_CTRL_LIVE --> NVME_CTRL_CONNECTING transition

[Max Gurtovoy]

There is no logical reason to move from live state to connecting
state. In case of initial connection establishment, the transition
should be NVME_CTRL_NEW --> NVME_CTRL_CONNECTING --> NVME_CTRL_LIVE.
In case of error recovery or reset, the transition should be
NVME_CTRL_LIVE --> NVME_CTRL_RESETTING --> NVME_CTRL_CONNECTING -->
NVME_CTRL_LIVE.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 1d7e304..c2a3700 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -297,7 +297,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 	case NVME_CTRL_CONNECTING:
 		switch (old_state) {
 		case NVME_CTRL_NEW:
-		case NVME_CTRL_LIVE:
 		case NVME_CTRL_RESETTING:
 			changed = true;
 			/* FALLTHRU */
-- 
1.8.3.1

[PATCH 5/5] nvme: delete NVME_CTRL_NEW --> NVME_CTRL_LIVE transition

[Max Gurtovoy]

In order to get for "new" to "live" state we must "connect" (or
"reset" and "connect" in case of pci transport), so disable the
option for this transition.

Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
---
 drivers/nvme/host/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c2a3700..b8ddb7f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -274,7 +274,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
 		break;
 	case NVME_CTRL_LIVE:
 		switch (old_state) {
-		case NVME_CTRL_NEW:
 		case NVME_CTRL_RESETTING:
 		case NVME_CTRL_CONNECTING:
 			changed = true;
-- 
1.8.3.1

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[Sagi Grimberg]

> Align with RDMA and PCI transports.
> 
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>

Was this tested?

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[Max Gurtovoy]

On 1/29/2018 9:42 PM, Sagi Grimberg wrote:
> 
>> Align with RDMA and PCI transports.
>>
>> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> 
> Was this tested?

No, I don't have the FC HW to test it.
I hope someone from the FC guys can help us with this one.
This patch was sent as proposal in V1..

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[Sagi Grimberg]

>> Was this tested?
> 
> No, I don't have the FC HW to test it.
> I hope someone from the FC guys can help us with this one.
> This patch was sent as proposal in V1..

Then you need to mention that in the commit message.

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[Max Gurtovoy]

On 1/30/2018 9:25 AM, Sagi Grimberg wrote:
> 
>>> Was this tested?
>>
>> No, I don't have the FC HW to test it.
>> I hope someone from the FC guys can help us with this one.
>> This patch was sent as proposal in V1..
> 
> Then you need to mention that in the commit message.

Sorry :)
I've mentioned it in my proposal (V1), forgot to add it in V2.

[PATCH 3/5] nvme-fc: use NVME_CTRL_CONNECTING state to mark init process

[James Smart]

On 1/28/2018 5:58 AM, Max Gurtovoy wrote:
> Align with RDMA and PCI transports.
>
> Signed-off-by: Max Gurtovoy <maxg at mellanox.com>
> ---
>   drivers/nvme/host/fc.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
> index b679971..2992896 100644
> --- a/drivers/nvme/host/fc.c
> +++ b/drivers/nvme/host/fc.c

The other patches in this set look ok, but with one caveat (5th patch) 
I'll get to shortly.? I agree with the desired statemachine look as 
well, with more comments on this below.

This fc patch though - I disagree with. There was a reason I avoided the 
"new" arguments that were in rdma and instead keyed off the the NEW 
controller state. We were coded differently and adding them in raises 
some conditions where they will not be set properly relative to how far 
the controller is initialized.

In FC, when we had failures in the initial connect - the controller 
state stayed in the NEW state for 3 retries of the initial controller 
create. The 3 retries were immediate and driven from the create_ctrl 
call.? It does bother me that fc did not go into the reconnect 
delay/retry loop like we would once we had passed initial connect, but 
if we do that, the controller state machine means we lose the NEW 
condition that tracked how much the controller was initialized and would 
miss some initial setup for the io queues as well as action on the 
queues when tearing down the association that got partially built before 
failing. It's actually the same situations you create with this patch - 
the "new" arguments aren't set to a correct value relative to how much 
the controller has initialized.

There's another effort going on as well.? Hannes has been pushing for 
simplified udev/systemd scripts.? Right now, the scripts require systemd 
as that call to create_cntrl can take a while - due to timeout on an io 
transaction or due to retries.? If the initial create_ctrl request can 
be relatively quick, we can get back to a simple udev script to 
scan/connect.? The trick being: the nvme cli for connect-all, which 
creates the discovery controller then connects to it to read discovery 
log messages - needs to glean the instance number for the discovery 
controller.? And to glean the number means create_ctrl needs to 
succeed.?? Mixing this need, and the desire to use the same reconnect 
delay/retry for initial connect - means I believe I have to formalize 
the "allocation state" of a controller, with 2 implementation options: 
a) a state flag tracking whether initial connect has been completed or 
not (e.g. what the new flag should be set to); or b) deal with how we 
always create an initial io tag set and allow the later association 
under the CONNECT state resize it based on the new association and 
resume it.

What I'd like to have you do is drop this patch and the 5th patch. The 
5th patch removes the NEW->LIVE transition.? Leave that, so the FC can 
continue to work as is without this 3rd patch.?? Then I can implement 
the work above independently.? I can resubmit the 5th patch when FC no 
longer needs it.

-- james

Note: as far as test the fc transport - that is what the fcloop driver 
is for.




> @@ -2704,7 +2704,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>    * on the link side, recreates the controller association.
>    */
>   static int
> -nvme_fc_create_association(struct nvme_fc_ctrl *ctrl)
> +nvme_fc_create_association(struct nvme_fc_ctrl *ctrl, bool new)
>   {
>   	struct nvmf_ctrl_options *opts = ctrl->ctrl.opts;
>   	int ret;
> @@ -2735,7 +2735,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   	if (ret)
>   		goto out_delete_hw_queue;
>   
> -	if (ctrl->ctrl.state != NVME_CTRL_NEW)
> +	if (!new)
>   		blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
>   
>   	ret = nvmf_connect_admin_queue(&ctrl->ctrl);
> @@ -2801,7 +2801,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   	 */
>   
>   	if (ctrl->ctrl.queue_count > 1) {
> -		if (ctrl->ctrl.state == NVME_CTRL_NEW)
> +		if (new)
>   			ret = nvme_fc_create_io_queues(ctrl);
>   		else
>   			ret = nvme_fc_reinit_io_queues(ctrl);
> @@ -3007,7 +3007,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   	}
>   
>   	if (ctrl->rport->remoteport.port_state == FC_OBJSTATE_ONLINE)
> -		ret = nvme_fc_create_association(ctrl);
> +		ret = nvme_fc_create_association(ctrl, false);
>   	else
>   		ret = -ENOTCONN;
>   
> @@ -3042,7 +3042,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   			container_of(to_delayed_work(work),
>   				struct nvme_fc_ctrl, connect_work);
>   
> -	ret = nvme_fc_create_association(ctrl);
> +	ret = nvme_fc_create_association(ctrl, false);
>   	if (ret)
>   		nvme_fc_reconnect_or_delete(ctrl, ret);
>   	else
> @@ -3096,6 +3096,7 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   	struct nvme_fc_ctrl *ctrl;
>   	unsigned long flags;
>   	int ret, idx, retry;
> +	bool changed;
>   
>   	if (!(rport->remoteport.port_role &
>   	    (FC_PORT_ROLE_NVME_DISCOVERY | FC_PORT_ROLE_NVME_TARGET))) {
> @@ -3212,8 +3213,11 @@ static inline blk_status_t nvme_fc_is_ready(struct nvme_fc_queue *queue,
>   	 * request fails, retry the initial connection creation up to
>   	 * three times before giving up and declaring failure.
>   	 */
> +	changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING);
> +	WARN_ON_ONCE(!changed);
> +
>   	for (retry = 0; retry < 3; retry++) {
> -		ret = nvme_fc_create_association(ctrl);
> +		ret = nvme_fc_create_association(ctrl, true);
>   		if (!ret)
>   			break;
>   	}

note: the diff, with everything tagged as being in the routine 
nvme_fc_is_ready() made the patch very odd to read