[PATCH 0/3] cxlflash: Minor fix and EH refactoring

[PATCH 0/3] cxlflash: Minor fix and EH refactoring

[Matthew R. Ochs]

This small series fixes a recently injected double free and also includes
two patches to refactor the host and reset handlers to ease the burden of
making this driver compatible with an updated SCSI EH framework.

This series is intended for 4.13 and is bisectable.

Matthew R. Ochs (3):
  cxlflash: Avoid double free of character device
  cxlflash: Update send_tmf() parameters
  cxlflash: Update debug prints in reset handlers

 drivers/scsi/cxlflash/main.c | 44 +++++++++++++++-----------------------------
 1 file changed, 15 insertions(+), 29 deletions(-)

-- 
2.1.0

[PATCH 1/3] cxlflash: Avoid double free of character device

[Matthew R. Ochs]

The device_unregister() service used when cleaning up the character
device is already responsible for the internal state associated with
the device upon successful creation. As the cxlflash driver does not
obtain a second reference to the character device, the explicit call
to put_device() is not required and can lead to an inconsistent sysfs
among other issues as the reference is no longer valid after the first
put_device() is performed.

Remove the unnecessary put_device() to remedy this issue.

Fixes: a834a36b57d9 ("scsi: cxlflash: Create character device to provide host management interface")
Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 7a787b6..455564f 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -923,7 +923,6 @@ static void cxlflash_put_minor(int minor)
  */
 static void cxlflash_release_chrdev(struct cxlflash_cfg *cfg)
 {
-	put_device(cfg->chardev);
 	device_unregister(cfg->chardev);
 	cfg->chardev = NULL;
 	cdev_del(&cfg->cdev);
-- 
2.1.0

Re: [PATCH 1/3] cxlflash: Avoid double free of character device

[Hannes Reinecke]

On 06/28/2017 07:14 PM, Matthew R. Ochs wrote:
> The device_unregister() service used when cleaning up the character
> device is already responsible for the internal state associated with
> the device upon successful creation. As the cxlflash driver does not
> obtain a second reference to the character device, the explicit call
> to put_device() is not required and can lead to an inconsistent sysfs
> among other issues as the reference is no longer valid after the first
> put_device() is performed.
> 
> Remove the unnecessary put_device() to remedy this issue.
> 
> Fixes: a834a36b57d9 ("scsi: cxlflash: Create character device to provide host management interface")
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>> ---
>  drivers/scsi/cxlflash/main.c | 1 -
>  1 file changed, 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 2/3] cxlflash: Update send_tmf() parameters

[Matthew R. Ochs]

The current send_tmf() implementation is based on the caller providing
a SCSI command reference. In reality all that is needed is a SCSI device
reference as the routine uses a private command.

Refactor send_tmf() to pass the private adapter configuration reference
and a SCSI device reference. As a nice side effect, this will ease the
burden of converting caller routines to be based solely off of a SCSI
device reference.

Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 455564f..7054e11 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -459,21 +459,20 @@ static u32 cmd_to_target_hwq(struct Scsi_Host *host, struct scsi_cmnd *scp,
 
 /**
  * send_tmf() - sends a Task Management Function (TMF)
- * @afu:	AFU to checkout from.
- * @scp:	SCSI command from stack describing target.
+ * @cfg:	Internal structure associated with the host.
+ * @sdev:	SCSI device destined for TMF.
  * @tmfcmd:	TMF command to send.
  *
  * Return:
  *	0 on success, SCSI_MLQUEUE_HOST_BUSY or -errno on failure
  */
-static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
+static int send_tmf(struct cxlflash_cfg *cfg, struct scsi_device *sdev,
+		    u64 tmfcmd)
 {
-	struct Scsi_Host *host = scp->device->host;
-	struct cxlflash_cfg *cfg = shost_priv(host);
+	struct afu *afu = cfg->afu;
 	struct afu_cmd *cmd = NULL;
 	struct device *dev = &cfg->dev->dev;
-	int hwq_index = cmd_to_target_hwq(host, scp, afu);
-	struct hwq *hwq = get_hwq(afu, hwq_index);
+	struct hwq *hwq = get_hwq(afu, PRIMARY_HWQ);
 	char *buf = NULL;
 	ulong lock_flags;
 	int rc = 0;
@@ -500,12 +499,12 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd)
 
 	cmd->parent = afu;
 	cmd->cmd_tmf = true;
-	cmd->hwq_index = hwq_index;
+	cmd->hwq_index = hwq->index;
 
 	cmd->rcb.ctx_id = hwq->ctx_hndl;
 	cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
-	cmd->rcb.port_sel = CHAN2PORTMASK(scp->device->channel);
-	cmd->rcb.lun_id = lun_to_lunid(scp->device->lun);
+	cmd->rcb.port_sel = CHAN2PORTMASK(sdev->channel);
+	cmd->rcb.lun_id = lun_to_lunid(sdev->lun);
 	cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID |
 			      SISL_REQ_FLAGS_SUP_UNDERRUN |
 			      SISL_REQ_FLAGS_TMF_CMD);
