From patchwork Fri Nov 3 22:56:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Smart X-Patchwork-Id: 10041313 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2375160384 for ; Fri, 3 Nov 2017 22:57:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 15891299CE for ; Fri, 3 Nov 2017 22:57:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0AA17299E1; Fri, 3 Nov 2017 22:57:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.5 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6CEB9299CE for ; Fri, 3 Nov 2017 22:57:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756147AbdKCW5J (ORCPT ); Fri, 3 Nov 2017 18:57:09 -0400 Received: from mail-qt0-f194.google.com ([209.85.216.194]:46812 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752509AbdKCW4u (ORCPT ); Fri, 3 Nov 2017 18:56:50 -0400 Received: by mail-qt0-f194.google.com with SMTP id 1so5073038qtn.3 for ; Fri, 03 Nov 2017 15:56:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=ySqWrJHpXhlcxWilhzs2oL3TjHgWOBr7WTe0cy4RZ3I=; b=YthoH05l/Stfd1NpeGVXLf3udwztqk9IH7C63MexRgxuyN/IaRkEvwJcrs9POQX287 OitrScKoGLU8jU66V23HbiRHW48a8V7LoVyiTJIXjC42SUjDJYs0ANm32rv/nF8MqAT3 IVzrr2tKrXiN5bsuZMj+KqLhtGuq5E35JwmWqolKTQBU6CKQwra8qMqYmBKFgtSzd3ve Kd8KGPTKryMzHVAv+oPI7VHt3I/tcd/YI5qe16LLBnsLKHz7+9YXMEr/RYjiW4OLwj+x loC5iB+4Rm+Jltprc7bd+ZKYB4mAVtim5oU+NFZCTO7cq0l/NM/rsxO4RNyFTotUFJ4Q J+fw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=ySqWrJHpXhlcxWilhzs2oL3TjHgWOBr7WTe0cy4RZ3I=; b=TVSGvnLxiz/jADBNLBl2gPI5f0iOFkX4lJL8QNaVmxFfC4ejmL5knbHJpad4akDBzA m1t14j/k2YjEPLgw5zQJkWfkha2dxCYFy9mJm5VTEQFhpBgQQ9UfsJgOh9dmk3XjgRvv yi4GcbVtxmTtUJGOJY1u8Qq6MuxX9+rO+33lwKX3v61jljT6ToG48wc5sFYEtmEcwpHx TgNxcm9BXglm3pEyz8qcbBUZ7zn3OB/dpOQf4sXx2zUbyu4oHtAEYss4mg7bEwwBEmOv znkwfMTj3BglIc5r6CIgN0+b0yoqanKT2ERFS9aja78YMxb565Itxf4ejJLUFr6tQLst +uHQ== X-Gm-Message-State: AMCzsaVACYAqZOajTcCSlbPy/IXYMp8s8ktV+Y177ZHbQFPwlO8mxKMZ avCL22pM+sfxIabX7GqIizLeXWdK X-Google-Smtp-Source: ABhQp+Qxlt4QfczDFzsUwhoFKZCkmt6USMLzsi2DJ1DTFCtTyl+CyA2YZWFFsPW3p8eSa47cdT2rMg== X-Received: by 10.200.46.114 with SMTP id s47mr12392413qta.165.1509749809618; Fri, 03 Nov 2017 15:56:49 -0700 (PDT) Received: from pallmd1.broadcom.com ([192.19.255.250]) by smtp.gmail.com with ESMTPSA id v21sm4765626qtv.21.2017.11.03.15.56.48 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 03 Nov 2017 15:56:49 -0700 (PDT) From: James Smart To: linux-scsi@vger.kernel.org Cc: James Smart , Dick Kennedy , James Smart Subject: [PATCH 13/17] lpfc: Correct driver deregistrations with host nvme transport Date: Fri, 3 Nov 2017 15:56:24 -0700 Message-Id: <20171103225628.24716-14-jsmart2021@gmail.com> X-Mailer: git-send-email 2.13.1 In-Reply-To: <20171103225628.24716-1-jsmart2021@gmail.com> References: <20171103225628.24716-1-jsmart2021@gmail.com> Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The driver's interaction with the host nvme transport has been incorrect for a while. The driver did not wait for the unregister callbacks (waited only 5 jiffies). Thus the driver may remove objects that may be referenced by subsequent abort commands from the transport, and the actual unregister callback was effectively a noop. This was especially problematic if the driver was unloaded. The driver now waits for the unregister callbacks, as it should, before continuing with teardown. Signed-off-by: Dick Kennedy Signed-off-by: James Smart --- drivers/scsi/lpfc/lpfc_disc.h | 2 + drivers/scsi/lpfc/lpfc_nvme.c | 113 ++++++++++++++++++++++++++++++++++++++++-- drivers/scsi/lpfc/lpfc_nvme.h | 2 + 3 files changed, 113 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/lpfc/lpfc_disc.h b/drivers/scsi/lpfc/lpfc_disc.h index f9a566eaef04..5a7547f9d8d8 100644 --- a/drivers/scsi/lpfc/lpfc_disc.h +++ b/drivers/scsi/lpfc/lpfc_disc.h @@ -134,6 +134,8 @@ struct lpfc_nodelist { struct lpfc_scsicmd_bkt *lat_data; /* Latency data */ uint32_t fc4_prli_sent; uint32_t upcall_flags; +#define NLP_WAIT_FOR_UNREG 0x1 + uint32_t nvme_fb_size; /* NVME target's supported byte cnt */ #define NVME_FB_BIT_SHIFT 9 /* PRLI Rsp first burst in 512B units. */ }; diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c index d3ada630b427..12d09a6a4563 100644 --- a/drivers/scsi/lpfc/lpfc_nvme.c +++ b/drivers/scsi/lpfc/lpfc_nvme.c @@ -154,6 +154,10 @@ lpfc_nvme_localport_delete(struct nvme_fc_local_port *localport) { struct lpfc_nvme_lport *lport = localport->private; + lpfc_printf_vlog(lport->vport, KERN_INFO, LOG_NVME, + "6173 localport %p delete complete\n", + lport); + /* release any threads waiting for the unreg to complete */ complete(&lport->lport_unreg_done); } @@ -946,10 +950,19 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn, freqpriv->nvme_buf = NULL; /* NVME targets need completion held off until the abort exchange - * completes. + * completes unless the NVME Rport is getting unregistered. */ - if (!(lpfc_ncmd->flags & LPFC_SBUF_XBUSY)) + if (!(lpfc_ncmd->flags & LPFC_SBUF_XBUSY) || + ndlp->upcall_flags & NLP_WAIT_FOR_UNREG) { + /* Clear the XBUSY flag to prevent double completions. + * The nvme rport is getting unregistered and there is + * no need to defer the IO. + */ + if (lpfc_ncmd->flags & LPFC_SBUF_XBUSY) + lpfc_ncmd->flags &= ~LPFC_SBUF_XBUSY; + nCmd->done(nCmd); + } spin_lock_irqsave(&phba->hbalock, flags); lpfc_ncmd->nrport = NULL; @@ -2234,6 +2247,47 @@ lpfc_nvme_create_localport(struct lpfc_vport *vport) return ret; } +/* lpfc_nvme_lport_unreg_wait - Wait for the host to complete an lport unreg. + * + * The driver has to wait for the host nvme transport to callback + * indicating the localport has successfully unregistered all + * resources. Since this is an uninterruptible wait, loop every ten + * seconds and print a message indicating no progress. + * + * An uninterruptible wait is used because of the risk of transport-to- + * driver state mismatch. + */ +void +lpfc_nvme_lport_unreg_wait(struct lpfc_vport *vport, + struct lpfc_nvme_lport *lport) +{ +#if (IS_ENABLED(CONFIG_NVME_FC)) + u32 wait_tmo; + int ret; + + /* Host transport has to clean up and confirm requiring an indefinite + * wait. Print a message if a 10 second wait expires and renew the + * wait. This is unexpected. + */ + wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000); + while (true) { + ret = wait_for_completion_timeout(&lport->lport_unreg_done, + wait_tmo); + if (unlikely(!ret)) { + lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR, + "6176 Lport %p Localport %p wait " + "timed out. Renewing.\n", + lport, vport->localport); + continue; + } + break; + } + lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR, + "6177 Lport %p Localport %p Complete Success\n", + lport, vport->localport); +#endif +} + /** * lpfc_nvme_destroy_localport - Destroy lpfc_nvme bound to nvme transport. * @pnvme: pointer to lpfc nvme data structure. @@ -2268,7 +2322,11 @@ lpfc_nvme_destroy_localport(struct lpfc_vport *vport) */ init_completion(&lport->lport_unreg_done); ret = nvme_fc_unregister_localport(localport); - wait_for_completion_timeout(&lport->lport_unreg_done, 5); + + /* Wait for completion. This either blocks + * indefinitely or succeeds + */ + lpfc_nvme_lport_unreg_wait(vport, lport); /* Regardless of the unregister upcall response, clear * nvmei_support. All rports are unregistered and the @@ -2424,6 +2482,47 @@ lpfc_nvme_register_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp) #endif } +/* lpfc_nvme_rport_unreg_wait - Wait for the host to complete an rport unreg. + * + * The driver has to wait for the host nvme transport to callback + * indicating the remoteport has successfully unregistered all + * resources. Since this is an uninterruptible wait, loop every ten + * seconds and print a message indicating no progress. + * + * An uninterruptible wait is used because of the risk of transport-to- + * driver state mismatch. + */ +void +lpfc_nvme_rport_unreg_wait(struct lpfc_vport *vport, + struct lpfc_nvme_rport *rport) +{ +#if (IS_ENABLED(CONFIG_NVME_FC)) + u32 wait_tmo; + int ret; + + /* Host transport has to clean up and confirm requiring an indefinite + * wait. Print a message if a 10 second wait expires and renew the + * wait. This is unexpected. + */ + wait_tmo = msecs_to_jiffies(LPFC_NVME_WAIT_TMO * 1000); + while (true) { + ret = wait_for_completion_timeout(&rport->rport_unreg_done, + wait_tmo); + if (unlikely(!ret)) { + lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_IOERR, + "6174 Rport %p Remoteport %p wait " + "timed out. Renewing.\n", + rport, rport->remoteport); + continue; + } + break; + } + lpfc_printf_vlog(vport, KERN_INFO, LOG_NVME_IOERR, + "6175 Rport %p Remoteport %p Complete Success\n", + rport, rport->remoteport); +#endif +} + /* lpfc_nvme_unregister_port - unbind the DID and port_role from this rport. * * There is no notion of Devloss or rport recovery from the current @@ -2480,14 +2579,20 @@ lpfc_nvme_unregister_port(struct lpfc_vport *vport, struct lpfc_nodelist *ndlp) /* No concern about the role change on the nvme remoteport. * The transport will update it. */ + ndlp->upcall_flags |= NLP_WAIT_FOR_UNREG; ret = nvme_fc_unregister_remoteport(remoteport); if (ret != 0) { lpfc_printf_vlog(vport, KERN_ERR, LOG_NVME_DISC, "6167 NVME unregister failed %d " "port_state x%x\n", ret, remoteport->port_state); + } else { + /* Wait for completion. This either blocks + * indefinitely or succeeds + */ + lpfc_nvme_rport_unreg_wait(vport, rport); + ndlp->upcall_flags &= ~NLP_WAIT_FOR_UNREG; } - } return; diff --git a/drivers/scsi/lpfc/lpfc_nvme.h b/drivers/scsi/lpfc/lpfc_nvme.h index fbfc1786cd04..903ec37f465f 100644 --- a/drivers/scsi/lpfc/lpfc_nvme.h +++ b/drivers/scsi/lpfc/lpfc_nvme.h @@ -27,6 +27,8 @@ #define LPFC_NVME_ERSP_LEN 0x20 +#define LPFC_NVME_WAIT_TMO 10 + struct lpfc_nvme_qhandle { uint32_t index; /* WQ index to use */ uint32_t qidx; /* queue index passed to create */