diff mbox

[2/2] lpfc: support nvmet_fc defer_rcv callback

Message ID 20170801221240.31723-3-jsmart2021@gmail.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

James Smart Aug. 1, 2017, 10:12 p.m. UTC 
Currently, calls to nvmet_fc_rcv_fcp_req() always copied the
FC-NVME cmd iu to a temporary buffer before returning, allowing
the driver to immediately repost the buffer to the hardware.

To address timing conditions on queue element structures vs async
command reception, the nvmet_fc transport occasionally may need to
hold on to the command iu buffer for a short period. In these cases,
the nvmet_fc_rcv_fcp_req() will return a special return code
(-EOVERFLOW). In these cases, the LLDD must delay until the new
defer_rcv lldd callback is called before recycling the buffer back
to the hw.

This patch adds support for the new nvmet_fc transport defer_rcv
callback and recognition of the new error code when passing commands
to the transport.

This patch is intended to enter the kernel through the nvme block
tree which pulls in the nvmet_fc api change at the same time. It is
not to be merged via the scsi trees without the latest nvme support
in it.

Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <james.smart@broadcom.com>
---
 drivers/scsi/lpfc/lpfc_attr.c    |  4 +++-
 drivers/scsi/lpfc/lpfc_debugfs.c |  5 ++++-
 drivers/scsi/lpfc/lpfc_nvmet.c   | 30 ++++++++++++++++++++++++++++++
 drivers/scsi/lpfc/lpfc_nvmet.h   |  1 +
 4 files changed, 38 insertions(+), 2 deletions(-)

Comments

Johannes Thumshirn Aug. 2, 2017, 8:32 a.m. UTC | #1 
On Tue, Aug 01, 2017 at 03:12:40PM -0700, James Smart wrote:
> This patch is intended to enter the kernel through the nvme block
> tree which pulls in the nvmet_fc api change at the same time. It is
> not to be merged via the scsi trees without the latest nvme support
> in it.

This should be placed below the '---' separator, so git won't pick it up and
preserve it in the history.

[...]

>  
>  		len += snprintf(buf+len, PAGE_SIZE-len,
> -				"FCP: Rcv %08x Release %08x Drop %08x\n",
> +				"FCP: Rcv %08x Defer %08x Release %08x "
> +				"Drop %08x\n",

Please don't split the string across lines, it makes grepping hard.
Checkpatch actually warns you about that.

>  				atomic_read(&tgtp->rcv_fcp_cmd_in),
> +				atomic_read(&tgtp->rcv_fcp_cmd_defer),
>  				atomic_read(&tgtp->xmt_fcp_release),
>  				atomic_read(&tgtp->rcv_fcp_cmd_drop));
>  
> diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
> index 5cc8b0f7d885..744f3f395b64 100644
> --- a/drivers/scsi/lpfc/lpfc_debugfs.c
> +++ b/drivers/scsi/lpfc/lpfc_debugfs.c
> @@ -782,8 +782,11 @@ lpfc_debugfs_nvmestat_data(struct lpfc_vport *vport, char *buf, int size)
>  				atomic_read(&tgtp->xmt_ls_rsp_error));
>  
>  		len += snprintf(buf + len, size - len,
> -				"FCP: Rcv %08x Drop %08x\n",
> +				"FCP: Rcv %08x Defer %08x Release %08x "
> +				"Drop %08x\n",
>  				atomic_read(&tgtp->rcv_fcp_cmd_in),
> +				atomic_read(&tgtp->rcv_fcp_cmd_defer),
> +				atomic_read(&tgtp->xmt_fcp_release),
>  				atomic_read(&tgtp->rcv_fcp_cmd_drop));

Ditto.


[...]

> +	tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private;

No need to cast from void *

[...]