@@ -2430,15 +2429,15 @@ static int cxlflash_eh_abort_handler(struct scsi_cmnd *scp)
 static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)
 {
 	int rc = SUCCESS;
-	struct Scsi_Host *host = scp->device->host;
+	struct scsi_device *sdev = scp->device;
+	struct Scsi_Host *host = sdev->host;
 	struct cxlflash_cfg *cfg = shost_priv(host);
 	struct device *dev = &cfg->dev->dev;
-	struct afu *afu = cfg->afu;
 	int rcr = 0;
 
 	dev_dbg(dev, "%s: (scp=%p) %d/%d/%d/%llu "
 		"cdb=(%08x-%08x-%08x-%08x)\n", __func__, scp, host->host_no,
-		scp->device->channel, scp->device->id, scp->device->lun,
+		sdev->channel, sdev->id, sdev->lun,
 		get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
 		get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
 		get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
@@ -2447,7 +2446,7 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)
 retry:
 	switch (cfg->state) {
 	case STATE_NORMAL:
-		rcr = send_tmf(afu, scp, TMF_LUN_RESET);
+		rcr = send_tmf(cfg, sdev, TMF_LUN_RESET);
 		if (unlikely(rcr))
 			rc = FAILED;
 		break;
-- 
2.1.0

Re: [PATCH 2/3] cxlflash: Update send_tmf() parameters

[Hannes Reinecke]

On 06/28/2017 07:14 PM, Matthew R. Ochs wrote:
> The current send_tmf() implementation is based on the caller providing
> a SCSI command reference. In reality all that is needed is a SCSI device
> reference as the routine uses a private command.
> 
> Refactor send_tmf() to pass the private adapter configuration reference
> and a SCSI device reference. As a nice side effect, this will ease the
> burden of converting caller routines to be based solely off of a SCSI
> device reference.
> 
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> ---
>  drivers/scsi/cxlflash/main.c | 27 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 14 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 3/3] cxlflash: Update debug prints in reset handlers

[Matthew R. Ochs]

The device and host reset handler contain debug prints to help
identify the entities being reset. Today these reset handlers
are based on a SCSI EH design that uses a SCSI command reference
as a means of identifying the target entity. As such, the debug
trace includes the SCSI command pointer and associated CDB. This
is not necessary as the SCSI command is simply the messenger in
these scenarios.

Refactor the debug prints in the host and reset handlers to only
present information that is applicable given the function scope.

Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
---
 drivers/scsi/cxlflash/main.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 7054e11..077f62e 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -2435,14 +2435,8 @@ static int cxlflash_eh_device_reset_handler(struct scsi_cmnd *scp)
 	struct device *dev = &cfg->dev->dev;
 	int rcr = 0;
 
-	dev_dbg(dev, "%s: (scp=%p) %d/%d/%d/%llu "
-		"cdb=(%08x-%08x-%08x-%08x)\n", __func__, scp, host->host_no,
-		sdev->channel, sdev->id, sdev->lun,
-		get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
-
+	dev_dbg(dev, "%s: %d/%d/%d/%llu\n", __func__,
+		host->host_no, sdev->channel, sdev->id, sdev->lun);
 retry:
 	switch (cfg->state) {
 	case STATE_NORMAL:
@@ -2483,13 +2477,7 @@ static int cxlflash_eh_host_reset_handler(struct scsi_cmnd *scp)
 	struct cxlflash_cfg *cfg = shost_priv(host);
 	struct device *dev = &cfg->dev->dev;
 
-	dev_dbg(dev, "%s: (scp=%p) %d/%d/%d/%llu "
-		"cdb=(%08x-%08x-%08x-%08x)\n", __func__, scp, host->host_no,
-		scp->device->channel, scp->device->id, scp->device->lun,
-		get_unaligned_be32(&((u32 *)scp->cmnd)[0]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[1]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[2]),
-		get_unaligned_be32(&((u32 *)scp->cmnd)[3]));
+	dev_dbg(dev, "%s: %d\n", __func__, host->host_no);
 
 	switch (cfg->state) {
 	case STATE_NORMAL:
-- 
2.1.0

Re: [PATCH 3/3] cxlflash: Update debug prints in reset handlers

[Hannes Reinecke]

On 06/28/2017 07:14 PM, Matthew R. Ochs wrote:
> The device and host reset handler contain debug prints to help
> identify the entities being reset. Today these reset handlers
> are based on a SCSI EH design that uses a SCSI command reference
> as a means of identifying the target entity. As such, the debug
> trace includes the SCSI command pointer and associated CDB. This
> is not necessary as the SCSI command is simply the messenger in
> these scenarios.
> 
> Refactor the debug prints in the host and reset handlers to only
> present information that is applicable given the function scope.
> 
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> ---
>  drivers/scsi/cxlflash/main.c | 18 +++---------------
>  1 file changed, 3 insertions(+), 15 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 0/3] cxlflash: Minor fix and EH refactoring

[Martin K. Petersen]

Matthew,

> This small series fixes a recently injected double free and also
> includes two patches to refactor the host and reset handlers to ease
> the burden of making this driver compatible with an updated SCSI EH
> framework.

Applied to 4.13/scsi-queue. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering