Re: [RFC 6/9] scsi: fc: start decoupling fc_block_scsi_eh from scsi_cmnd

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> Scsi_cmnd is an unsuitable argument for eh_device_reset_handler(),
> eh_target_reset_handler(), and eh_host_reset_handler()
> which do not have the scope of one single SCSI command.
> These callbacks tend to use fc_block_scsi_eh() requiring scsi_cmnd.
> In order to start decoupling above eh callbacks from scsi_cmnd,
> introduce a new variant of the function called fc_block_rport()
> taking an fc_rport as argument.
> Refactor the old fc_block_scsi_eh() to simply delegate to fc_block_rport().
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/scsi/scsi_transport_fc.c | 31 ++-
>  include/scsi/scsi_transport_fc.h |  1 +
>  2 files changed, 27 insertions(+), 5 deletions(-)
> 
Very good.
I need that for my patchset as well.
Martin, would it be possible to apply this independent of this patchset?
It would help me a lot when redrafting my patchset.

Reviewed-by: Hannes Reinecke 

Cheers,

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

Re: [RFC 5/9] zfcp: decouple SCSI setup of TMF from scsi_cmnd

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> Actually change the signature of zfcp_fsf_fcp_task_mgmt().
> Since it was prepared in the previous patch, we only need to delete
> some local auto variables which are now the intended arguments.
> 
> Refactor zfcp_scsi_forget_cmnds() to now take a mandatory zfcp_port
> and an optional zfcp_scsi_dev, which can be NULL for target reset,
> instead of a mandatory zfcp_scsi_dev.
> 
> Prepare zfcp_fsf_fcp_task_mgmt's caller zfcp_task_mgmt_function()
> to have its function body only depend on a mandatory zfcp_port
> and an optional scsi_device, which can be NULL for target reset.
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/s390/scsi/zfcp_ext.h  |  4 +++-
>  drivers/s390/scsi/zfcp_fsf.c  | 15 ---
>  drivers/s390/scsi/zfcp_scsi.c | 28 +++-
>  3 files changed, 30 insertions(+), 17 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

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

Re: [RFC 4/9] zfcp: decouple FSF request setup of TMF from scsi_cmnd

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> The scsi_device argument of zfcp_fc_fcp_tm() can now be NULL.
> 
> In zfcp_fsf_fcp_task_mgmt() resolve the still old argument scsi_cmnd
> into scsi_device very early and only depend on scsi_device and derived
> objects in the function body.
> 
> Scsi_device and derived zfcp_scsi_dev can later be NULL for the
> target reset case, so do not depend on them unconditionally.
> For the generic case, rather change to using zfcp_port directly.
> 
> This prepares to later change the function signature replacing the
> scsi_cmnd argument with zfcp_port and an
> optional scsi_device which can be NULL.
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/s390/scsi/zfcp_fc.h  |  6 --
>  drivers/s390/scsi/zfcp_fsf.c | 25 +
>  2 files changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
> index 24949868d027..0e5b01c33873 100644
> --- a/drivers/s390/scsi/zfcp_fc.h
> +++ b/drivers/s390/scsi/zfcp_fc.h
> @@ -235,13 +235,15 @@ void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct 
> scsi_cmnd *scsi)
>  /**
>   * zfcp_fc_fcp_tm() - Setup FCP command as task management command.
>   * @fcp: Pointer to FCP_CMND IU to set up.
> - * @dev: Pointer to SCSI_device where to send the task management command.
> + * @dev: Pointer to SCSI device if LUN Reset TMF, or %NULL.
>   * @tm_flags: Task management flags to setup tm command.
>   */
>  static inline
>  void zfcp_fc_fcp_tm(struct fcp_cmnd *fcp, struct scsi_device *dev, u8 
> tm_flags)
>  {
> - int_to_scsilun(dev->lun, (struct scsi_lun *) >fc_lun);
> + if (dev)
> + int_to_scsilun(dev->lun, (struct scsi_lun *) >fc_lun);
> +
>   fcp->fc_tm_flags = tm_flags;
>  }
>  
Hmm. This function is becoming so small, _and_ with a conditional to
boot. Maybe you should simply open-coding it?

> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
> index f221a34c26df..2dc7d2a6f6ea 100644
> --- a/drivers/s390/scsi/zfcp_fsf.c
> +++ b/drivers/s390/scsi/zfcp_fsf.c
> @@ -2339,13 +2339,19 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
> scsi_cmnd *scmnd,
>  {
>   struct zfcp_fsf_req *req = NULL;
>   struct fcp_cmnd *fcp_cmnd;
> - struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scmnd->device);
> - struct zfcp_qdio *qdio = zfcp_sdev->port->adapter->qdio;
> + struct scsi_device *sdev = scmnd->device;
> + struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
> + struct zfcp_port *port = zfcp_sdev->port;
> + struct zfcp_qdio *qdio = port->adapter->qdio;
>  
> - if (unlikely(!(atomic_read(_sdev->status) &
> + if (unlikely(!(atomic_read(>status) &
>  ZFCP_STATUS_COMMON_UNBLOCKED)))
>   return NULL;
>  
> + if (unlikely(zfcp_sdev && !(atomic_read(_sdev->status) &
> + ZFCP_STATUS_COMMON_UNBLOCKED)))
> + return NULL;
> +
>   spin_lock_irq(>req_q_lock);
>   if (zfcp_qdio_sbal_get(qdio))
>   goto out;
> @@ -2360,18 +2366,21 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
> scsi_cmnd *scmnd,
>   }
>  
>   fcp_cmnd = >qtcb->bottom.io.fcp_cmnd.iu;
> - req->data = (fcp_cmnd->fc_tm_flags & FCP_TMF_LUN_RESET) ?
> - scmnd->device : (void *)sdev_to_zfcp(scmnd->device)->port;
> + if (fcp_cmnd->fc_tm_flags & FCP_TMF_LUN_RESET) {
> + req->data = sdev;
> + req->qtcb->header.lun_handle = zfcp_sdev->lun_handle;
> + } else
> + req->data = port;
> +
>   req->handler = zfcp_fsf_fcp_task_mgmt_handler;
> - req->qtcb->header.lun_handle = zfcp_sdev->lun_handle;
> - req->qtcb->header.port_handle = zfcp_sdev->port->handle;
> + req->qtcb->header.port_handle = port->handle;
>   req->qtcb->bottom.io.data_direction = FSF_DATADIR_CMND;
>   req->qtcb->bottom.io.service_class = FSF_CLASS_3;
>   req->qtcb->bottom.io.fcp_cmnd_length = FCP_CMND_LEN;
>  
>   zfcp_qdio_set_sbale_last(qdio, >qdio_req);
>  
> - zfcp_fc_fcp_tm(fcp_cmnd, scmnd->device, tm_flags);
> + zfcp_fc_fcp_tm(fcp_cmnd, sdev, tm_flags);
>  
>   zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT);
>   if (!zfcp_fsf_req_send(req))
> 
Otherwise:

Reviewed-by: Hannes Reinecke 

Cheers,

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

Re: [RFC 3/9] zfcp: split FCP_CMND IU setup between SCSI I/O and TMF again

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> This reverts commit 2443c8b23aea ("[SCSI] zfcp: Merge FCP task management
> setup with regular FCP command setup"), because this introduced a
> dependency on the unsuitable SCSI command for scsi_eh / TMF.
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/s390/scsi/zfcp_fc.h  | 22 ++
>  drivers/s390/scsi/zfcp_fsf.c |  4 ++--
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

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

Re: [RFC 2/9] zfcp: decouple TMF response handler from scsi_cmnd

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> Do not get scsi_device via req->data any more, but pass an optional(!)
> scsi_device to zfcp_fsf_fcp_handler_common(). The latter must now guard
> any access to scsi_device as it can be NULL.
> 
> Since we always have at least a zfcp port as scope, pass this as mandatory
> argument to zfcp_fsf_fcp_handler_common() because we cannot get it through
> scsi_device => zfcp_scsi_dev => port any more.
> 
> Hence, the callers of zfcp_fsf_fcp_handler_common() must resolve req->data.
> 
> TMF handling now has different context data in fsf_req->data
> depending on the TMF scope in fcp_cmnd->fc_tm_flags:
> * scsi_device if FCP_TMF_LUN_RESET,
> * zfcp_port if FCP_TMF_TGT_RESET.
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/s390/scsi/zfcp_fsf.c | 72 
> 
>  1 file changed, 46 insertions(+), 26 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

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

Re: [RFC 1/9] zfcp: drop unsuitable scsi_cmnd usage from SCSI traces for scsi_eh / TMF

[Hannes Reinecke]

On 07/25/2017 04:14 PM, Steffen Maier wrote:
> The SCSI command pointer passed to scsi_eh callbacks is just one arbitrary
> command of potentially many that are in the eh queue to be processed.
> The command is only used to indirectly pass the TMF scope in terms of
> SCSI ID/target and SCSI LUN for LUN reset.
> 
> Hence, zfcp had filled in SCSI trace record fields which do not really
> belong to the TMF. This was confusing.
> 
> Therefore, refactor the TMF tracing to work without SCSI command
> and instead pass explicit arguments for SCSI ID and SCSI LUN.
> As context we now need a pointer to zfcp_adapter.
> To make it even clearer, we set all bits to 1 for the fields, which do
> not belong to the TMF, to indicate that these fields are invalid.
> 
> The old zfcp_dbf_scsi() became zfcp_dbf_scsi_common() to now handle both
> SCSI commands and TMFs. The old argument scsi_cmnd is now optional and
> can be NULL with TMFs. Two new arguments scsi_id and scsi_lun are
> optional and only used if scsi_cmnd is NULL, i.e. with TMFs.
> 
> Signed-off-by: Steffen Maier 
> ---
>  drivers/s390/scsi/zfcp_dbf.c  | 51 
> ---
>  drivers/s390/scsi/zfcp_dbf.h  | 26 +++---
>  drivers/s390/scsi/zfcp_ext.h  |  8 ---
>  drivers/s390/scsi/zfcp_scsi.c | 20 -
>  4 files changed, 71 insertions(+), 34 deletions(-)
> 
Reviewed-by: Hannes Reinecke 

Cheers,

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

[GIT PULL] SCSI fixes for 4.13-rc2

[James Bottomley]

Three small fixes.  The transfer size fixes are actually correcting
some performance drops on the hpsa and smartpqi cards.  The cards
actually have an internal cache for request speed up but bypass it for
transfers > 1MB.  Since 4.3 the efficiency of our merges has rendered
the cache mostly unused, so limit transfers to under 1MB to recover the
cache boost.

The patch is available here:

git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes

The short changelog is:

Johannes Thumshirn (1):
  scsi: sg: fix static checker warning in sg_is_valid_dxfer

Yadan Fan (2):
  scsi: smartpqi: limit transfer length to 1MB
  scsi: hpsa: limit transfer length to 1MB

And the diffstat:

 drivers/scsi/hpsa.c  | 2 +-
 drivers/scsi/sg.c| 7 +--
 drivers/scsi/smartpqi/smartpqi.h | 2 +-
 3 files changed, 7 insertions(+), 4 deletions(-)

With full diff below.

James

---

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 8914eab84337..4f7cdb28bd38 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -938,7 +938,7 @@ static struct scsi_host_template hpsa_driver_template = {
 #endif
.sdev_attrs = hpsa_sdev_attrs,
.shost_attrs = hpsa_shost_attrs,
-   .max_sectors = 8192,
+   .max_sectors = 1024,
.no_write_same = 1,
 };
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 1e82d4128a84..4fe606b000b4 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -759,8 +759,11 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
return false;
return true;
case SG_DXFER_FROM_DEV:
-   if (hp->dxfer_len < 0)
-   return false;
+   /*
+* for SG_DXFER_FROM_DEV we always set dxfer_len to > 0. dxferp
+* can either be NULL or != NULL so there's no point in checking
+* it either. So just return true.
+*/
return true;
case SG_DXFER_TO_DEV:
case SG_DXFER_TO_FROM_DEV:
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index 07ec8a8877de..e164ffade38a 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -690,7 +690,7 @@ struct pqi_config_table_heartbeat {
 
 #define PQI_MAX_OUTSTANDING_REQUESTS   ((u32)~0)
 #define PQI_MAX_OUTSTANDING_REQUESTS_KDUMP 32
-#define PQI_MAX_TRANSFER_SIZE  (4 * 1024U * 1024U)
+#define PQI_MAX_TRANSFER_SIZE  (1024U * 1024U)
 #define PQI_MAX_TRANSFER_SIZE_KDUMP(512 * 1024U)
 
 #define RAID_MAP_MAX_ENTRIES   1024

Re: [PATCH 2/2] scsi: aacraid: Off by one NUL terminator

[Bart Van Assche]

On Tue, 2017-07-25 at 22:51 +0300, Dan Carpenter wrote:
> We're putting a NUL terminator one character beyond the end of the
> struct and that's obviously wrong.  On the other hand, I'm not positive
> this is the correct fix.  This change was added deliberately and was
> mentioned in the changlog of commit b836439faf04 ("aacraid: 4KB sector
> support").  The relevant section is "Also fix up a name truncation
> problem".  Can someone review this code and figure out the right thing
> to do?
> 
> Fixes: b836439faf04 ("aacraid: 4KB sector support")
> Signed-off-by: Dan Carpenter 
> 
> diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
> index 4591113c49de..22c7461f65c9 100644
> --- a/drivers/scsi/aacraid/aachba.c
> +++ b/drivers/scsi/aacraid/aachba.c
> @@ -549,7 +549,7 @@ static void get_container_name_callback(void *context, 
> struct fib * fibptr)
>   if ((le32_to_cpu(get_name_reply->status) == CT_OK)
>&& (get_name_reply->data[0] != '\0')) {
>   char *sp = get_name_reply->data;
> - sp[sizeof(((struct aac_get_name_resp *)NULL)->data)] = '\0';
> + sp[sizeof(((struct aac_get_name_resp *)NULL)->data) - 1] = '\0';
>   while (*sp == ' ')
>   ++sp;
>   if (*sp) {

Hello Dan,

If others agree with the approach of this patch, please use FIELD_SIZEOF()
instead of leaving it open-coded.

Thanks,

Bart.

Re: Please help me with kernel hung

[Alan Stern]

On Wed, 26 Jul 2017, Михаил Гаврилов wrote:

> On 25 July 2017 at 02:12, Alan Stern  wrote:
> > This is enough for now.  The problem is that a USB transfer is started
> > but never stopped, which indicates a problem with the host controller.
> >
> > In this case you're using an xHCI host controller.  Unfortunately the
> > xHCI maintainer is on vacation, so he won't be able to help for a
> > while.
> >
> > In the meantime, you could try doing a git bisection to find which
> > commit caused the problem to occur.
> >
> > Alan Stern
> >
> 
> Sorry, I am afraid I can't bisect because i don't know how triggering
> this issue again.
> It means that I already several times rebooted after this and machine not 
> hung.
> 
> But still occurs another problem that reproducible more successful
> when I run KVM virtual machine and run inside kernel compilation then
> host hung without any records in journalctl log. It's really annoying
> because I don't know who is culprit here. Can here helps bisect?

I don't know.  Try it and see.

Alan Stern

Re: Please help me with kernel hung

[Михаил Гаврилов]

On 25 July 2017 at 02:12, Alan Stern  wrote:
> This is enough for now.  The problem is that a USB transfer is started
> but never stopped, which indicates a problem with the host controller.
>
> In this case you're using an xHCI host controller.  Unfortunately the
> xHCI maintainer is on vacation, so he won't be able to help for a
> while.
>
> In the meantime, you could try doing a git bisection to find which
> commit caused the problem to occur.
>
> Alan Stern
>

Sorry, I am afraid I can't bisect because i don't know how triggering
this issue again.
It means that I already several times rebooted after this and machine not hung.

But still occurs another problem that reproducible more successful
when I run KVM virtual machine and run inside kernel compilation then
host hung without any records in journalctl log. It's really annoying
because I don't know who is culprit here. Can here helps bisect?

[PATCH 4.12 042/196] scsi: virtio_scsi: let host do exception handling

[Greg Kroah-Hartman]

4.12-stable review patch.  If anyone has any objections, please let me know.

--

From: Paolo Bonzini 

commit e72c9a2a67a6400c8ef3d01d4c461dbbbfa0e1f0 upstream.

virtio_scsi tries to do exception handling after the default 30 seconds
timeout expires.  However, it's better to let the host control the
timeout, otherwise with a heavy I/O load it is likely that an abort will
also timeout.  This leads to fatal errors like filesystems going
offline.

Disable the 'sd' timeout and allow the host to do exception handling,
following the precedent of the storvsc driver.

Hannes has a proposal to introduce timeouts in virtio, but this provides
an immediate solution for stable kernels too.

[mkp: fixed typo]

Reported-by: Douglas Miller 
Cc: "James E.J. Bottomley" 
Cc: "Martin K. Petersen" 
Cc: Hannes Reinecke 
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Paolo Bonzini 
Signed-off-by: Martin K. Petersen 
Signed-off-by: Greg Kroah-Hartman 

---
 drivers/scsi/virtio_scsi.c |   12 
 1 file changed, 12 insertions(+)

--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -796,6 +796,16 @@ static int virtscsi_map_queues(struct Sc
return blk_mq_virtio_map_queues(>tag_set, vscsi->vdev, 2);
 }
 
+/*
+ * The host guarantees to respond to each command, although I/O
+ * latencies might be higher than on bare metal.  Reset the timer
+ * unconditionally to give the host a chance to perform EH.
+ */
+static enum blk_eh_timer_return virtscsi_eh_timed_out(struct scsi_cmnd *scmnd)
+{
+   return BLK_EH_RESET_TIMER;
+}
+
 static struct scsi_host_template virtscsi_host_template_single = {
.module = THIS_MODULE,
.name = "Virtio SCSI HBA",
@@ -806,6 +816,7 @@ static struct scsi_host_template virtscs
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+   .eh_timed_out = virtscsi_eh_timed_out,
.slave_alloc = virtscsi_device_alloc,
 
.can_queue = 1024,
@@ -826,6 +837,7 @@ static struct scsi_host_template virtscs
.change_queue_depth = virtscsi_change_queue_depth,
.eh_abort_handler = virtscsi_abort,
.eh_device_reset_handler = virtscsi_device_reset,
+   .eh_timed_out = virtscsi_eh_timed_out,
 
.can_queue = 1024,
.dma_boundary = UINT_MAX,

[PATCH 2/2] scsi: aacraid: Off by one NUL terminator

[Dan Carpenter]

We're putting a NUL terminator one character beyond the end of the
struct and that's obviously wrong.  On the other hand, I'm not positive
this is the correct fix.  This change was added deliberately and was
mentioned in the changlog of commit b836439faf04 ("aacraid: 4KB sector
support").  The relevant section is "Also fix up a name truncation
problem".  Can someone review this code and figure out the right thing
to do?

Fixes: b836439faf04 ("aacraid: 4KB sector support")
Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 4591113c49de..22c7461f65c9 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -549,7 +549,7 @@ static void get_container_name_callback(void *context, 
struct fib * fibptr)
if ((le32_to_cpu(get_name_reply->status) == CT_OK)
 && (get_name_reply->data[0] != '\0')) {
char *sp = get_name_reply->data;
-   sp[sizeof(((struct aac_get_name_resp *)NULL)->data)] = '\0';
+   sp[sizeof(((struct aac_get_name_resp *)NULL)->data) - 1] = '\0';
while (*sp == ' ')
++sp;
if (*sp) {

[PATCH 1/2] scsi: aacraid: reading out of bounds

[Dan Carpenter]

"qd.id" comes directly from the copy_from_user() on the line before so
we should verify that it's within bounds.

Signed-off-by: Dan Carpenter 
---
This bug predates git.

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c
index 707ee2f5954d..4591113c49de 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -3198,10 +3198,11 @@ static int query_disk(struct aac_dev *dev, void __user 
*arg)
return -EBUSY;
if (copy_from_user(, arg, sizeof (struct aac_query_disk)))
return -EFAULT;
-   if (qd.cnum == -1)
+   if (qd.cnum == -1) {
+   if (qd.id < 0 || qd.id >= dev->maximum_num_containers)
+   return -EINVAL;
qd.cnum = qd.id;
-   else if ((qd.bus == -1) && (qd.id == -1) && (qd.lun == -1))
-   {
+   } else if ((qd.bus == -1) && (qd.id == -1) && (qd.lun == -1)) {
if (qd.cnum < 0 || qd.cnum >= dev->maximum_num_containers)
return -EINVAL;
qd.instance = dev->scsi_host_ptr->host_no;

Re: [REGRESSION] 28676d869bbb (scsi: sg: check for valid direction before starting the request) breaks mtx tape library control

[Douglas Gilbert]

On 2017-07-19 04:36 AM, Johannes Thumshirn wrote:

On Wed, Jul 19, 2017 at 03:13:34AM -0500, Jason L Tibbitts III wrote:

[   46.304530] sg_is_valid_dxfer: dxfer_direction: -2, dxfer_len: 0


Ahh now I see the -2 (SG_DXFER_TO_DEV) is the crucial point here. It is 0 in
your case.

This would "fix" it but I'm not generally sure it is _the_ solution:

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 1e82d4128a84..b421ec81d775 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -764,7 +764,7 @@ static bool sg_is_valid_dxfer(sg_io_hdr_t *hp)
return true;
case SG_DXFER_TO_DEV:
case SG_DXFER_TO_FROM_DEV:
-   if (!hp->dxferp || hp->dxfer_len == 0)
+   if (!hp->dxferp)
return false;
return true;
case SG_DXFER_UNKNOWN:

Doug, what are the rules for SG_DXFER_TO_{FROM_}DEV? Apparently we can't
be sure len > 0, can we rely on dxferp being present?


_TO_ is toward the device (i.e. WRITE) and what T10 call "data-out".
_FROM_ is from the device (e.g. INQUIRY) and what T10 call "data-in".

_TO_FROM_ is basically _FROM_ (and has nothing to do with bidi).
_TO_FROM_ is very old and is meant to try and detect "short reads"
by prefilling the indirect buffer (by reading from dxferp) before
it is overwritten by the _FROM_ (and then writing to dxferp). It
is from the time when not all LLDs or HBAs provided an indication
of a "short read". Today users have the 'sg_io_hdr_t::resid' for
that purpose. Whether the sg driver in lk 4.12 still does that
I haven't checked.

The only limit that should be placed on dxfer_len is something like
<= 2**28 (256M) in my opinion. Big enough that the kernel would
reject it and small enough to catch negative values placed in an
unsigned.


The overall problem is that sg_is_valid_dxfer() introduced in lk 4.12
is doing more sanity checks than were done before. My policy was
to ignore ("don't care") combinations of dxfer_direction, dxferp and
dxfer_len that were harmless. Anyway those three variables are
incomplete since the SCSI command and the device also dictate the
length of the data-in transfer.

Doug Gilbert

Re: [REGRESSION] 28676d869bbb (scsi: sg: check for valid direction before starting the request) breaks mtx tape library control

[Jason L Tibbitts III]

> "JT" == Johannes Thumshirn  writes:

JT> Yes please (on top of the snippet I've sent you last).

OK, I'm at 4.12 with 68c59fcea1f2c6a54c62aa896cc623c1b5bc9b47 cherry
picked, plus the fix patch and the debug patch you've sent previously.
To make sure we're on the same page, I'll include the patch at the end.

Running "mtx -f /dev/sg7 status" gives proper output with this logged to
the console:

[   36.742905] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 56
[   36.750036] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 136
[   36.791673] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240
[   37.339790] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240
[   37.393597] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240

And running "mtx -f /dev/sg7 next 0" gives the following output:

[root@backup2 ~]# mtx -f /dev/sg7 next 0
Unloading drive 0 into Storage Element 1...mtx: Request Sense: Long
Report=yes
mtx: Request Sense: Valid Residual=no
mtx: Request Sense: Error Code=0 (Unknown?!)
mtx: Request Sense: Sense Key=No Sense
mtx: Request Sense: FileMark=no
mtx: Request Sense: EOM=no
mtx: Request Sense: ILI=no
mtx: Request Sense: Additional Sense Code = 00
mtx: Request Sense: Additional Sense Qualifier = 00
mtx: Request Sense: BPV=no
mtx: Request Sense: Error in CDB=no
mtx: Request Sense: SKSV=no
MOVE MEDIUM from Element Address 1 to 1001 Failed

And the following is logged to the console:

[  192.732294] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 56
[  192.739492] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 136
[  192.781507] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240
[  193.392401] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240
[  193.448970] sg_is_valid_dxfer: dxfer_direction: -3, dxfer_len: 4240
[  193.495130] sg_is_valid_dxfer: dxfer_direction: -2, dxfer_len: 0

That's not any different than what I provided before, and honestly I
wouldn't expect it to be.

Is there something else I can log or some debugging switch I can twiddle
to give you any more information?  I can also try to be more available
to try and avoid the timezone-induced day-long cycle time.  I'm
available on IRC (tibbs on freenode and oftc) and can try to stay up
late or get up early or something to try and avoid this time zone
mismatch.

Here's what an strace of the last mtx call says:

open("/dev/sg7", O_RDWR)= 3
ioctl(3, SG_GET_VERSION_NUM, [30536])   = 0
ioctl(3, SG_SET_TIMEOUT, [3])   = 0
brk(NULL)   = 0x55d65f68a000
brk(0x55d65f6ab000) = 0x55d65f6ab000
brk(NULL)   = 0x55d65f6ab000
ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, 
cmd_len=6, cmdp="\x12\x00\x00\x00\x38\x00", mx_sb_len=20, iovec_count=0, 
dxfer_len=56, timeout=3, flags=0, 
dxferp="\x08\x80\x05\x02\x45\x00\x00\x02\x42\x44\x54\x20\x20\x20\x20\x20\x46\x6c\x65\x78\x53\x74\x6f\x72\x20\x49\x49\x20\x20\x20\x20\x20"...,
 status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, 
driver_status=0, resid=0, duration=1, info=0}) = 0
ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, 
cmd_len=6, cmdp="\x1a\x08\x1d\x00\x88\x00", mx_sb_len=20, iovec_count=0, 
dxfer_len=136, timeout=30, flags=0, 
dxferp="\x17\x00\x00\x00\x9d\x12\x00\x00\x00\x01\x03\xe9\x00\x30\x00\x65\x00\x00\x00\x01\x00\x01\x00\x00",
 status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, 
driver_status=0, resid=112, duration=61, info=0}) = 0
ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0x7ffdbfaee7a0) = 0
ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, 
cmd_len=12, cmdp="\xb8\x32\x03\xe9\x00\x30\x00\x00\x10\x90\x00\x00", 
mx_sb_len=20, iovec_count=0, dxfer_len=4240, timeout=30, flags=0, 
dxferp="\x03\xe9\x00\x30\x00\x00\x09\xc8\x02\x80\x00\x34\x00\x00\x09\xc0\x03\xe9\x08\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00"...,
 status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, 
driver_status=0, resid=1728, duration=542, info=0}) = 0
ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0x7ffdbfaee7a0) = 0
ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, 
cmd_len=12, cmdp="\xb8\x34\x00\x01\x00\x01\x00\x00\x10\x90\x00\x00", 
mx_sb_len=20, iovec_count=0, dxfer_len=4240, timeout=30, flags=0, 
dxferp="\x00\x01\x00\x01\x00\x00\x00\x3c\x04\x80\x00\x34\x00\x00\x00\x34\x00\x01\x09\x00\x00\x00\x00\x00\x00\x81\x03\xe9\x43\x30\x30\x30"...,
 status=0, masked_status=0, msg_status=0, sb_len_wr=0, sbp="", host_status=0, 
driver_status=0, resid=4172, duration=47, info=0}) = 0
ioctl(3, CDROMAUDIOBUFSIZ or SCSI_IOCTL_GET_IDLUN, 0x7ffdbfaee7a0) = 0
ioctl(3, SG_IO, {interface_id='S', dxfer_direction=SG_DXFER_FROM_DEV, 
cmd_len=12, cmdp="\xb8\x31\x00\x00\x00\x01\x00\x00\x10\x90\x00\x00", 
mx_sb_len=20, iovec_count=0, dxfer_len=4240, timeout=30, flags=0, 

RE: [PATCH v2] scsi: ufs: Implement Auto-Hibern8 setup

[Potomski, MichalX]

> On Tue, 2017-07-25 at 11:45 +0200, Michal Potomski wrote:
> > Since Auto-Hibern8 feature has to be enabled by the user
> > proper API has been given via SysFS.
> >
> > We expose this API to user-space, since we don't know
> > in driver, what kind of additional Power Management rules
> > user wants to use. Due to that we want to expose API, to
> > let user or high level S/W decide, whether it wants to
> > use Auto-Hibern8 feature for Power Saving and give him
> > "slider" to decide between performance and power efficiency.
> > This is important because different platforms using
> > the same UFS host and/or device might want different
> > options on this one, e.g. High-End Desktop PC might
> > want to have this feature disabled, while Mobile or
> > Server platforms might want to have this feature enabled,
> > but levels will vary depending on what's to be acheived.
> >
> > As this feature is meant to be transparent for driver,
> > we'd like to let user decide whether he wants this enabled
> > or not.
> 
> Less than ten minutes after v1 of this patch was posted version v2 was
> posted. What is the difference between v1 and v2? Second and later versions
> of a patch or patch series are expected to include a changelog.
> 
V1 had some minor style issues, which were present in version that wasn't
targeted to be sent, hence the quick V2 post, which was the corrected one.

Proper changelog will be included in future versions.
> Since I'm not familiar with the Auto-Hibern8 feature: what impact does it
> have on command processing? Does it e.g. cause SCSI or TMF commands that
> are
> sent to the UFS device to be ignored, to fail or to time out?
> 
Actually behavior should be transparent for all of the higher layers, since this
shall be fully controlled by UFS Host, which will put UFS Device to Hibern8 
state,
when it has no ongoing commands to it for set up time. If there will be any 
command
UFS Host should order the Device to exit Hibern8 mode and proceed normally.

Bottom line is that in model case, it shouldn't cause any of errors mentioned 
by you.
There is possible throughput degradation in case, if transfers are sporadic in 
terms
of timer, which we did set up, though. According to specification it also 
shouldn't affect
Hibern8 states triggered by Power Management, nor any other functionality.

I will alter commit message in V3, to cover this question.
> > +static ssize_t ufshcd_auto_hibern8_show(struct device *dev,
> > +   struct device_attribute *attr, char *buf)
> > +{
> > +   struct ufs_hba *hba = dev_get_drvdata(dev);
> > +   u32 val;
> > +   unsigned long timer;
> > +   u8 scale;
> > +   char *unit;
> > +
> > +   val = ufshcd_read_auto_hibern8_state(hba);
> > +   timer = val & UFSHCI_AHIBERN8_TIMER_MASK;
> > +   scale = (val & UFSHCI_AHIBERN8_SCALE_MASK)
> > +   >> UFSHCI_AHIBERN8_SCALE_OFFSET;
> > +
> > +   unit = scale >= UFSHCI_AHIBERN8_SCALE_1MS ? "ms" : "us";
> > +
> > +   for (scale %= UFSHCI_AHIBERN8_SCALE_1MS; scale > 0; --scale)
> > +   timer *= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
> > +
> > +   return snprintf(buf, PAGE_SIZE, "%ld %s\n", timer, unit);
> > +}
> 
> From Documentation/filesystems/sysfs.txt: "All new sysfs attributes must be
> documented in Documentation/ABI. See also Documentation/ABI/README for
> more
> information." Please add proper documentation of the attributes added by this
> patch when you resubmit this patch.
> 
> Regarding this attribute: units should be documented in Documentation/ABI/*/*
> instead of specifying these in the attribute itself. In other words, the
> "ms" / "us" suffix should be left out.
> 
Will be fixed in V3
> > @@ -7693,16 +7766,34 @@ static void
> ufshcd_add_spm_lvl_sysfs_nodes(struct ufs_hba *hba)
> > dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
> >  }
> >
> > +static void ufshcd_add_ahibern8_sysfs_node(struct ufs_hba *hba)
> > +{
> > +   if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
> > +   return;
> > +
> > +   hba->ahibern8_attr.show = ufshcd_auto_hibern8_show;
> > +   hba->ahibern8_attr.store = ufshcd_auto_hibern8_store;
> > +   sysfs_attr_init(>ahibern8_attr.attr);
> > +   hba->ahibern8_attr.attr.name = "ufs_auto_hibern8";
> > +   hba->ahibern8_attr.attr.mode = 0600;
> > +   if (device_create_file(hba->dev, >ahibern8_attr))
> > +   dev_err(hba->dev, "Failed to create sysfs for
> ufs_auto_hibern8\n");
> > +}
> 
> Please use a static variable at file scope and DRIVER_ATTR_RW() to initialize
> the sysfs attributes instead of initializing the sysfs attributes explicitly.
> 
> Additionally, please set sdev_attrs in struct scsi_host_template instead of
> calling device_create_file() explicitly. The above device_create_file() call
> occurs after a UFS device has been made visible in sysfs and hence will cause
> trouble (race condition) if a udev rule tries to set this attribute from 
> inside
> a rule that is triggered by device creation.
> 
> I am aware the above code 

Re: [PATCH] scsi: megaraid: fix ifnullfree.cocci warnings

[James Bottomley]

On Tue, 2017-07-25 at 23:40 +0800, kbuild test robot wrote:
> drivers/scsi/megaraid/megaraid_sas_fusion.c:608:2-18: WARNING: NULL
> check before freeing functions like kfree, debugfs_remove,
> debugfs_remove_recursive or usb_free_urb is not needed. Maybe
> consider reorganizing relevant code to avoid passing NULL values.
> drivers/scsi/megaraid/megaraid_sas_fusion.c:629:2-18: WARNING: NULL
> check before freeing functions like kfree, debugfs_remove,
> debugfs_remove_recursive or usb_free_urb is not needed. Maybe
> consider reorganizing relevant code to avoid passing NULL values.
> 
>  NULL check before some freeing functions is not needed.

Hey, guys, we yelled at Markus Elfring for sending hundreds of patches
like this.  The reason then was too much code churn for too little
value: the if() might be unnecessary but it really doesn't cost very
much in terms of execution time, so the code churn disadvantage greatly
 outweighs any benefits.

The reason hasn't changed, so please stop this otherwise you're going
to turn the very valuable 0day test infrastructure into a thousand
patch spambot.

James

[PATCH] scsi: megaraid: fix ifnullfree.cocci warnings

[kbuild test robot]

drivers/scsi/megaraid/megaraid_mbox.c:1256:2-18: WARNING: NULL check before 
freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or 
usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid 
passing NULL values.
drivers/scsi/megaraid/megaraid_mbox.c:1265:2-18: WARNING: NULL check before 
freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or 
usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid 
passing NULL values.
drivers/scsi/megaraid/megaraid_mbox.c:1274:2-18: WARNING: NULL check before 
freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or 
usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid 
passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

Fixes: cbb80044a6c7 ("scsi: megaraid: Replace PCI pool old API")
CC: Romain Perier 
Signed-off-by: Fengguang Wu 
---

 megaraid_mbox.c |9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

--- a/drivers/scsi/megaraid/megaraid_mbox.c
+++ b/drivers/scsi/megaraid/megaraid_mbox.c
@@ -1252,8 +1252,7 @@ megaraid_mbox_teardown_dma_pools(adapter
dma_pool_free(raid_dev->sg_pool_handle, sg_pci_blk[i].vaddr,
sg_pci_blk[i].dma_addr);
}
-   if (raid_dev->sg_pool_handle)
-   dma_pool_destroy(raid_dev->sg_pool_handle);
+   dma_pool_destroy(raid_dev->sg_pool_handle);
 
 
epthru_pci_blk = raid_dev->epthru_pool;
@@ -1261,8 +1260,7 @@ megaraid_mbox_teardown_dma_pools(adapter
dma_pool_free(raid_dev->epthru_pool_handle,
epthru_pci_blk[i].vaddr, epthru_pci_blk[i].dma_addr);
}
-   if (raid_dev->epthru_pool_handle)
-   dma_pool_destroy(raid_dev->epthru_pool_handle);
+   dma_pool_destroy(raid_dev->epthru_pool_handle);
 
 
mbox_pci_blk = raid_dev->mbox_pool;
@@ -1270,8 +1268,7 @@ megaraid_mbox_teardown_dma_pools(adapter
dma_pool_free(raid_dev->mbox_pool_handle,
mbox_pci_blk[i].vaddr, mbox_pci_blk[i].dma_addr);
}
-   if (raid_dev->mbox_pool_handle)
-   dma_pool_destroy(raid_dev->mbox_pool_handle);
+   dma_pool_destroy(raid_dev->mbox_pool_handle);
 
return;
 }

[PATCH] scsi: megaraid: fix ifnullfree.cocci warnings

[kbuild test robot]

drivers/scsi/megaraid/megaraid_mm.c:1020:2-18: WARNING: NULL check before 
freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or 
usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid 
passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

Fixes: cbb80044a6c7 ("scsi: megaraid: Replace PCI pool old API")
CC: Romain Perier 
Signed-off-by: Fengguang Wu 
---

 megaraid_mm.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/drivers/scsi/megaraid/megaraid_mm.c
+++ b/drivers/scsi/megaraid/megaraid_mm.c
@@ -1016,8 +1016,7 @@ memalloc_error:
kfree(adapter->kioc_list);
kfree(adapter->mbox_list);
 
-   if (adapter->pthru_dma_pool)
-   dma_pool_destroy(adapter->pthru_dma_pool);
+   dma_pool_destroy(adapter->pthru_dma_pool);
 
kfree(adapter);
 

[PATCH] scsi: megaraid: fix ifnullfree.cocci warnings

[kbuild test robot]

drivers/scsi/megaraid/megaraid_sas_fusion.c:608:2-18: WARNING: NULL check 
before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive 
or usb_free_urb is not needed. Maybe consider reorganizing relevant code to 
avoid passing NULL values.
drivers/scsi/megaraid/megaraid_sas_fusion.c:629:2-18: WARNING: NULL check 
before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive 
or usb_free_urb is not needed. Maybe consider reorganizing relevant code to 
avoid passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

Fixes: cbb80044a6c7 ("scsi: megaraid: Replace PCI pool old API")
CC: Romain Perier 
Signed-off-by: Fengguang Wu 
---

 megaraid_sas_fusion.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/drivers/scsi/megaraid/megaraid_sas_fusion.c
+++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c
@@ -604,8 +604,7 @@ megasas_free_rdpq_fusion(struct megasas_
fusion->reply_frames_desc_phys[i]);
}
 
-   if (fusion->reply_frames_desc_pool)
-   dma_pool_destroy(fusion->reply_frames_desc_pool);
+   dma_pool_destroy(fusion->reply_frames_desc_pool);
 
if (fusion->rdpq_virt)
pci_free_consistent(instance->pdev,
@@ -625,8 +624,7 @@ megasas_free_reply_fusion(struct megasas
fusion->reply_frames_desc[0],
fusion->reply_frames_desc_phys[0]);
 
-   if (fusion->reply_frames_desc_pool)
-   dma_pool_destroy(fusion->reply_frames_desc_pool);
+   dma_pool_destroy(fusion->reply_frames_desc_pool);
 
 }
 

Re: [PATCH v2] scsi: ufs: Implement Auto-Hibern8 setup

[Bart Van Assche]

On Tue, 2017-07-25 at 11:45 +0200, Michal Potomski wrote:
> Since Auto-Hibern8 feature has to be enabled by the user
> proper API has been given via SysFS.
> 
> We expose this API to user-space, since we don't know
> in driver, what kind of additional Power Management rules
> user wants to use. Due to that we want to expose API, to
> let user or high level S/W decide, whether it wants to
> use Auto-Hibern8 feature for Power Saving and give him
> "slider" to decide between performance and power efficiency.
> This is important because different platforms using
> the same UFS host and/or device might want different
> options on this one, e.g. High-End Desktop PC might
> want to have this feature disabled, while Mobile or
> Server platforms might want to have this feature enabled,
> but levels will vary depending on what's to be acheived.
> 
> As this feature is meant to be transparent for driver,
> we'd like to let user decide whether he wants this enabled
> or not.

Less than ten minutes after v1 of this patch was posted version v2 was
posted. What is the difference between v1 and v2? Second and later versions
of a patch or patch series are expected to include a changelog.

Since I'm not familiar with the Auto-Hibern8 feature: what impact does it
have on command processing? Does it e.g. cause SCSI or TMF commands that are
sent to the UFS device to be ignored, to fail or to time out?

> +static ssize_t ufshcd_auto_hibern8_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ufs_hba *hba = dev_get_drvdata(dev);
> + u32 val;
> + unsigned long timer;
> + u8 scale;
> + char *unit;
> +
> + val = ufshcd_read_auto_hibern8_state(hba);
> + timer = val & UFSHCI_AHIBERN8_TIMER_MASK;
> + scale = (val & UFSHCI_AHIBERN8_SCALE_MASK)
> + >> UFSHCI_AHIBERN8_SCALE_OFFSET;
> +
> + unit = scale >= UFSHCI_AHIBERN8_SCALE_1MS ? "ms" : "us";
> +
> + for (scale %= UFSHCI_AHIBERN8_SCALE_1MS; scale > 0; --scale)
> + timer *= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
> +
> + return snprintf(buf, PAGE_SIZE, "%ld %s\n", timer, unit);
> +}

>From Documentation/filesystems/sysfs.txt: "All new sysfs attributes must be
documented in Documentation/ABI. See also Documentation/ABI/README for more
information." Please add proper documentation of the attributes added by this
patch when you resubmit this patch.

Regarding this attribute: units should be documented in Documentation/ABI/*/*
instead of specifying these in the attribute itself. In other words, the
"ms" / "us" suffix should be left out.

> @@ -7693,16 +7766,34 @@ static void ufshcd_add_spm_lvl_sysfs_nodes(struct 
> ufs_hba *hba)
>   dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
>  }
>  
> +static void ufshcd_add_ahibern8_sysfs_node(struct ufs_hba *hba)
> +{
> + if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
> + return;
> +
> + hba->ahibern8_attr.show = ufshcd_auto_hibern8_show;
> + hba->ahibern8_attr.store = ufshcd_auto_hibern8_store;
> + sysfs_attr_init(>ahibern8_attr.attr);
> + hba->ahibern8_attr.attr.name = "ufs_auto_hibern8";
> + hba->ahibern8_attr.attr.mode = 0600;
> + if (device_create_file(hba->dev, >ahibern8_attr))
> + dev_err(hba->dev, "Failed to create sysfs for 
> ufs_auto_hibern8\n");
> +}

Please use a static variable at file scope and DRIVER_ATTR_RW() to initialize
the sysfs attributes instead of initializing the sysfs attributes explicitly.

Additionally, please set sdev_attrs in struct scsi_host_template instead of
calling device_create_file() explicitly. The above device_create_file() call
occurs after a UFS device has been made visible in sysfs and hence will cause
trouble (race condition) if a udev rule tries to set this attribute from inside
a rule that is triggered by device creation.

I am aware the above code follows the style that is used for other UFS sysfs
attributes. If you would have the time to convert the code for creating the
existing UFS sysfs attributes that would be great.

Thanks,

Bart.

Re: [PATCH v2] scsi: qedf: Limit number of CQs

[Chad Dupuis]

On Tue, 25 Jul 2017, 5:19am, Thomas Bogendoerfer wrote:

> From: Thomas Bogendoerfer 
> 
> FCOE offloading failed with:
> 
> [qed_sp_fcoe_func_start:150(sp-0-3b:00.02)]Cannot satisfy CQ amount. CQs
>requested 8, CQs available 6. Aborting function start
> [qed_fcoe_start:821()]Failed to start fcoe
> [__qedf_probe:3041]:6: Cannot start FCoE function.
> 
> The reason is a newly introduced check in the qed main part. This change
> also provides the information about how many CQs are available, so we
> simply limit the number of requested CQs..
> 
> Fixes: 3c5da9427802 ("qed: Share additional information with qedf")
> Signed-off-by: Thomas Bogendoerfer 
> ---
> 
> Changes in v2:
> - integrated suggested change from Chad Dupuis
> 
>  drivers/scsi/qedf/qedf.h  |  3 ++-
>  drivers/scsi/qedf/qedf_main.c | 20 +---
>  2 files changed, 11 insertions(+), 12 deletions(-)
> 

V2 looks good.

Acked-by: Chad Dupuis 

Re: [PATCH v2] scsi: qedf: Limit number of CQs

[Johannes Thumshirn]

Looks good,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH 32/47] scsi: Use scsi_target as argument for eh_target_reset_handler()

[Hannes Reinecke]

On 07/24/2017 08:10 PM, Steffen Maier wrote:
> 
> On 06/28/2017 10:32 AM, Hannes Reinecke wrote:
>> The target reset function should only depend on the scsi target,
>> not the scsi command.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
> 
>>   drivers/s390/scsi/zfcp_scsi.c   | 20 ++--
> 
>>   drivers/scsi/scsi_debug.c   | 21 -
>>   drivers/scsi/scsi_error.c   |  5 +--
> 
>>   include/scsi/scsi_host.h|  2 +-
> 
>>   33 files changed, 214 insertions(+), 174 deletions(-)
> 
>> diff --git a/drivers/s390/scsi/zfcp_scsi.c
>> b/drivers/s390/scsi/zfcp_scsi.c
>> index dd7bea0..92a3902 100644
>> --- a/drivers/s390/scsi/zfcp_scsi.c
>> +++ b/drivers/s390/scsi/zfcp_scsi.c
>> @@ -309,9 +309,25 @@ static int
>> zfcp_scsi_eh_device_reset_handler(struct scsi_cmnd *scpnt)
>>   return zfcp_task_mgmt_function(scpnt->device, FCP_TMF_LUN_RESET);
>>   }
>>
>> -static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
>> +/*
>> + * Note: We need to select a LUN as the storage array doesn't
>> + * necessarily supports LUN 0 and might refuse the target reset.
>> + */
> 
> Do you have any real experience with targets regarding this?
> 
> Did you even try this and it failed?
> If so, how did it fail?
> 
Hehe.

Actually, it was _you_ (well, not you personally, but the zfcp
maintainer at that time) who insisted on _not_ having to rely on LUN 0,
as that LUN might not be available on non-NPIV setups.
In the same vein he argued that we should be using the WLUN here.

> It seems other drivers hardcode LUN 0 for target reset [see below].
> 
> At least you made a similar loop to search for a suitable "victim"
> scsi_device with some other driver changes below, so zfcp is not the
> only one.
> 
> In fact, this is one of my open questions in my own patch set:
> Is the TMF flag in the FCP_CMND IU sufficient or does the transmission
> path require a valid FCP_LUN also in the same IU even for a target reset.
> 
Technically, you need an IT nexus for the target reset.
As the SCSI target is somewhat under-represented in the linux SCSI stack
typically it's easier to use a scsi device for this, and derive the IT
nexus from there.
And target reset is a tad tricky anyway; it got deprecated with later
SCSI releases (SPC-3?), so chances is that it doesn't do anything.

(You could do yourself a favour and enquire with your friendly array
vendors if _they_ support target reset; I have a strong feeling that
they don't. In which case you might as well drop it completely, and
target reset doing an IT nexus reset.)

>> +static int zfcp_scsi_eh_target_reset_handler(struct scsi_target
>> *starget)
>>   {
>> -return zfcp_task_mgmt_function(scpnt->device, FCP_TMF_TGT_RESET);
>> +struct fc_rport *rport = starget_to_rport(starget);
>> +struct Scsi_Host *shost = rport_to_shost(rport);
>> +struct scsi_device *sdev = NULL, *tmp_sdev;
>> +
> 
> In "[PATCH 09/47] zfcp: open-code fc_block_scsi_eh() for host reset" you
> introduced a shost lock, but here you did not?
> 
> Does the midlayer already hold an shost lock when calling any of these
> eh callbacks?
> 
Yes.

> Not sure if that's the corresponding part of
> Documentation/scsi/scsi_eh.txt (but even if, I don't understand who's
> supposed to have the shost lock):
>> [2-1-2] Flow of scmds through EH
>>  2. EH starts
>> ACTION: move all scmds to EH's local eh_work_q.  shost->eh_cmd_q
>> is cleared.
>> LOCKING: shost->host_lock (not strictly necessary, just for
>>  consistency)
>> [2-2-3] Things to consider
>>  - For consistency, when accessing/modifying shost data structure,
>>grab shost->host_lock.
> 
> 
>> +shost_for_each_device(tmp_sdev, shost) {
>> +if (tmp_sdev->id == starget->id) {
>> +sdev = tmp_sdev;
>> +break;
>> +}
>> +}
>> +if (!sdev)
>> +return FAILED;
>> +return zfcp_task_mgmt_function(sdev, FCP_TMF_TGT_RESET);
>>   }
> 
> Ah, this "solves" the problem of needing a scsi_device even though we
> only get scsi_target as scope argument.
> 
>> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c
>> b/drivers/scsi/lpfc/lpfc_scsi.c
>> index 107c0f6..573bd43 100644
>> --- a/drivers/scsi/lpfc/lpfc_scsi.c
>> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
>> @@ -5226,16 +5226,16 @@ void lpfc_poll_timeout(unsigned long ptr)
>>*  0x2002 - Success
>>**/
>>   static int
>> -lpfc_target_reset_handler(struct scsi_cmnd *cmnd)
>> +lpfc_target_reset_handler(struct scsi_target *starget)
>>   {
>> -struct Scsi_Host  *shost = cmnd->device->host;
>> +struct fc_rport *rport = starget_to_rport(starget);
>> +struct Scsi_Host  *shost = rport_to_shost(rport);
>>   struct lpfc_vport *vport = (struct lpfc_vport *) shost->hostdata;
>>   struct lpfc_rport_data *rdata;
>>   struct lpfc_nodelist *pnode;
>> -unsigned tgt_id = cmnd->device->id;
>> -uint64_t lun_id = cmnd->device->lun;
>> +unsigned 

[RFC 7/9] zfcp: use fc_block_rport for TMFs and host reset to decouple from scsi_cmnd

[Steffen Maier]

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_scsi.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 05c823ccb959..8e96196fa877 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -287,7 +287,7 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, 
u8 tm_flags)
break;
 
zfcp_erp_wait(adapter);
-   ret = fc_block_scsi_eh(scpnt);
+   ret = port->rport ? fc_block_rport(port->rport) : 0;
if (ret) {
zfcp_dbf_scsi_devreset("fiof", adapter, tm_flags, NULL,
   scsi_id, scsi_lun);
@@ -337,11 +337,13 @@ static int zfcp_scsi_eh_host_reset_handler(struct 
scsi_cmnd *scpnt)
 {
struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
+   struct zfcp_port *port;
int ret;
 
zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
zfcp_erp_wait(adapter);
-   ret = fc_block_scsi_eh(scpnt);
+   port = zfcp_sdev->port;
+   ret = port->rport ? fc_block_rport(port->rport) : 0;
if (ret)
return ret;
 
-- 
2.11.2

[RFC 9/9] zfcp: decouple our scsi_eh callbacks from scsi_cmnd

[Steffen Maier]

zfcp_scsi_eh_device_reset_handler() now only depends on scsi_device.
zfcp_scsi_eh_target_reset_handler() now only depends on scsi_target.
zfcp_scsi_eh_host_reset_handler() now only depends on Scsi_Host.
All derive other objects from these intended callback arguments.

Actually change the signature of zfcp_task_mgmt_function() used by
zfcp_scsi_eh_device_reset_handler() & zfcp_scsi_eh_target_reset_handler().
Since it was prepared in a previous patch, we only need to delete
some local auto variables which are now the intended arguments.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_scsi.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 11cf33ea8c14..4cb38cfd46e3 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -266,11 +266,17 @@ static void zfcp_scsi_forget_cmnds(struct zfcp_port *port,
write_unlock_irqrestore(>abort_lock, flags);
 }
 
-static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
+/**
+ * zfcp_task_mgmt_function() - Synchronously send a task management function.
+ * @port: Pointer to zfcp port indicating scope.
+ * @sdev: Pointer to SCSI device as scope, or %NULL if scope is only port.
+ * @tm_flags: Task management flags,
+ *here we only handle %FCP_TMF_TGT_RESET or %FCP_TMF_LUN_RESET.
+ */
+static int zfcp_task_mgmt_function(struct zfcp_port *port,
+  struct scsi_device *sdev, u8 tm_flags)
 {
-   struct scsi_device *sdev = scpnt->device;
-   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
-   struct zfcp_port *port = zfcp_sdev->port;
+   struct zfcp_scsi_dev *zfcp_sdev = sdev ? sdev_to_zfcp(sdev) : NULL;
struct zfcp_adapter *adapter = port->adapter;
unsigned int scsi_id = port->starget_id;
u64 scsi_lun = ZFCP_DBF_INVALID_LUN;
@@ -325,18 +331,36 @@ static int zfcp_task_mgmt_function(struct scsi_cmnd 
*scpnt, u8 tm_flags)
 
 static int zfcp_scsi_eh_device_reset_handler(struct scsi_cmnd *scpnt)
 {
-   return zfcp_task_mgmt_function(scpnt, FCP_TMF_LUN_RESET);
+   struct scsi_device *sdev = scpnt->device;
+   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
+   struct zfcp_port *port = zfcp_sdev->port;
+
+   return zfcp_task_mgmt_function(port, sdev, FCP_TMF_LUN_RESET);
 }
 
 static int zfcp_scsi_eh_target_reset_handler(struct scsi_cmnd *scpnt)
 {
-   return zfcp_task_mgmt_function(scpnt, FCP_TMF_TGT_RESET);
+   struct scsi_target *starget = scsi_target(scpnt->device);
+   struct fc_rport *rport = starget_to_rport(starget);
+   struct zfcp_adapter *adapter =
+   (struct zfcp_adapter *)rport_to_shost(rport)->hostdata[0];
+   struct zfcp_port *port = zfcp_get_port_by_wwpn(adapter,
+  rport->port_name);
+   if (!port) {
+   zfcp_dbf_scsi_devreset("nopt", adapter, FCP_TMF_TGT_RESET,
+  NULL, port->starget_id,
+  ZFCP_DBF_INVALID_LUN);
+   return 0;
+   }
+
+   return zfcp_task_mgmt_function(port, NULL, FCP_TMF_TGT_RESET);
 }
 
 static int zfcp_scsi_eh_host_reset_handler(struct scsi_cmnd *scpnt)
 {
-   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
-   struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
+   struct Scsi_Host *shost = scpnt->device->host;
+   struct zfcp_adapter *adapter =
+   (struct zfcp_adapter *)shost->hostdata[0];
struct zfcp_port *port;
int ret = SUCCESS;
 
-- 
2.11.2

[RFC 8/9] zfcp: fix waiting for rport(s) unblock in eh_host_reset_handler

[Steffen Maier]

v2.6.30 commit 63caf367e1c9 ("[SCSI] zfcp: Improve reliability of SCSI eh
handlers in zfcp") added calls to zfcp_erp_wait() within
eh_abort_handler(), eh_device_reset_handler(), eh_target_reset_handler()
in order to synchronize with zfcp recovery completion before returning
from a scsi_eh callback (e.g. with SUCCESS) to prevent eh escalation.

v2.6.33 commit af4de36d911a ("[SCSI] zfcp: Block scsi_eh thread for rport
state BLOCKED") introduced the use of fc_block_scsi_eh() for
eh_abort_handler(), eh_device_reset_handler(), eh_target_reset_handler(),
and eh_host_reset_handler(), because zfcp_erp_wait() from above commit is
not sufficient.
The use in zfcp_task_mgmt_function() is correct even for a LUN reset,
as described in commit 6f2ce1c6af37 ("scsi: zfcp: fix rport unblock race
with LUN recovery").
However, the one call in zfcp_scsi_eh_host_reset_handler() waiting for
just one arbitrary port of the arbitrary scsi_cmnd seems insufficient
as the preceding adapter recovery could have recovered multiple ports
for which we all should wait to unblock (or have run into FAST_IO_FAIL).

Therefore, we now wait for all ports of the adapter with this fix.

NB: We cannot easily wait for an event because there is a time window
between zfcp_erp_wait() returned and zfcp_erp_try_rport_unblock() as part
of zfcp_erp_action_cleanup() actually scheduled rport_work which will
unblock an rport in zfcp_scsi_rport_work() asynchronously. Hence a
flush_work() could come early before queue_work() was even done.

v2.6.35 commit a1dbfddd02d2 ("[SCSI] zfcp: Pass return code from
fc_block_scsi_eh to scsi eh") fixed v2.6.33 for the FAST_IO_FAIL case.

Signed-off-by: Steffen Maier 
Fixes: af4de36d911a ("[SCSI] zfcp: Block scsi_eh thread for rport state 
BLOCKED")
Fixes: a1dbfddd02d2 ("[SCSI] zfcp: Pass return code from fc_block_scsi_eh to 
scsi eh")
---
 drivers/s390/scsi/zfcp_scsi.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index 8e96196fa877..11cf33ea8c14 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -338,16 +338,29 @@ static int zfcp_scsi_eh_host_reset_handler(struct 
scsi_cmnd *scpnt)
struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
struct zfcp_port *port;
-   int ret;
+   int ret = SUCCESS;
 
zfcp_erp_adapter_reopen(adapter, 0, "schrh_1");
zfcp_erp_wait(adapter);
-   port = zfcp_sdev->port;
-   ret = port->rport ? fc_block_rport(port->rport) : 0;
-   if (ret)
-   return ret;
+   /* after internal recovery, wait for async unblock of rport(s) */
+   read_lock(>port_list_lock);
+   list_for_each_entry(port, >port_list, list) {
+   int fc_ret;
+
+   if (!port->rport)
+   continue;
+
+   fc_ret = fc_block_rport(port->rport);
+   /* Any rport ran into fast_io_fail_tmo: FAST_IO_FAIL.
+* To let pending requests bubble up, even if too many
+* because of other rports without this timeout.
+*/
+   if (fc_ret)
+   ret = fc_ret;
+   }
+   read_unlock(>port_list_lock);
 
-   return SUCCESS;
+   return ret;
 }
 
 struct scsi_transport_template *zfcp_scsi_transport_template;
-- 
2.11.2

[RFC 3/9] zfcp: split FCP_CMND IU setup between SCSI I/O and TMF again

[Steffen Maier]

This reverts commit 2443c8b23aea ("[SCSI] zfcp: Merge FCP task management
setup with regular FCP command setup"), because this introduced a
dependency on the unsuitable SCSI command for scsi_eh / TMF.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_fc.h  | 22 ++
 drivers/s390/scsi/zfcp_fsf.c |  4 ++--
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
index 41f22d3dc6d1..24949868d027 100644
--- a/drivers/s390/scsi/zfcp_fc.h
+++ b/drivers/s390/scsi/zfcp_fc.h
@@ -206,21 +206,14 @@ struct zfcp_fc_wka_ports {
  * zfcp_fc_scsi_to_fcp - setup FCP command with data from scsi_cmnd
  * @fcp: fcp_cmnd to setup
  * @scsi: scsi_cmnd where to get LUN, task attributes/flags and CDB
- * @tm: task management flags to setup task management command
  */
 static inline
-void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi,
-u8 tm_flags)
+void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi)
 {
u32 datalen;
 
int_to_scsilun(scsi->device->lun, (struct scsi_lun *) >fc_lun);
 
-   if (unlikely(tm_flags)) {
-   fcp->fc_tm_flags = tm_flags;
-   return;
-   }
-
fcp->fc_pri_ta = FCP_PTA_SIMPLE;
 
if (scsi->sc_data_direction == DMA_FROM_DEVICE)
@@ -240,6 +233,19 @@ void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct 
scsi_cmnd *scsi,
 }
 
 /**
+ * zfcp_fc_fcp_tm() - Setup FCP command as task management command.
+ * @fcp: Pointer to FCP_CMND IU to set up.
+ * @dev: Pointer to SCSI_device where to send the task management command.
+ * @tm_flags: Task management flags to setup tm command.
+ */
+static inline
+void zfcp_fc_fcp_tm(struct fcp_cmnd *fcp, struct scsi_device *dev, u8 tm_flags)
+{
+   int_to_scsilun(dev->lun, (struct scsi_lun *) >fc_lun);
+   fcp->fc_tm_flags = tm_flags;
+}
+
+/**
  * zfcp_fc_evap_fcp_rsp - evaluate FCP RSP IU and update scsi_cmnd accordingly
  * @fcp_rsp: FCP RSP IU to evaluate
  * @scsi: SCSI command where to update status and sense buffer
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 8b2b2ea552d6..f221a34c26df 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2265,7 +2265,7 @@ int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *scsi_cmnd)
 
BUILD_BUG_ON(sizeof(struct fcp_cmnd) > FSF_FCP_CMND_SIZE);
fcp_cmnd = >qtcb->bottom.io.fcp_cmnd.iu;
-   zfcp_fc_scsi_to_fcp(fcp_cmnd, scsi_cmnd, 0);
+   zfcp_fc_scsi_to_fcp(fcp_cmnd, scsi_cmnd);
 
if ((scsi_get_prot_op(scsi_cmnd) != SCSI_PROT_NORMAL) &&
scsi_prot_sg_count(scsi_cmnd)) {
@@ -2371,7 +2371,7 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
scsi_cmnd *scmnd,
 
zfcp_qdio_set_sbale_last(qdio, >qdio_req);
 
-   zfcp_fc_scsi_to_fcp(fcp_cmnd, scmnd, tm_flags);
+   zfcp_fc_fcp_tm(fcp_cmnd, scmnd->device, tm_flags);
 
zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT);
if (!zfcp_fsf_req_send(req))
-- 
2.11.2

[RFC 6/9] scsi: fc: start decoupling fc_block_scsi_eh from scsi_cmnd

[Steffen Maier]

Scsi_cmnd is an unsuitable argument for eh_device_reset_handler(),
eh_target_reset_handler(), and eh_host_reset_handler()
which do not have the scope of one single SCSI command.
These callbacks tend to use fc_block_scsi_eh() requiring scsi_cmnd.
In order to start decoupling above eh callbacks from scsi_cmnd,
introduce a new variant of the function called fc_block_rport()
taking an fc_rport as argument.
Refactor the old fc_block_scsi_eh() to simply delegate to fc_block_rport().

Signed-off-by: Steffen Maier 
---
 drivers/scsi/scsi_transport_fc.c | 31 ++-
 include/scsi/scsi_transport_fc.h |  1 +
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index d4cf32d55546..3594043834c7 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3272,8 +3272,8 @@ fc_scsi_scan_rport(struct work_struct *work)
 }
 
 /**
- * fc_block_scsi_eh - Block SCSI eh thread for blocked fc_rport
- * @cmnd: SCSI command that scsi_eh is trying to recover
+ * fc_block_rport() - Block SCSI eh thread for blocked fc_rport.
+ * @rport: Remote port that scsi_eh is trying to recover.
  *
  * This routine can be called from a FC LLD scsi_eh callback. It
  * blocks the scsi_eh thread until the fc_rport leaves the
@@ -3285,10 +3285,9 @@ fc_scsi_scan_rport(struct work_struct *work)
  * FAST_IO_FAIL if the fast_io_fail_tmo fired, this should be
  * passed back to scsi_eh.
  */
-int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
+int fc_block_rport(struct fc_rport *rport)
 {
-   struct Scsi_Host *shost = cmnd->device->host;
-   struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
+   struct Scsi_Host *shost = rport_to_shost(rport);
unsigned long flags;
 
spin_lock_irqsave(shost->host_lock, flags);
@@ -3305,6 +3304,28 @@ int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
 
return 0;
 }
+EXPORT_SYMBOL(fc_block_rport);
+
+/**
+ * fc_block_scsi_eh - Block SCSI eh thread for blocked fc_rport
+ * @cmnd: SCSI command that scsi_eh is trying to recover
+ *
+ * This routine can be called from a FC LLD scsi_eh callback. It
+ * blocks the scsi_eh thread until the fc_rport leaves the
+ * FC_PORTSTATE_BLOCKED, or the fast_io_fail_tmo fires. This is
+ * necessary to avoid the scsi_eh failing recovery actions for blocked
+ * rports which would lead to offlined SCSI devices.
+ *
+ * Returns: 0 if the fc_rport left the state FC_PORTSTATE_BLOCKED.
+ * FAST_IO_FAIL if the fast_io_fail_tmo fired, this should be
+ * passed back to scsi_eh.
+ */
+int fc_block_scsi_eh(struct scsi_cmnd *cmnd)
+{
+   struct fc_rport *rport = starget_to_rport(scsi_target(cmnd->device));
+
+   return fc_block_rport(rport);
+}
 EXPORT_SYMBOL(fc_block_scsi_eh);
 
 /**
diff --git a/include/scsi/scsi_transport_fc.h b/include/scsi/scsi_transport_fc.h
index 6e208bb32c78..d8cae7bd8161 100644
--- a/include/scsi/scsi_transport_fc.h
+++ b/include/scsi/scsi_transport_fc.h
@@ -808,6 +808,7 @@ void fc_host_post_vendor_event(struct Scsi_Host *shost, u32 
event_number,
 struct fc_vport *fc_vport_create(struct Scsi_Host *shost, int channel,
struct fc_vport_identifiers *);
 int fc_vport_terminate(struct fc_vport *vport);
+int fc_block_rport(struct fc_rport *rport);
 int fc_block_scsi_eh(struct scsi_cmnd *cmnd);
 enum blk_eh_timer_return fc_eh_timed_out(struct scsi_cmnd *scmd);
 
-- 
2.11.2

[RFC 4/9] zfcp: decouple FSF request setup of TMF from scsi_cmnd

[Steffen Maier]

The scsi_device argument of zfcp_fc_fcp_tm() can now be NULL.

In zfcp_fsf_fcp_task_mgmt() resolve the still old argument scsi_cmnd
into scsi_device very early and only depend on scsi_device and derived
objects in the function body.

Scsi_device and derived zfcp_scsi_dev can later be NULL for the
target reset case, so do not depend on them unconditionally.
For the generic case, rather change to using zfcp_port directly.

This prepares to later change the function signature replacing the
scsi_cmnd argument with zfcp_port and an
optional scsi_device which can be NULL.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_fc.h  |  6 --
 drivers/s390/scsi/zfcp_fsf.c | 25 +
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
index 24949868d027..0e5b01c33873 100644
--- a/drivers/s390/scsi/zfcp_fc.h
+++ b/drivers/s390/scsi/zfcp_fc.h
@@ -235,13 +235,15 @@ void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct 
scsi_cmnd *scsi)
 /**
  * zfcp_fc_fcp_tm() - Setup FCP command as task management command.
  * @fcp: Pointer to FCP_CMND IU to set up.
- * @dev: Pointer to SCSI_device where to send the task management command.
+ * @dev: Pointer to SCSI device if LUN Reset TMF, or %NULL.
  * @tm_flags: Task management flags to setup tm command.
  */
 static inline
 void zfcp_fc_fcp_tm(struct fcp_cmnd *fcp, struct scsi_device *dev, u8 tm_flags)
 {
-   int_to_scsilun(dev->lun, (struct scsi_lun *) >fc_lun);
+   if (dev)
+   int_to_scsilun(dev->lun, (struct scsi_lun *) >fc_lun);
+
fcp->fc_tm_flags = tm_flags;
 }
 
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index f221a34c26df..2dc7d2a6f6ea 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2339,13 +2339,19 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
scsi_cmnd *scmnd,
 {
struct zfcp_fsf_req *req = NULL;
struct fcp_cmnd *fcp_cmnd;
-   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scmnd->device);
-   struct zfcp_qdio *qdio = zfcp_sdev->port->adapter->qdio;
+   struct scsi_device *sdev = scmnd->device;
+   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
+   struct zfcp_port *port = zfcp_sdev->port;
+   struct zfcp_qdio *qdio = port->adapter->qdio;
 
-   if (unlikely(!(atomic_read(_sdev->status) &
+   if (unlikely(!(atomic_read(>status) &
   ZFCP_STATUS_COMMON_UNBLOCKED)))
return NULL;
 
+   if (unlikely(zfcp_sdev && !(atomic_read(_sdev->status) &
+   ZFCP_STATUS_COMMON_UNBLOCKED)))
+   return NULL;
+
spin_lock_irq(>req_q_lock);
if (zfcp_qdio_sbal_get(qdio))
goto out;
@@ -2360,18 +2366,21 @@ struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct 
scsi_cmnd *scmnd,
}
 
fcp_cmnd = >qtcb->bottom.io.fcp_cmnd.iu;
-   req->data = (fcp_cmnd->fc_tm_flags & FCP_TMF_LUN_RESET) ?
-   scmnd->device : (void *)sdev_to_zfcp(scmnd->device)->port;
+   if (fcp_cmnd->fc_tm_flags & FCP_TMF_LUN_RESET) {
+   req->data = sdev;
+   req->qtcb->header.lun_handle = zfcp_sdev->lun_handle;
+   } else
+   req->data = port;
+
req->handler = zfcp_fsf_fcp_task_mgmt_handler;
-   req->qtcb->header.lun_handle = zfcp_sdev->lun_handle;
-   req->qtcb->header.port_handle = zfcp_sdev->port->handle;
+   req->qtcb->header.port_handle = port->handle;
req->qtcb->bottom.io.data_direction = FSF_DATADIR_CMND;
req->qtcb->bottom.io.service_class = FSF_CLASS_3;
req->qtcb->bottom.io.fcp_cmnd_length = FCP_CMND_LEN;
 
zfcp_qdio_set_sbale_last(qdio, >qdio_req);
 
-   zfcp_fc_fcp_tm(fcp_cmnd, scmnd->device, tm_flags);
+   zfcp_fc_fcp_tm(fcp_cmnd, sdev, tm_flags);
 
zfcp_fsf_start_timer(req, ZFCP_SCSI_ER_TIMEOUT);
if (!zfcp_fsf_req_send(req))
-- 
2.11.2

[RFC 2/9] zfcp: decouple TMF response handler from scsi_cmnd

[Steffen Maier]

Do not get scsi_device via req->data any more, but pass an optional(!)
scsi_device to zfcp_fsf_fcp_handler_common(). The latter must now guard
any access to scsi_device as it can be NULL.

Since we always have at least a zfcp port as scope, pass this as mandatory
argument to zfcp_fsf_fcp_handler_common() because we cannot get it through
scsi_device => zfcp_scsi_dev => port any more.

Hence, the callers of zfcp_fsf_fcp_handler_common() must resolve req->data.

TMF handling now has different context data in fsf_req->data
depending on the TMF scope in fcp_cmnd->fc_tm_flags:
* scsi_device if FCP_TMF_LUN_RESET,
* zfcp_port if FCP_TMF_TGT_RESET.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_fsf.c | 72 
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 69d1dc3ec79d..8b2b2ea552d6 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2035,27 +2035,30 @@ static void zfcp_fsf_req_trace(struct zfcp_fsf_req 
*req, struct scsi_cmnd *scsi)
sizeof(blktrc));
 }
 
-static void zfcp_fsf_fcp_handler_common(struct zfcp_fsf_req *req)
+/**
+ * zfcp_fsf_fcp_handler_common() - FCP response handler common to I/O and TMF.
+ * @req: Pointer to FSF request.
+ * @port: Pointer to zfcp port.
+ * @sdev: Pointer to SCSI device, or %NULL with Target Reset TMF.
+ */
+static void zfcp_fsf_fcp_handler_common(struct zfcp_fsf_req *req,
+   struct zfcp_port *port,
+   struct scsi_device *sdev)
 {
-   struct scsi_cmnd *scmnd = req->data;
-   struct scsi_device *sdev = scmnd->device;
-   struct zfcp_scsi_dev *zfcp_sdev;
struct fsf_qtcb_header *header = >qtcb->header;
 
if (unlikely(req->status & ZFCP_STATUS_FSFREQ_ERROR))
return;
 
-   zfcp_sdev = sdev_to_zfcp(sdev);
-
switch (header->fsf_status) {
case FSF_HANDLE_MISMATCH:
case FSF_PORT_HANDLE_NOT_VALID:
-   zfcp_erp_adapter_reopen(zfcp_sdev->port->adapter, 0, "fssfch1");
+   zfcp_erp_adapter_reopen(req->adapter, 0, "fssfch1");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
case FSF_FCPLUN_NOT_VALID:
case FSF_LUN_HANDLE_NOT_VALID:
-   zfcp_erp_port_reopen(zfcp_sdev->port, 0, "fssfch2");
+   zfcp_erp_port_reopen(port, 0, "fssfch2");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
case FSF_SERVICE_CLASS_NOT_SUPPORTED:
@@ -2066,10 +2069,10 @@ static void zfcp_fsf_fcp_handler_common(struct 
zfcp_fsf_req *req)
"Incorrect direction %d, LUN 0x%016Lx on port "
"0x%016Lx closed\n",
req->qtcb->bottom.io.data_direction,
-   (unsigned long long)zfcp_scsi_dev_lun(sdev),
-   (unsigned long long)zfcp_sdev->port->wwpn);
-   zfcp_erp_adapter_shutdown(zfcp_sdev->port->adapter, 0,
- "fssfch3");
+   sdev ? (unsigned long long)zfcp_scsi_dev_lun(sdev) :
+   ZFCP_DBF_INVALID_LUN,
+   (unsigned long long)port->wwpn);
+   zfcp_erp_adapter_shutdown(req->adapter, 0, "fssfch3");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
case FSF_CMND_LENGTH_NOT_VALID:
@@ -2077,29 +2080,32 @@ static void zfcp_fsf_fcp_handler_common(struct 
zfcp_fsf_req *req)
"Incorrect CDB length %d, LUN 0x%016Lx on "
"port 0x%016Lx closed\n",
req->qtcb->bottom.io.fcp_cmnd_length,
-   (unsigned long long)zfcp_scsi_dev_lun(sdev),
-   (unsigned long long)zfcp_sdev->port->wwpn);
-   zfcp_erp_adapter_shutdown(zfcp_sdev->port->adapter, 0,
- "fssfch4");
+   sdev ? (unsigned long long)zfcp_scsi_dev_lun(sdev) :
+   ZFCP_DBF_INVALID_LUN,
+   (unsigned long long)port->wwpn);
+   zfcp_erp_adapter_shutdown(req->adapter, 0, "fssfch4");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
case FSF_PORT_BOXED:
-   zfcp_erp_set_port_status(zfcp_sdev->port,
+   zfcp_erp_set_port_status(port,
 ZFCP_STATUS_COMMON_ACCESS_BOXED);
-   zfcp_erp_port_reopen(zfcp_sdev->port,
+   zfcp_erp_port_reopen(port,
 ZFCP_STATUS_COMMON_ERP_FAILED, "fssfch5");
req->status |= ZFCP_STATUS_FSFREQ_ERROR;
break;
case FSF_LUN_BOXED:
-   zfcp_erp_set_lun_status(sdev, 

[RFC 5/9] zfcp: decouple SCSI setup of TMF from scsi_cmnd

[Steffen Maier]

Actually change the signature of zfcp_fsf_fcp_task_mgmt().
Since it was prepared in the previous patch, we only need to delete
some local auto variables which are now the intended arguments.

Refactor zfcp_scsi_forget_cmnds() to now take a mandatory zfcp_port
and an optional zfcp_scsi_dev, which can be NULL for target reset,
instead of a mandatory zfcp_scsi_dev.

Prepare zfcp_fsf_fcp_task_mgmt's caller zfcp_task_mgmt_function()
to have its function body only depend on a mandatory zfcp_port
and an optional scsi_device, which can be NULL for target reset.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_ext.h  |  4 +++-
 drivers/s390/scsi/zfcp_fsf.c  | 15 ---
 drivers/s390/scsi/zfcp_scsi.c | 28 +++-
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
index f3101bc5d1bc..26772b0c1c39 100644
--- a/drivers/s390/scsi/zfcp_ext.h
+++ b/drivers/s390/scsi/zfcp_ext.h
@@ -118,7 +118,9 @@ extern int zfcp_fsf_send_els(struct zfcp_adapter *, u32,
 struct zfcp_fsf_ct_els *, unsigned int);
 extern int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *);
 extern void zfcp_fsf_req_free(struct zfcp_fsf_req *);
-extern struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd *, u8);
+extern struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct zfcp_port *port,
+  struct scsi_device *sdev,
+  u8 tm_flags);
 extern struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd *);
 extern void zfcp_fsf_reqid_check(struct zfcp_qdio *, int);
 
diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
index 2dc7d2a6f6ea..7cc2d7ee1f56 100644
--- a/drivers/s390/scsi/zfcp_fsf.c
+++ b/drivers/s390/scsi/zfcp_fsf.c
@@ -2329,19 +2329,20 @@ static void zfcp_fsf_fcp_task_mgmt_handler(struct 
zfcp_fsf_req *req)
 }
 
 /**
- * zfcp_fsf_fcp_task_mgmt - send SCSI task management command
- * @scmnd: SCSI command to send the task management command for
- * @tm_flags: unsigned byte for task management flags
- * Returns: on success pointer to struct fsf_req, NULL otherwise
+ * zfcp_fsf_fcp_task_mgmt() - Send SCSI task management command (TMF).
+ * @port: Pointer to zfcp port as scope for TMF.
+ * @sdev: Pointer to scsi device if LUN Reset TMF, or %NULL.
+ * @tm_flags: Unsigned byte for task management flags.
+ *
+ * Return: On success pointer to struct fsf_req, %NULL otherwise.
  */
-struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd *scmnd,
+struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct zfcp_port *port,
+   struct scsi_device *sdev,
u8 tm_flags)
 {
struct zfcp_fsf_req *req = NULL;
struct fcp_cmnd *fcp_cmnd;
-   struct scsi_device *sdev = scmnd->device;
struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
-   struct zfcp_port *port = zfcp_sdev->port;
struct zfcp_qdio *qdio = port->adapter->qdio;
 
if (unlikely(!(atomic_read(>status) &
diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
index cd0f811452b7..05c823ccb959 100644
--- a/drivers/s390/scsi/zfcp_scsi.c
+++ b/drivers/s390/scsi/zfcp_scsi.c
@@ -234,12 +234,20 @@ static void zfcp_scsi_forget_cmnd(struct zfcp_fsf_req 
*old_req, void *data)
old_req->data = NULL;
 }
 
-static void zfcp_scsi_forget_cmnds(struct zfcp_scsi_dev *zsdev, u8 tm_flags)
+/**
+ * zfcp_scsi_forget_cmnds() - Forget pending SCSI requests on given scope.
+ * @port: Pointer to zfcp port indicating scope.
+ * @zsdev: Pointer to zfcp scsi dev as scope, or %NULL if scope is only port.
+ * @tm_flags: Task management flags,
+ *here we only handle %FCP_TMF_TGT_RESET or %FCP_TMF_LUN_RESET.
+ */
+static void zfcp_scsi_forget_cmnds(struct zfcp_port *port,
+  struct zfcp_scsi_dev *zsdev, u8 tm_flags)
 {
-   struct zfcp_adapter *adapter = zsdev->port->adapter;
+   struct zfcp_adapter *adapter = port->adapter;
struct zfcp_scsi_req_filter filter = {
.tmf_scope = FCP_TMF_TGT_RESET,
-   .port_handle = zsdev->port->handle,
+   .port_handle = port->handle,
};
unsigned long flags;
 
@@ -260,19 +268,21 @@ static void zfcp_scsi_forget_cmnds(struct zfcp_scsi_dev 
*zsdev, u8 tm_flags)
 
 static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
 {
-   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
-   struct zfcp_adapter *adapter = zfcp_sdev->port->adapter;
-   unsigned int scsi_id = zfcp_sdev->port->starget_id;
+   struct scsi_device *sdev = scpnt->device;
+   struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
+   struct zfcp_port *port = zfcp_sdev->port;
+   struct zfcp_adapter *adapter = port->adapter;
+   unsigned int scsi_id 

[RFC 1/9] zfcp: drop unsuitable scsi_cmnd usage from SCSI traces for scsi_eh / TMF

[Steffen Maier]

The SCSI command pointer passed to scsi_eh callbacks is just one arbitrary
command of potentially many that are in the eh queue to be processed.
The command is only used to indirectly pass the TMF scope in terms of
SCSI ID/target and SCSI LUN for LUN reset.

Hence, zfcp had filled in SCSI trace record fields which do not really
belong to the TMF. This was confusing.

Therefore, refactor the TMF tracing to work without SCSI command
and instead pass explicit arguments for SCSI ID and SCSI LUN.
As context we now need a pointer to zfcp_adapter.
To make it even clearer, we set all bits to 1 for the fields, which do
not belong to the TMF, to indicate that these fields are invalid.

The old zfcp_dbf_scsi() became zfcp_dbf_scsi_common() to now handle both
SCSI commands and TMFs. The old argument scsi_cmnd is now optional and
can be NULL with TMFs. Two new arguments scsi_id and scsi_lun are
optional and only used if scsi_cmnd is NULL, i.e. with TMFs.

Signed-off-by: Steffen Maier 
---
 drivers/s390/scsi/zfcp_dbf.c  | 51 ---
 drivers/s390/scsi/zfcp_dbf.h  | 26 +++---
 drivers/s390/scsi/zfcp_ext.h  |  8 ---
 drivers/s390/scsi/zfcp_scsi.c | 20 -
 4 files changed, 71 insertions(+), 34 deletions(-)

diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
index 8227076c9cbb..dca624aaa7c0 100644
--- a/drivers/s390/scsi/zfcp_dbf.c
+++ b/drivers/s390/scsi/zfcp_dbf.c
@@ -577,16 +577,19 @@ void zfcp_dbf_san_in_els(char *tag, struct zfcp_fsf_req 
*fsf)
 }
 
 /**
- * zfcp_dbf_scsi - trace event for scsi commands
- * @tag: identifier for event
- * @sc: pointer to struct scsi_cmnd
- * @fsf: pointer to struct zfcp_fsf_req
+ * zfcp_dbf_scsi_common() - Common trace event helper for scsi.
+ * @tag: Identifier for event.
+ * @level: trace level of event.
+ * @adapter: Pointer to zfcp adapter as context for this event.
+ * @sc: Pointer to SCSI command, or NULL with task management function (TMF).
+ * @fsf: Pointer to FSF request, or NULL.
+ * @scsi_id: SCSI ID/target to indicate scope, only for TMF.
+ * @scsi_lun: SCSI LUN if TMF is Logical Unit Reset, else 
%ZFCP_DBF_INVALID_LUN.
  */
-void zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd *sc,
-  struct zfcp_fsf_req *fsf)
+void zfcp_dbf_scsi_common(char *tag, int level, struct zfcp_adapter *adapter,
+ struct scsi_cmnd *sc, struct zfcp_fsf_req *fsf,
+ unsigned int scsi_id, u64 scsi_lun)
 {
-   struct zfcp_adapter *adapter =
-   (struct zfcp_adapter *) sc->device->host->hostdata[0];
struct zfcp_dbf *dbf = adapter->dbf;
struct zfcp_dbf_scsi *rec = >scsi_buf;
struct fcp_resp_with_ext *fcp_rsp;
@@ -598,16 +601,28 @@ void zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd 
*sc,
 
memcpy(rec->tag, tag, ZFCP_DBF_TAG_LEN);
rec->id = ZFCP_DBF_SCSI_CMND;
-   rec->scsi_result = sc->result;
-   rec->scsi_retries = sc->retries;
-   rec->scsi_allowed = sc->allowed;
-   rec->scsi_id = sc->device->id;
-   rec->scsi_lun = (u32)sc->device->lun;
-   rec->scsi_lun_64_hi = (u32)(sc->device->lun >> 32);
-   rec->host_scribble = (unsigned long)sc->host_scribble;
-
-   memcpy(rec->scsi_opcode, sc->cmnd,
-  min((int)sc->cmd_len, ZFCP_DBF_SCSI_OPCODE));
+   if (sc) {
+   rec->scsi_result = sc->result;
+   rec->scsi_retries = sc->retries;
+   rec->scsi_allowed = sc->allowed;
+   rec->scsi_id = sc->device->id;
+   rec->scsi_lun = (u32)sc->device->lun;
+   rec->scsi_lun_64_hi = (u32)(sc->device->lun >> 32);
+   rec->host_scribble = (unsigned long)sc->host_scribble;
+
+   memcpy(rec->scsi_opcode, sc->cmnd,
+  min_t(int, sc->cmd_len, ZFCP_DBF_SCSI_OPCODE));
+   } else {
+   rec->scsi_result = ~0;
+   rec->scsi_retries = ~0;
+   rec->scsi_allowed = ~0;
+   rec->scsi_id = scsi_id;
+   rec->scsi_lun = (u32)scsi_lun;
+   rec->scsi_lun_64_hi = (u32)(scsi_lun >> 32);
+   rec->host_scribble = ~0;
+
+   memset(rec->scsi_opcode, 0xff, ZFCP_DBF_SCSI_OPCODE);
+   }
 
if (fsf) {
rec->fsf_req_id = fsf->req_id;
diff --git a/drivers/s390/scsi/zfcp_dbf.h b/drivers/s390/scsi/zfcp_dbf.h
index 3508c00458f4..6e29e7cccbc4 100644
--- a/drivers/s390/scsi/zfcp_dbf.h
+++ b/drivers/s390/scsi/zfcp_dbf.h
@@ -358,7 +358,8 @@ void _zfcp_dbf_scsi(char *tag, int level, struct scsi_cmnd 
*scmd,
scmd->device->host->hostdata[0];
 
if (debug_level_enabled(adapter->dbf->scsi, level))
-   zfcp_dbf_scsi(tag, level, scmd, req);
+   zfcp_dbf_scsi_common(tag, level, adapter, scmd, req,
+~0, ZFCP_DBF_INVALID_LUN);
 }
 
 /**

[RFC 0/9] zfcp: decouple scsi_eh callbacks from scsi_cmnd

[Steffen Maier]

This is an early request for comments.
The patch set serves as a zfcp preparation step for Hannes' series
"[PATCH 00/47] SCSI EH argument reshuffle part II"
http://www.spinics.net/lists/linux-scsi/msg65.html
or
http://marc.info/?l=linux-scsi=150091945302995=2

The series is based on 18 preceding zfcp patches,
including some stable regression bugfixes for zfcp tracing.
Hence it might not apply cleanly.
However, we plan to post the 18 preceding patches soon for integration
and I would like to get those in first.

Please do not apply to any tree that will merge into upstream yet,
as it's not ready for prime time.
It only builds (after each patch; sparse and checkpatch clean)
but it has not seen any function testing yet.

There are still some open questions:
* Search victim scsi_device in target_reset_handler just to get a LUN?
  (Even if we do, zfcp_fsf_fcp_handler_common() should not print that LUN!)
  http://www.spinics.net/lists/linux-scsi/msg64.html
  http://www.spinics.net/lists/linux-scsi/msg66.html
* Exact rport blocking logic in host_reset_handler.
  http://www.spinics.net/lists/linux-scsi/msg65.html


Steffen Maier (9):
  zfcp: drop unsuitable scsi_cmnd usage from SCSI traces for scsi_eh /
TMF
  zfcp: decouple TMF response handler from scsi_cmnd
  zfcp: split FCP_CMND IU setup between SCSI I/O and TMF again
  zfcp: decouple FSF request setup of TMF from scsi_cmnd
  zfcp: decouple SCSI setup of TMF from scsi_cmnd
  scsi: fc: start decoupling fc_block_scsi_eh from scsi_cmnd
  zfcp: use fc_block_rport for TMFs and host reset to decouple from
scsi_cmnd
  zfcp: fix waiting for rport(s) unblock in eh_host_reset_handler
  zfcp: decouple our scsi_eh callbacks from scsi_cmnd

 drivers/s390/scsi/zfcp_dbf.c |  51 ---
 drivers/s390/scsi/zfcp_dbf.h |  26 +++---
 drivers/s390/scsi/zfcp_ext.h |  12 +++--
 drivers/s390/scsi/zfcp_fc.h  |  24 ++---
 drivers/s390/scsi/zfcp_fsf.c | 106 +--
 drivers/s390/scsi/zfcp_scsi.c| 105 +-
 drivers/scsi/scsi_transport_fc.c |  31 ++--
 include/scsi/scsi_transport_fc.h |   1 +
 8 files changed, 252 insertions(+), 104 deletions(-)

-- 
2.11.2

Re: [PATCH 24/47] zfcp: use scsi device as argument for zfcp_task_mgmt_function()

[Hannes Reinecke]

On 07/24/2017 07:15 PM, Steffen Maier wrote:
> 
> 
> On 06/28/2017 10:32 AM, Hannes Reinecke wrote:
>> zfcp_task_mgmt_function() is only used for lun and device reset,
>> so it should be using the scsi device as an argument, not the
>> scsi command.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>   drivers/s390/scsi/zfcp_ext.h  |  2 +-
>>   drivers/s390/scsi/zfcp_fc.h   |  9 +
>>   drivers/s390/scsi/zfcp_fsf.c  | 21 +++--
>>   drivers/s390/scsi/zfcp_scsi.c | 12 ++--
>>   4 files changed, 19 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/s390/scsi/zfcp_ext.h b/drivers/s390/scsi/zfcp_ext.h
>> index 57f1d4a..64d4db7 100644
>> --- a/drivers/s390/scsi/zfcp_ext.h
>> +++ b/drivers/s390/scsi/zfcp_ext.h
>> @@ -117,7 +117,7 @@ extern int zfcp_fsf_send_els(struct zfcp_adapter
>> *, u32,
>>struct zfcp_fsf_ct_els *, unsigned int);
>>   extern int zfcp_fsf_fcp_cmnd(struct scsi_cmnd *);
>>   extern void zfcp_fsf_req_free(struct zfcp_fsf_req *);
>> -extern struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd
>> *, u8);
>> +extern struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_device
>> *, u8);
>>   extern struct zfcp_fsf_req *zfcp_fsf_abort_fcp_cmnd(struct scsi_cmnd
>> *);
>>   extern void zfcp_fsf_reqid_check(struct zfcp_qdio *, int);
>>
>> diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h
>> index df2b541..f08eaf9 100644
>> --- a/drivers/s390/scsi/zfcp_fc.h
>> +++ b/drivers/s390/scsi/zfcp_fc.h
>> @@ -206,19 +206,12 @@ struct zfcp_fc_wka_ports {
>>* zfcp_fc_scsi_to_fcp - setup FCP command with data from scsi_cmnd
>>* @fcp: fcp_cmnd to setup
>>* @scsi: scsi_cmnd where to get LUN, task attributes/flags and CDB
>> - * @tm: task management flags to setup task management command
>>*/
>>   static inline
>> -void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi,
>> - u8 tm_flags)
>> +void zfcp_fc_scsi_to_fcp(struct fcp_cmnd *fcp, struct scsi_cmnd *scsi)
>>   {
>>   int_to_scsilun(scsi->device->lun, (struct scsi_lun *)
>> >fc_lun);
>>
>> -if (unlikely(tm_flags)) {
>> -fcp->fc_tm_flags = tm_flags;
>> -return;
>> -}
>> -
>>   fcp->fc_pri_ta = FCP_PTA_SIMPLE;
>>
>>   if (scsi->sc_data_direction == DMA_FROM_DEVICE)
> 
> When I wrote my zfcp decoupling patch series I did a lot of git history
> research in order to double check if my changes are OK.
> 
> Here, I figured that I'd like to separately revert commit 2443c8b23aea
> ("[SCSI] zfcp: Merge FCP task management setup with regular FCP command
> setup"), because this introduced a dependency on the unsuitable SCSI
> command for scsi_eh / TMF. Even though it was one of the first commits I
> posted upstream as newbie zfcp maintainer... little did I know.
> 
Okay, do.

>> diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c
>> index 27ff38f..c4bd3d4 100644
>> --- a/drivers/s390/scsi/zfcp_fsf.c
>> +++ b/drivers/s390/scsi/zfcp_fsf.c
>> @@ -2036,10 +2036,9 @@ static void zfcp_fsf_req_trace(struct
>> zfcp_fsf_req *req, struct scsi_cmnd *scsi)
> 
>> -static void zfcp_fsf_fcp_handler_common(struct zfcp_fsf_req *req)
>> +static void zfcp_fsf_fcp_handler_common(struct zfcp_fsf_req *req,
>> +struct scsi_device *sdev)
> 
> 
>> @@ -2120,7 +2119,7 @@ static void zfcp_fsf_fcp_cmnd_handler(struct
>> zfcp_fsf_req *req)
> 
>> -zfcp_fsf_fcp_handler_common(req);
>> +zfcp_fsf_fcp_handler_common(req, scpnt->device);
> 
>> @@ -2296,8 +2295,9 @@ static void
>> zfcp_fsf_fcp_task_mgmt_handler(struct zfcp_fsf_req *req)
>>   {
> 
>> +struct scsi_device *sdev = req->data;
>>
>> -zfcp_fsf_fcp_handler_common(req);
>> +zfcp_fsf_fcp_handler_common(req, sdev);
> 
> Moving the resolving of req->data into the callers of
> zfcp_fsf_fcp_handler_common() I did the same way in my own patch series.
> 
:-)

>> @@ -2313,12 +2313,12 @@ static void
>> zfcp_fsf_fcp_task_mgmt_handler(struct zfcp_fsf_req *req)
> 
>> -struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd *scmnd,
>> +struct zfcp_fsf_req *zfcp_fsf_fcp_task_mgmt(struct scsi_device *sdev,
>>   u8 tm_flags)
> 
>> @@ -2338,7 +2338,7 @@ struct zfcp_fsf_req
>> *zfcp_fsf_fcp_task_mgmt(struct scsi_cmnd *scmnd,
> 
>> -req->data = scmnd;
>> +req->data = sdev;
> 
>> diff --git a/drivers/s390/scsi/zfcp_scsi.c
>> b/drivers/s390/scsi/zfcp_scsi.c
>> index 5fada93..dd7bea0 100644
>> --- a/drivers/s390/scsi/zfcp_scsi.c
>> +++ b/drivers/s390/scsi/zfcp_scsi.c
>> @@ -259,17 +259,17 @@ static void zfcp_scsi_forget_cmnds(struct
>> zfcp_scsi_dev *zsdev, u8 tm_flags)
> 
>> -static int zfcp_task_mgmt_function(struct scsi_cmnd *scpnt, u8 tm_flags)
>> +static int zfcp_task_mgmt_function(struct scsi_device *sdev, u8
>> tm_flags)
>>   {
>> -struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(scpnt->device);
>> +struct zfcp_scsi_dev *zfcp_sdev = sdev_to_zfcp(sdev);
>>   struct zfcp_adapter *adapter 

Re: [patch 0/5] scsi/bnx2*: Plug hotplug race, correct locking and simplify hotplug code

[Chad Dupuis]

On Mon, 24 Jul 2017, 6:52am, Thomas Gleixner wrote:

> The conversion of the cpu hotplug locking to a percpu rwsem does not longer
> allow recursive locking of the hotplug lock.
> 
> The BNX2I and BNX2FC drivers install/remove hotplug states with the hotplug
> lock held. The install/removal code acquired the hotplug lock as well.
> 
> While looking into this, I noticed an interesting hotplug race in the
> BNX2FC driver, which could result in dereferencing a NULL pointer or freed
> and potentially reused memory.
> 
> The following series addresses these problems and as a final step on top it
> simplifies the hotplug code in both drivers.
> 
> Thanks,
> 
>   tglx
> 
> 
>  drivers/scsi/bnx2fc/bnx2fc_fcoe.c |   68 
> --
>  drivers/scsi/bnx2fc/bnx2fc_hwi.c  |   45 -
>  drivers/scsi/bnx2i/bnx2i_init.c   |   64 ---
>  include/linux/cpuhotplug.h|2 -
>  4 files changed, 53 insertions(+), 126 deletions(-)
> 

We tested the series and everything was fine.  Ack to the series.

Acked-by: Chad Dupuis 

[PATCH v2] scsi: ufs: Implement Auto-Hibern8 setup

[Michal Potomski]

From: Michał Potomski 

Since Auto-Hibern8 feature has to be enabled by the user
proper API has been given via SysFS.

We expose this API to user-space, since we don't know
in driver, what kind of additional Power Management rules
user wants to use. Due to that we want to expose API, to
let user or high level S/W decide, whether it wants to
use Auto-Hibern8 feature for Power Saving and give him
"slider" to decide between performance and power efficiency.
This is important because different platforms using
the same UFS host and/or device might want different
options on this one, e.g. High-End Desktop PC might
want to have this feature disabled, while Mobile or
Server platforms might want to have this feature enabled,
but levels will vary depending on what's to be acheived.

As this feature is meant to be transparent for driver,
we'd like to let user decide whether he wants this enabled
or not.

Signed-off-by: Michał Potomski 
---
 drivers/scsi/ufs/ufshcd.c | 91 +++
 drivers/scsi/ufs/ufshcd.h |  2 ++
 drivers/scsi/ufs/ufshci.h | 23 +---
 3 files changed, 112 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5bc9dc14e075..7bed33a023db 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -931,6 +931,31 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool 
scale_up)
 }
 
 /**
+ * ufshcd_read_auto_hibern8_state - Reads hosts auto-hibern8 feature state
+ * @hba: per adapter instance
+ */
+u32 ufshcd_read_auto_hibern8_state(struct ufs_hba *hba)
+{
+   return ufshcd_readl(hba, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
+ * ufshcd_setup_auto_hibern8 - Sets up hosts auto-hibern8 feature
+ * @hba: per adapter instance
+ * @scale: timer scale (1/10/100us/1/10/100ms)
+ * @timer_val: value to be multipled with scale (idle timeout)
+ */
+void ufshcd_setup_auto_hibern8(struct ufs_hba *hba, u8 scale, u16 timer_val)
+{
+   u32 val = (scale << UFSHCI_AHIBERN8_SCALE_OFFSET)
+   & UFSHCI_AHIBERN8_SCALE_MASK;
+
+   val |= timer_val & UFSHCI_AHIBERN8_TIMER_MASK;
+
+   ufshcd_writel(hba, val, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
  * ufshcd_is_devfreq_scaling_required - check if scaling is required or not
  * @hba: per adapter instance
  * @scale_up: True if scaling up and false if scaling down
@@ -5715,6 +5740,54 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
return err;
 }
 
+#define UFS_AHIBERN8_SCALE_STEP_MAGNITUDE  10
+
+static ssize_t ufshcd_auto_hibern8_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct ufs_hba *hba = dev_get_drvdata(dev);
+   u32 val;
+   unsigned long timer;
+   u8 scale;
+   char *unit;
+
+   val = ufshcd_read_auto_hibern8_state(hba);
+   timer = val & UFSHCI_AHIBERN8_TIMER_MASK;
+   scale = (val & UFSHCI_AHIBERN8_SCALE_MASK)
+   >> UFSHCI_AHIBERN8_SCALE_OFFSET;
+
+   unit = scale >= UFSHCI_AHIBERN8_SCALE_1MS ? "ms" : "us";
+
+   for (scale %= UFSHCI_AHIBERN8_SCALE_1MS; scale > 0; --scale)
+   timer *= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+
+   return snprintf(buf, PAGE_SIZE, "%ld %s\n", timer, unit);
+}
+
+static ssize_t ufshcd_auto_hibern8_store(struct device *dev,
+   struct device_attribute *atr, const char *buf, size_t count)
+{
+   struct ufs_hba *hba = dev_get_drvdata(dev);
+   unsigned long timer;
+   u8 scale = UFSHCI_AHIBERN8_SCALE_1US;
+
+   if (kstrtoul(buf, 0, ))
+   return -EINVAL;
+
+   while (timer > UFSHCI_AHIBERN8_TIMER_MASK &&
+  scale < UFSHCI_AHIBERN8_SCALE_MAX) {
+   timer /= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+   ++scale;
+   }
+
+   if (scale >= UFSHCI_AHIBERN8_SCALE_MAX)
+   return -EINVAL;
+
+   ufshcd_setup_auto_hibern8(hba, scale, (u16) timer);
+
+   return count;
+}
+
 /**
  * ufshcd_host_reset_and_restore - reset and restore host controller
  * @hba: per-adapter instance
@@ -7693,16 +7766,34 @@ static void ufshcd_add_spm_lvl_sysfs_nodes(struct 
ufs_hba *hba)
dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
 }
 
+static void ufshcd_add_ahibern8_sysfs_node(struct ufs_hba *hba)
+{
+   if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
+   return;
+
+   hba->ahibern8_attr.show = ufshcd_auto_hibern8_show;
+   hba->ahibern8_attr.store = ufshcd_auto_hibern8_store;
+   sysfs_attr_init(>ahibern8_attr.attr);
+   hba->ahibern8_attr.attr.name = "ufs_auto_hibern8";
+   hba->ahibern8_attr.attr.mode = 0600;
+   if (device_create_file(hba->dev, >ahibern8_attr))
+   dev_err(hba->dev, "Failed to create sysfs for 
ufs_auto_hibern8\n");
+}
+
 static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
 {
ufshcd_add_rpm_lvl_sysfs_nodes(hba);
  

[PATCH] scsi: ufs: Implement Auto-Hibern8 setup

[Michal Potomski]

From: Michał Potomski 

Since Auto-Hibern8 feature has to be enabled by the user
proper API has been given via SysFS.

We expose this API to user-space, since we don't know
in driver, what kind of additional Power Management rules
user wants to use. Due to that we want to expose API, to
let user or high level S/W decide, whether it wants to
use Auto-Hibern8 feature for Power Saving and give him
"slider" to decide between performance and power efficiency.
This is important because different platforms using
the same UFS host and/or device might want different
options on this one, e.g. High-End Desktop PC might
want to have this feature disabled, while Mobile or
Server platforms might want to have this feature enabled,
but levels will vary depending on what's to be acheived.

As this feature is meant to be transparent for driver,
we'd like to let user decide whether he wants this enabled
or not.

Signed-off-by: Michał Potomski 
---
 drivers/scsi/ufs/ufshcd.c | 92 +++
 drivers/scsi/ufs/ufshcd.h |  2 ++
 drivers/scsi/ufs/ufshci.h | 23 +---
 3 files changed, 113 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 5bc9dc14e075..a401ab61b5a2 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -931,6 +931,31 @@ static int ufshcd_scale_clks(struct ufs_hba *hba, bool 
scale_up)
 }
 
 /**
+ * ufshcd_read_auto_hibern8_state - Reads hosts auto-hibern8 feature state
+ * @hba: per adapter instance
+ */
+u32 ufshcd_read_auto_hibern8_state(struct ufs_hba *hba)
+{
+   return ufshcd_readl(hba, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
+ * ufshcd_setup_auto_hibern8 - Sets up hosts auto-hibern8 feature
+ * @hba: per adapter instance
+ * @scale: timer scale (1/10/100us/1/10/100ms)
+ * @timer_val: value to be multipled with scale (idle timeout)
+ */
+void ufshcd_setup_auto_hibern8(struct ufs_hba *hba, u8 scale, u16 timer_val)
+{
+   u32 val = (scale << UFSHCI_AHIBERN8_SCALE_OFFSET)
+   & UFSHCI_AHIBERN8_SCALE_MASK;
+
+   val |= timer_val & UFSHCI_AHIBERN8_TIMER_MASK;
+
+   ufshcd_writel(hba, val, REG_AUTO_HIBERN8_IDLE_TIMER);
+}
+
+/**
  * ufshcd_is_devfreq_scaling_required - check if scaling is required or not
  * @hba: per adapter instance
  * @scale_up: True if scaling up and false if scaling down
@@ -5715,6 +5740,55 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
return err;
 }
 
+#define UFS_AHIBERN8_SCALE_STEP_MAGNITUDE  10
+
+static ssize_t ufshcd_auto_hibern8_show(struct device *dev,
+   struct device_attribute *attr, char *buf)
+{
+   struct ufs_hba *hba = dev_get_drvdata(dev);
+   u32 val;
+   unsigned long timer;
+   u8 scale;
+   char *unit;
+
+   val = ufshcd_read_auto_hibern8_state(hba);
+   timer = val & UFSHCI_AHIBERN8_TIMER_MASK;
+   scale = (val & UFSHCI_AHIBERN8_SCALE_MASK) 
+   >> UFSHCI_AHIBERN8_SCALE_OFFSET;
+
+   unit = scale >= UFSHCI_AHIBERN8_SCALE_1MS ? "ms" : "us";
+
+   for (scale %= UFSHCI_AHIBERN8_SCALE_1MS; scale > 0; --scale)
+   timer *= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+
+   return snprintf(buf, PAGE_SIZE, "%ld %s\n", timer, unit);
+}
+
+static ssize_t ufshcd_auto_hibern8_store(struct device *dev,
+   struct device_attribute *atr, const char *buf, size_t count)
+{
+   struct ufs_hba *hba = dev_get_drvdata(dev);
+   unsigned long timer;
+   u8 scale = UFSHCI_AHIBERN8_SCALE_1US;
+
+   if (kstrtoul(buf, 0, )) {
+   return -EINVAL;
+   }
+
+   while (timer > UFSHCI_AHIBERN8_TIMER_MASK &&
+  scale < UFSHCI_AHIBERN8_SCALE_MAX) {
+   timer /= UFS_AHIBERN8_SCALE_STEP_MAGNITUDE;
+   ++scale;
+   }
+
+   if (scale >= UFSHCI_AHIBERN8_SCALE_MAX)
+   return -EINVAL;
+
+   ufshcd_setup_auto_hibern8(hba, scale, (u16) timer);
+
+   return count;
+}
+
 /**
  * ufshcd_host_reset_and_restore - reset and restore host controller
  * @hba: per-adapter instance
@@ -7693,16 +7767,34 @@ static void ufshcd_add_spm_lvl_sysfs_nodes(struct 
ufs_hba *hba)
dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
 }
 
+static void ufshcd_add_ahibern8_sysfs_node(struct ufs_hba *hba)
+{
+   if (!(hba->capabilities & MASK_AUTO_HIBERN8_SUPPORT))
+   return;
+
+   hba->ahibern8_attr.show = ufshcd_auto_hibern8_show;
+   hba->ahibern8_attr.store = ufshcd_auto_hibern8_store;
+   sysfs_attr_init(>ahibern8_attr.attr);
+   hba->ahibern8_attr.attr.name = "ufs_auto_hibern8";
+   hba->ahibern8_attr.attr.mode = 0600;
+   if (device_create_file(hba->dev, >ahibern8_attr))
+   dev_err(hba->dev, "Failed to create sysfs for 
ufs_auto_hibern8\n");
+}
+
 static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
 {

[PATCH v2] scsi: qedf: Limit number of CQs

[Thomas Bogendoerfer]

From: Thomas Bogendoerfer 

FCOE offloading failed with:

[qed_sp_fcoe_func_start:150(sp-0-3b:00.02)]Cannot satisfy CQ amount. CQs
 requested 8, CQs available 6. Aborting function start
[qed_fcoe_start:821()]Failed to start fcoe
[__qedf_probe:3041]:6: Cannot start FCoE function.

The reason is a newly introduced check in the qed main part. This change
also provides the information about how many CQs are available, so we
simply limit the number of requested CQs..

Fixes: 3c5da9427802 ("qed: Share additional information with qedf")
Signed-off-by: Thomas Bogendoerfer 
---

Changes in v2:
- integrated suggested change from Chad Dupuis

 drivers/scsi/qedf/qedf.h  |  3 ++-
 drivers/scsi/qedf/qedf_main.c | 20 +---
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/qedf/qedf.h b/drivers/scsi/qedf/qedf.h
index 4d038926a455..351f06dfc5a0 100644
--- a/drivers/scsi/qedf/qedf.h
+++ b/drivers/scsi/qedf/qedf.h
@@ -528,7 +528,8 @@ struct fip_vlan {
 #define QEDF_WRITE(1 << 0)
 #define MAX_FIBRE_LUNS 0x
 
-#define QEDF_MAX_NUM_CQS   8
+#define MIN_NUM_CPUS_MSIX(x)   min_t(u32, x->dev_info.num_cqs, \
+   num_online_cpus())
 
 /*
  * PCI function probe defines
diff --git a/drivers/scsi/qedf/qedf_main.c b/drivers/scsi/qedf/qedf_main.c
index 7786c97e033f..1d13c9ca517d 100644
--- a/drivers/scsi/qedf/qedf_main.c
+++ b/drivers/scsi/qedf/qedf_main.c
@@ -2760,11 +2760,9 @@ static int qedf_set_fcoe_pf_param(struct qedf_ctx *qedf)
 * we allocation is the minimum off:
 *
 * Number of CPUs
-* Number of MSI-X vectors
-* Max number allocated in hardware (QEDF_MAX_NUM_CQS)
+* Number allocated by qed for our PCI function
 */
-   qedf->num_queues = min((unsigned int)QEDF_MAX_NUM_CQS,
-   num_online_cpus());
+   qedf->num_queues = MIN_NUM_CPUS_MSIX(qedf);
 
QEDF_INFO(&(qedf->dbg_ctx), QEDF_LOG_DISC, "Number of CQs is %d.\n",
   qedf->num_queues);
@@ -2962,6 +2960,13 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
goto err1;
}
 
+   /* Learn information crucial for qedf to progress */
+   rc = qed_ops->fill_dev_info(qedf->cdev, >dev_info);
+   if (rc) {
+   QEDF_ERR(&(qedf->dbg_ctx), "Failed to dev info.\n");
+   goto err1;
+   }
+
/* queue allocation code should come here
 * order should be
 *  slowpath_start
@@ -2977,13 +2982,6 @@ static int __qedf_probe(struct pci_dev *pdev, int mode)
}
qed_ops->common->update_pf_params(qedf->cdev, >pf_params);
 
-   /* Learn information crucial for qedf to progress */
-   rc = qed_ops->fill_dev_info(qedf->cdev, >dev_info);
-   if (rc) {
-   QEDF_ERR(&(qedf->dbg_ctx), "Failed to dev info.\n");
-   goto err1;
-   }
-
/* Record BDQ producer doorbell addresses */
qedf->bdq_primary_prod = qedf->dev_info.primary_dbq_rq_addr;
qedf->bdq_secondary_prod = qedf->dev_info.secondary_bdq_rq_addr;
-- 
2.12.3

Re: [PATCH v2 6/6] qla2xxx: Refactor usage of Active command arrays

[Johannes Thumshirn]

On Fri, Jul 21, 2017 at 09:32:28AM -0700, Himanshu Madhani wrote:
> +typedef enum {
> + TYPE_SRB,
> + TYPE_TGT_CMD,
> +} cmd_type_t;

Minor Nitpick if you have to re-send, please no typedefs.

Anyways,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 4/6] qla2xxx: Simpify unregistration of FC-NVMe local/remote ports

[Johannes Thumshirn]

Thanks, 
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 3/6] qla2xxx: Added change to enable ZIO for FC-NVMe devices

[Johannes Thumshirn]

Looks good to me,
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH v2 2/6] qla2xxx: Move function prototype to correct header

[Johannes Thumshirn]

Thanks, 
Reviewed-by: Johannes Thumshirn 
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [REGRESSION] 28676d869bbb (scsi: sg: check for valid direction before starting the request) breaks mtx tape library control

[Johannes Thumshirn]

On Fri, Jul 21, 2017 at 02:23:16PM -0500, Jason L Tibbitts III wrote:
> I can also apply the debugging patch and try again if that would give
> you more useful information.

Yes please (on top of the snippet I've sent you last).

Thanks a lot,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850