lkml.org 
[lkml]   [2019]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH AUTOSEL 5.0 69/79] nvme-fc: correct csn initialization and increments on error
    Date
    From: James Smart <jsmart2021@gmail.com>

    [ Upstream commit 67f471b6ed3b09033c4ac77ea03f92afdb1989fe ]

    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>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/nvme/host/fc.c | 20 +++++++++++++++-----
    1 file changed, 15 insertions(+), 5 deletions(-)

    diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
    index c37d5bbd72ab..8625b73d94bf 100644
    --- a/drivers/nvme/host/fc.c
    +++ b/drivers/nvme/host/fc.c
    @@ -1857,7 +1857,7 @@ nvme_fc_init_queue(struct nvme_fc_ctrl *ctrl, int idx)
    memset(queue, 0, sizeof(*queue));
    queue->ctrl = ctrl;
    queue->qnum = idx;
    - atomic_set(&queue->csn, 1);
    + atomic_set(&queue->csn, 0);
    queue->dev = ctrl->dev;

    if (idx > 0)
    @@ -1899,7 +1899,7 @@ nvme_fc_free_queue(struct nvme_fc_queue *queue)
    */

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

    static void
    @@ -2195,7 +2195,6 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
    {
    struct nvme_fc_cmd_iu *cmdiu = &op->cmd_iu;
    struct nvme_command *sqe = &cmdiu->sqe;
    - u32 csn;
    int ret, opstate;

    /*
    @@ -2210,8 +2209,6 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,

    /* format the FC-NVME CMD IU and fcp_req */
    cmdiu->connection_id = cpu_to_be64(queue->connection_id);
    - csn = atomic_inc_return(&queue->csn);
    - cmdiu->csn = cpu_to_be32(csn);
    cmdiu->data_len = cpu_to_be32(data_len);
    switch (io_dir) {
    case NVMEFC_FCP_WRITE:
    @@ -2269,11 +2266,24 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue,
    if (!(op->flags & FCOP_FLAGS_AEN))
    blk_mq_start_request(op->rq);

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

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

    --
    2.19.1
    \
     
     \ /
      Last update: 2019-04-27 03:52    [W:4.068 / U:0.408 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site