> +	/* Processing of FCP command is deferred */
> +	if (rc == -EOVERFLOW) {
> +		lpfc_nvmeio_data(phba,
> +				 "NVMET RCV BUSY: xri x%x sz %d from %06x\n",
> +				 oxid, size, sid);
> +		/* defer reposting rcv buffer till .defer_rcv callback */
> +		ctxp->rqb_buffer = (void *)nvmebuf;

nvmebuf is a 'struct rqb_dmabuf *' and ctxp->rqb_buffer expects 'struct
rqb_dmabuf *', why do you need a void * cast here?
Johannes Thumshirn Aug. 2, 2017, 9:17 a.m. UTC | #2 
On Wed, Aug 02, 2017 at 10:32:20AM +0200, Johannes Thumshirn wrote:
> On Tue, Aug 01, 2017 at 03:12:40PM -0700, James Smart wrote:
> > This patch is intended to enter the kernel through the nvme block
> > tree which pulls in the nvmet_fc api change at the same time. It is
> > not to be merged via the scsi trees without the latest nvme support
> > in it.
> 
> This should be placed below the '---' separator, so git won't pick it up and
> preserve it in the history.

Btw, I just found out and thought it's worth sharing: You can use 'git notes'
for these notes and if you use git format-patch --notes git automatically
places you r notes underneath the separator.

Byte,
	Johannes
James Smart Aug. 2, 2017, 2:32 p.m. UTC | #3 
On 8/2/2017 1:32 AM, Johannes Thumshirn wrote:
>>   		len += snprintf(buf+len, PAGE_SIZE-len,
>> -				"FCP: Rcv %08x Release %08x Drop %08x\n",
>> +				"FCP: Rcv %08x Defer %08x Release %08x "
>> +				"Drop %08x\n",
> 
> Please don't split the string across lines, it makes grepping hard.
> Checkpatch actually warns you about that.

The indentation and length of the string exceeds the 80 chararacters. so 
has to be split.  lpfc is littered with this in the debug path. It'll be 
one of the things we address in the efct refactoring. For now, this 
style has been grandfathered in.

-- james
Christoph Hellwig Aug. 10, 2017, 9:08 a.m. UTC | #4 
Applied to nvme-4.13 with the void casts removed, but the strings left
as the surrounding code.
diff mbox

Patch

diff --git a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c
index 4ed48ed38e79..7ee1a94c0b33 100644
--- a/drivers/scsi/lpfc/lpfc_attr.c
+++ b/drivers/scsi/lpfc/lpfc_attr.c
@@ -205,8 +205,10 @@  lpfc_nvme_info_show(struct device *dev, struct device_attribute *attr,
 				atomic_read(&tgtp->xmt_ls_rsp_error));
 
 		len += snprintf(buf+len, PAGE_SIZE-len,
-				"FCP: Rcv %08x Release %08x Drop %08x\n",
+				"FCP: Rcv %08x Defer %08x Release %08x "
+				"Drop %08x\n",
 				atomic_read(&tgtp->rcv_fcp_cmd_in),
+				atomic_read(&tgtp->rcv_fcp_cmd_defer),
 				atomic_read(&tgtp->xmt_fcp_release),
 				atomic_read(&tgtp->rcv_fcp_cmd_drop));
 
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 5cc8b0f7d885..744f3f395b64 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -782,8 +782,11 @@  lpfc_debugfs_nvmestat_data(struct lpfc_vport *vport, char *buf, int size)
 				atomic_read(&tgtp->xmt_ls_rsp_error));
 
 		len += snprintf(buf + len, size - len,
-				"FCP: Rcv %08x Drop %08x\n",
+				"FCP: Rcv %08x Defer %08x Release %08x "
+				"Drop %08x\n",
 				atomic_read(&tgtp->rcv_fcp_cmd_in),
+				atomic_read(&tgtp->rcv_fcp_cmd_defer),
+				atomic_read(&tgtp->xmt_fcp_release),
 				atomic_read(&tgtp->rcv_fcp_cmd_drop));
 
 		if (atomic_read(&tgtp->rcv_fcp_cmd_in) !=
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index fbeec344c6cc..a5dc1b83064a 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -841,12 +841,31 @@  lpfc_nvmet_xmt_fcp_release(struct nvmet_fc_target_port *tgtport,
 	lpfc_nvmet_ctxbuf_post(phba, ctxp->ctxbuf);
 }
 
+static void
+lpfc_nvmet_defer_rcv(struct nvmet_fc_target_port *tgtport,
+		     struct nvmefc_tgt_fcp_req *rsp)
+{
+	struct lpfc_nvmet_tgtport *tgtp;
+	struct lpfc_nvmet_rcv_ctx *ctxp =
+		container_of(rsp, struct lpfc_nvmet_rcv_ctx, ctx.fcp_req);
+	struct rqb_dmabuf *nvmebuf = ctxp->rqb_buffer;
+	struct lpfc_hba *phba = ctxp->phba;
+
+	lpfc_nvmeio_data(phba, "NVMET DEFERRCV: xri x%x sz %d CPU %02x\n",
+			 ctxp->oxid, ctxp->size, smp_processor_id());
+
+	tgtp = (struct lpfc_nvmet_tgtport *)phba->targetport->private;
+	atomic_inc(&tgtp->rcv_fcp_cmd_defer);
+	lpfc_rq_buf_free(phba, &nvmebuf->hbuf); /* repost */
+}
+
 static struct nvmet_fc_target_template lpfc_tgttemplate = {
 	.targetport_delete = lpfc_nvmet_targetport_delete,
 	.xmt_ls_rsp     = lpfc_nvmet_xmt_ls_rsp,
 	.fcp_op         = lpfc_nvmet_xmt_fcp_op,
 	.fcp_abort      = lpfc_nvmet_xmt_fcp_abort,
 	.fcp_req_release = lpfc_nvmet_xmt_fcp_release,
+	.defer_rcv	= lpfc_nvmet_defer_rcv,
 
 	.max_hw_queues  = 1,
 	.max_sgl_segments = LPFC_NVMET_DEFAULT_SEGS,
@@ -1504,6 +1523,17 @@  lpfc_nvmet_unsol_fcp_buffer(struct lpfc_hba *phba,
 		return;
 	}
 
+	/* Processing of FCP command is deferred */
+	if (rc == -EOVERFLOW) {
+		lpfc_nvmeio_data(phba,
+				 "NVMET RCV BUSY: xri x%x sz %d from %06x\n",
+				 oxid, size, sid);
+		/* defer reposting rcv buffer till .defer_rcv callback */
+		ctxp->rqb_buffer = (void *)nvmebuf;
+		atomic_inc(&tgtp->rcv_fcp_cmd_out);
+		return;
+	}
+
 	atomic_inc(&tgtp->rcv_fcp_cmd_drop);
 	lpfc_printf_log(phba, KERN_ERR, LOG_NVME_IOERR,
 			"6159 FCP Drop IO x%x: err x%x: x%x x%x x%x\n",
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.h b/drivers/scsi/lpfc/lpfc_nvmet.h
index e675ef17be08..48a76788b003 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.h
+++ b/drivers/scsi/lpfc/lpfc_nvmet.h
@@ -49,6 +49,7 @@  struct lpfc_nvmet_tgtport {
 	atomic_t rcv_fcp_cmd_in;
 	atomic_t rcv_fcp_cmd_out;
 	atomic_t rcv_fcp_cmd_drop;
+	atomic_t rcv_fcp_cmd_defer;
 	atomic_t xmt_fcp_release;
 
 	/* Stats counters - lpfc_nvmet_xmt_fcp_op */