Skip to content

Commit 67f471b

Browse files
jsmart-ghChristoph Hellwig
authored andcommitted
nvme-fc: correct csn initialization and increments on error
This patch fixes a long-standing bug that initialized the FC-NVME cmnd iu CSN value to 1. Early FC-NVME specs had the connection starting with CSN=1. By the time the spec reached approval, the language had changed to state a connection should start with CSN=0. This patch corrects the initialization value for FC-NVME connections. Additionally, in reviewing the transport, the CSN value is assigned to the new IU early in the start routine. It's possible that a later dma map request may fail, causing the command to never be sent to the controller. Change the location of the assignment so that it is immediately prior to calling the lldd. Add a comment block to explain the impacts if the lldd were to additionally fail sending the command. Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com> Signed-off-by: James Smart <jsmart2021@gmail.com> Reviewed-by: Ewan D. Milne <emilne@redhat.com> Signed-off-by: Christoph Hellwig <hch@lst.de>
1 parent a3761c3 commit 67f471b

File tree

1 file changed

+15
-5
lines changed
  • drivers/nvme/host

1 file changed

+15
-5
lines changed

drivers/nvme/host/fc.c

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1845,7 +1845,7 @@ nvme_fc_init_queue(struct nvme_fc_ctrl *ctrl, int idx)
18451845
memset(queue, 0, sizeof(*queue));
18461846
queue->ctrl = ctrl;
18471847
queue->qnum = idx;
1848-
atomic_set(&queue->csn, 1);
1848+
atomic_set(&queue->csn, 0);
18491849
queue->dev = ctrl->dev;
18501850

18511851
if (idx > 0)
@@ -1887,7 +1887,7 @@ nvme_fc_free_queue(struct nvme_fc_queue *queue)
18871887
*/
18881888

18891889
queue->connection_id = 0;
1890-
atomic_set(&queue->csn, 1);
1890+
atomic_set(&queue->csn, 0);
18911891
}
18921892

18931893
static void
@@ -2183,7 +2183,6 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
21832183
{
21842184
struct nvme_fc_cmd_iu *cmdiu = &op->cmd_iu;
21852185
struct nvme_command *sqe = &cmdiu->sqe;
2186-
u32 csn;
21872186
int ret, opstate;
21882187

21892188
/*
@@ -2198,8 +2197,6 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
21982197

21992198
/* format the FC-NVME CMD IU and fcp_req */
22002199
cmdiu->connection_id = cpu_to_be64(queue->connection_id);
2201-
csn = atomic_inc_return(&queue->csn);
2202-
cmdiu->csn = cpu_to_be32(csn);
22032200
cmdiu->data_len = cpu_to_be32(data_len);
22042201
switch (io_dir) {
22052202
case NVMEFC_FCP_WRITE:
@@ -2257,11 +2254,24 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
22572254
if (!(op->flags & FCOP_FLAGS_AEN))
22582255
blk_mq_start_request(op->rq);
22592256

2257+
cmdiu->csn = cpu_to_be32(atomic_inc_return(&queue->csn));
22602258
ret = ctrl->lport->ops->fcp_io(&ctrl->lport->localport,
22612259
&ctrl->rport->remoteport,
22622260
queue->lldd_handle, &op->fcp_req);
22632261

22642262
if (ret) {
2263+
/*
2264+
* If the lld fails to send the command is there an issue with
2265+
* the csn value? If the command that fails is the Connect,
2266+
* no - as the connection won't be live. If it is a command
2267+
* post-connect, it's possible a gap in csn may be created.
2268+
* Does this matter? As Linux initiators don't send fused
2269+
* commands, no. The gap would exist, but as there's nothing
2270+
* that depends on csn order to be delivered on the target
2271+
* side, it shouldn't hurt. It would be difficult for a
2272+
* target to even detect the csn gap as it has no idea when the
2273+
* cmd with the csn was supposed to arrive.
2274+
*/
22652275
opstate = atomic_xchg(&op->state, FCPOP_STATE_COMPLETE);
22662276
__nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate);
22672277

0 commit comments

Comments
 (0)