Re: [PATCH 2/2] scsi: Fix RCU handling for VPD pages

[Hannes Reinecke]

On 01/21/2016 07:35 AM, Alexander Duyck wrote:
> This patch is meant to fix the RCU handling for VPD pages.  The original
> code had a number of issues including the fact that the local variables
> were being declared as __rcu, the RCU variable being directly accessed
> outside of the RCU locked region, and the fact that length was not
> associated with the data so it would be possible to get a mix and match of
> the length for one VPD page with the data from another.
> 
> Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
> Signed-off-by: Alexander Duyck 
> ---
>  drivers/scsi/scsi.c|   52 
> +++-
>  drivers/scsi/scsi_lib.c|   12 +-
>  drivers/scsi/scsi_sysfs.c  |   14 +++-
>  include/scsi/scsi_device.h |   14 
>  4 files changed, 50 insertions(+), 42 deletions(-)
> 
Thanks for fixing this up. I didn't really like the two distinct
variables for vpd buffer and length, too, but hadn't thought of
using a struct for here.

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] scsi: Do not attach VPD to devices that don't support it

[Hannes Reinecke]

On 01/21/2016 07:35 AM, Alexander Duyck wrote:
> The patch "scsi: rescan VPD attributes" introduced a regression in which
> devices that don't support VPD were being scanned for VPD attributes
> anyway.  This could cause issues for this parts and should be avoided so
> the check for scsi_level has been moved out of scsi_add_lun and into
> scsi_attach_vpd so that all callers will not scan VPD for devices that
> don't support it.
> 
> Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
> Signed-off-by: Alexander Duyck 
> ---
>  drivers/scsi/scsi.c  |3 +++
>  drivers/scsi/scsi_scan.c |3 +--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index b1bf42b93fcc..ed085e78c893 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -784,6 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
>   int pg83_supported = 0;
>   unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
>  
> + if (sdev->scsi_level < SCSI_3)
> + return;
> +
>   if (sdev->skip_vpd_pages)
>   return;
>  retry_pg0:
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a820668d442..1b16c89e0cf9 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -986,8 +986,7 @@ static int scsi_add_lun(struct scsi_device *sdev, 
> unsigned char *inq_result,
>   }
>   }
>  
> - if (sdev->scsi_level >= SCSI_3)
> - scsi_attach_vpd(sdev);
> + scsi_attach_vpd(sdev);
>  
>   sdev->max_queue_depth = sdev->queue_depth;
>  
> 
Isn't this slightly pointless, given that we're testing the inverse
condition in scsi_attach_vpd()?

And in anycase, I guess we should be using the same logic sd.c is
using. Please see the attached patch.

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)
From bc662c5a0255e868746ef317e2eff04dc3fcfac5 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke 
Date: Thu, 21 Jan 2016 08:18:49 +0100
Subject: [PATCH] scsi: Do not attach VPD to devices that don't support it

The patch "scsi: rescan VPD attributes" introduced a regression in which
devices that don't support VPD were being scanned for VPD attributes
anyway.  This could cause issues for this parts and should be avoided so
the check for scsi_level has been moved out of scsi_add_lun and into
scsi_attach_vpd so that all callers will not scan VPD for devices that
don't support it.

Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")

Suggested-by: Alexander Duyck 
Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/scsi.c|  3 ++-
 drivers/scsi/sd.c  | 19 +--
 include/scsi/scsi_device.h | 25 +
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b1bf42b..1deb6ad 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -784,8 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
 	int pg83_supported = 0;
 	unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
 
-	if (sdev->skip_vpd_pages)
+	if (!scsi_device_supports_vpd(sdev))
 		return;
+
 retry_pg0:
 	vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
 	if (!vpd_buf)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5451980..868d58c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2789,23 +2789,6 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
 		sdkp->ws10 = 1;
 }
 
-static int sd_try_extended_inquiry(struct scsi_device *sdp)
-{
-	/* Attempt VPD inquiry if the device blacklist explicitly calls
-	 * for it.
-	 */
-	if (sdp->try_vpd_pages)
-		return 1;
-	/*
-	 * Although VPD inquiries can go to SCSI-2 type devices,
-	 * some USB ones crash on receiving them, and the pages
-	 * we currently ask for are for SPC-3 and beyond
-	 */
-	if (sdp->scsi_level > SCSI_SPC_2 && !sdp->skip_vpd_pages)
-		return 1;
-	return 0;
-}
-
 /**
  *	sd_revalidate_disk - called the first time a new disk is seen,
  *	performs disk spin up, read_capacity, etc.
@@ -2844,7 +2827,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 	if (sdkp->media_present) {
 		sd_read_capacity(sdkp, buffer);
 
-		if (sd_try_extended_inquiry(sdp)) {
+		if (scsi_device_supports_vpd(sdp)) {
 			sd_read_block_provisioning(sdkp);
 			sd_read_block_limits(sdkp);
 			sd_read_block_characteristics(sdkp);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index a5fc682..d9aea6c 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -515,6 +515,31 @@ static inline int scsi_device_tpgs(struct scsi_device *sdev)
 	return sdev->inquiry ? (sdev->inquiry[5] >> 4) & 0x3 : 0;
 }
 
+/**
+ * scsi_device_supports_vpd - test if a device supports VPD pages
+ * @sdev: the &struct

[PATCH 2/2] scsi: Fix RCU handling for VPD pages

[Alexander Duyck]

This patch is meant to fix the RCU handling for VPD pages.  The original
code had a number of issues including the fact that the local variables
were being declared as __rcu, the RCU variable being directly accessed
outside of the RCU locked region, and the fact that length was not
associated with the data so it would be possible to get a mix and match of
the length for one VPD page with the data from another.

Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
Signed-off-by: Alexander Duyck 
---
 drivers/scsi/scsi.c|   52 +++-
 drivers/scsi/scsi_lib.c|   12 +-
 drivers/scsi/scsi_sysfs.c  |   14 +++-
 include/scsi/scsi_device.h |   14 
 4 files changed, 50 insertions(+), 42 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index ed085e78c893..143b384fd145 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -782,7 +782,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
int vpd_len = SCSI_VPD_PG_LEN;
int pg80_supported = 0;
int pg83_supported = 0;
-   unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
+   unsigned char *vpd_buf;
 
if (sdev->scsi_level < SCSI_3)
return;
@@ -816,58 +816,60 @@ retry_pg0:
vpd_len = SCSI_VPD_PG_LEN;
 
if (pg80_supported) {
+   struct scsi_vpd_pg *vpd, *orig_vpd;
 retry_pg80:
-   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
-   if (!vpd_buf)
+   vpd = kmalloc(sizeof(*vpd) + vpd_len, GFP_KERNEL);
+   if (!vpd)
return;
 
-   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x80, vpd_len);
+   result = scsi_vpd_inquiry(sdev, vpd->buf, 0x80, vpd_len);
if (result < 0) {
-   kfree(vpd_buf);
+   kfree(vpd);
return;
}
if (result > vpd_len) {
vpd_len = result;
-   kfree(vpd_buf);
+   kfree(vpd);
goto retry_pg80;
}
+   vpd->len = result;
+
mutex_lock(&sdev->inquiry_mutex);
-   orig_vpd_buf = sdev->vpd_pg80;
-   sdev->vpd_pg80_len = result;
-   rcu_assign_pointer(sdev->vpd_pg80, vpd_buf);
+   orig_vpd = rcu_dereference_protected(sdev->vpd_pg80, 1);
+   rcu_assign_pointer(sdev->vpd_pg80, vpd);
mutex_unlock(&sdev->inquiry_mutex);
-   synchronize_rcu();
-   if (orig_vpd_buf) {
-   kfree(orig_vpd_buf);
-   orig_vpd_buf = NULL;
-   }
+
+   if (orig_vpd)
+   kfree_rcu(orig_vpd, rcu);
vpd_len = SCSI_VPD_PG_LEN;
}
 
if (pg83_supported) {
+   struct scsi_vpd_pg *vpd, *orig_vpd;
 retry_pg83:
-   vpd_buf = kmalloc(vpd_len, GFP_KERNEL);
-   if (!vpd_buf)
+   vpd = kmalloc(sizeof(*vpd) + vpd_len, GFP_KERNEL);
+   if (!vpd)
return;
 
-   result = scsi_vpd_inquiry(sdev, vpd_buf, 0x83, vpd_len);
+   result = scsi_vpd_inquiry(sdev, vpd->buf, 0x83, vpd_len);
if (result < 0) {
-   kfree(vpd_buf);
+   kfree(vpd);
return;
}
if (result > vpd_len) {
vpd_len = result;
-   kfree(vpd_buf);
+   kfree(vpd);
goto retry_pg83;
}
+   vpd->len = result;
+
mutex_lock(&sdev->inquiry_mutex);
-   orig_vpd_buf = sdev->vpd_pg83;
-   sdev->vpd_pg83_len = result;
-   rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
+   orig_vpd = rcu_dereference_protected(sdev->vpd_pg83, 1);
+   rcu_assign_pointer(sdev->vpd_pg83, vpd);
mutex_unlock(&sdev->inquiry_mutex);
-   synchronize_rcu();
-   if (orig_vpd_buf)
-   kfree(orig_vpd_buf);
+
+   if (orig_vpd)
+   kfree_rcu(orig_vpd, rcu);
}
 }
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index fa6b2c4eb7a2..e44f66bc4c90 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -3175,7 +3175,7 @@ int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, 
size_t id_len)
u8 cur_id_type = 0xff;
u8 cur_id_size = 0;
unsigned char *d, *cur_id_str;
-   unsigned char __rcu *vpd_pg83;
+   struct scsi_vpd_pg *vpd_pg83;
int id_size = -EINVAL;
 
rcu_read_lock();
@@ -3205,8 +3205,8 @@ int scsi_vpd_lun_id(struct scsi_device *sdev, char *id, 
size_t id_len)
}
 
memset(id, 0, id_len);
-   d = vpd

[PATCH 1/2] scsi: Do not attach VPD to devices that don't support it

[Alexander Duyck]

The patch "scsi: rescan VPD attributes" introduced a regression in which
devices that don't support VPD were being scanned for VPD attributes
anyway.  This could cause issues for this parts and should be avoided so
the check for scsi_level has been moved out of scsi_add_lun and into
scsi_attach_vpd so that all callers will not scan VPD for devices that
don't support it.

Fixes: 09e2b0b14690 ("scsi: rescan VPD attributes")
Signed-off-by: Alexander Duyck 
---
 drivers/scsi/scsi.c  |3 +++
 drivers/scsi/scsi_scan.c |3 +--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index b1bf42b93fcc..ed085e78c893 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -784,6 +784,9 @@ void scsi_attach_vpd(struct scsi_device *sdev)
int pg83_supported = 0;
unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
 
+   if (sdev->scsi_level < SCSI_3)
+   return;
+
if (sdev->skip_vpd_pages)
return;
 retry_pg0:
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6a820668d442..1b16c89e0cf9 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -986,8 +986,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned 
char *inq_result,
}
}
 
-   if (sdev->scsi_level >= SCSI_3)
-   scsi_attach_vpd(sdev);
+   scsi_attach_vpd(sdev);
 
sdev->max_queue_depth = sdev->queue_depth;
 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: export function scsi_scan.c:sanitize_inquiry_string

[Johannes Thumshirn]

On Wed, Jan 20, 2016 at 01:41:41PM -0600, Don Brace wrote:
> The hpsa driver uses this function to cleanup inquiry
> data. Our new pqi driver will also use this
> function. This function was copied into both drivers.
> 
> This patch exports sanitize_inquiry_string so the hpsa
> and the pqi drivers can use this function directly.
> 
> Hannes recommended the change in review:
> https://www.marc.info/?l=linux-scsi&m=144619249003064&w=2
> that using the existing function in scsi_scan would be
> preferrable.
> 
> Suggested-by: Hannes Reinecke 
> Reviewed-by: Kevin Barnett 
> Reviewed-by: Justin Lindley 
> Reviewed-by: Scott Teel 
> Reviewed-by: Hannes Reinecke 
> Signed-off-by: Don Brace 
> ---
>  drivers/scsi/scsi_scan.c |   12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..1f02e84 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -518,7 +518,8 @@ void scsi_target_reap(struct scsi_target *starget)
>  }
>  
>  /**
> - * sanitize_inquiry_string - remove non-graphical chars from an INQUIRY 
> result string
> + * scsi_sanitize_inquiry_string - remove non-graphical chars from an
> + *INQUIRY result string
>   * @s: INQUIRY result string to sanitize
>   * @len: length of the string
>   *
> @@ -531,7 +532,7 @@ void scsi_target_reap(struct scsi_target *starget)
>   *   string terminator, so all the following characters are set to
>   *   spaces.
>   **/
> -static void sanitize_inquiry_string(unsigned char *s, int len)
> +void scsi_sanitize_inquiry_string(unsigned char *s, int len)
>  {
>   int terminated = 0;
>  
> @@ -542,6 +543,7 @@ static void sanitize_inquiry_string(unsigned char *s, int 
> len)
>   *s = ' ';
>   }
>  }
> +EXPORT_SYMBOL(scsi_sanitize_inquiry_string);
>  
>  /**
>   * scsi_probe_lun - probe a single LUN using a SCSI INQUIRY
> @@ -627,9 +629,9 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
> unsigned char *inq_result,
>   }
>  
>   if (result == 0) {
> - sanitize_inquiry_string(&inq_result[8], 8);
> - sanitize_inquiry_string(&inq_result[16], 16);
> - sanitize_inquiry_string(&inq_result[32], 4);
> + scsi_sanitize_inquiry_string(&inq_result[8], 8);
> + scsi_sanitize_inquiry_string(&inq_result[16], 16);
> + scsi_sanitize_inquiry_string(&inq_result[32], 4);
>  
>   response_len = inq_result[4] + 5;
>   if (response_len > 255)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/2] scsi: Fix endless loop of ATA hard resets due to VPD reads

[Alexander Duyck]

Recent changes to the kernel pulled in during the merge window have
resulted in my system generating an endless loop of the following type of
errors:

[  318.965756] ata14: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[  318.968457] ata14.00: configured for UDMA/66
[  318.970656] ata14: EH complete
[  318.984366] ata14.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6
[  318.986854] ata14.00: irq_stat 0x4001
[  318.989138] ata14.00: cmd a0/01:00:00:00:01/00:00:00:00:00/a0 tag 22 dma 
16640 in
 Inquiry 12 01 00 00 ff 00res 00/00:00:00:00:00/00:00:00:00:00/00 Emask 
0x3 (HSM violation)
[  318.995986] ata14: hard resetting link

I bisected the issue and found the patch responsible for the issue was
commit 09e2b0b14690 "scsi: rescan VPD attributes".  This commit contained
several issues.

First, the commit had changed the behavior in terms of what devices we
called scsi_attach_vpd() for.  As a result we were calling it for devices
that didn't support a scsi_level of 6, SCSI 3, so VPD accesses could
result in errors.

Second, the commit as well as a follow-on patch for it contained a number
of RCU errors.  Specifically the code was structured such that we had
accesses outside of RCU locked regions, and repeated use of the RCU
protected pointer without using the proper accessors.  As such it was
possible to get into a serious corruption situation should a pointer be
updated.

Ultimately neither of these bugs were my root cause.  It turns out the
Marvel Console SCSI device in my system needed to have a flag set to
disable VPD access in order to keep things from looping through the error
repeatedly.  In order to resolve it I had to add the kernel parameter
"scsi_mod.dev_flags=Marvell:Console:0x400".  This allowed my system to
boot without any errors, however the first two issues described above are
still relevent so I thought I would provide the patches since I had already
written them up.

---

Alexander Duyck (2):
  scsi: Do not attach VPD to devices that don't support it
  scsi: Fix RCU handling for VPD pages


 drivers/scsi/scsi.c|   55 
 drivers/scsi/scsi_lib.c|   12 +-
 drivers/scsi/scsi_scan.c   |3 +-
 drivers/scsi/scsi_sysfs.c  |   14 ++-
 include/scsi/scsi_device.h |   14 +++
 5 files changed, 54 insertions(+), 44 deletions(-)

--
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: export function scsi_scan.c:sanitize_inquiry_string

[Matthew R. Ochs]

> On Jan 20, 2016, at 1:41 PM, Don Brace  wrote:
> 
> The hpsa driver uses this function to cleanup inquiry
> data. Our new pqi driver will also use this
> function. This function was copied into both drivers.
> 
> This patch exports sanitize_inquiry_string so the hpsa
> and the pqi drivers can use this function directly.
> 
> Hannes recommended the change in review:
> https://www.marc.info/?l=linux-scsi&m=144619249003064&w=2
> that using the existing function in scsi_scan would be
> preferrable.

I also made this suggestion when reviewing the same patch:
https://www.marc.info/?l=linux-scsi&m=144613827316617&w=2

Reviewed-by: Matthew R. Ochs 

> 
> Suggested-by: Hannes Reinecke 
> Reviewed-by: Kevin Barnett 
> Reviewed-by: Justin Lindley 
> Reviewed-by: Scott Teel 
> Reviewed-by: Hannes Reinecke 
> Signed-off-by: Don Brace 
> ---
> drivers/scsi/scsi_scan.c |   12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 6a82066..1f02e84 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -518,7 +518,8 @@ void scsi_target_reap(struct scsi_target *starget)
> }
> 
> /**
> - * sanitize_inquiry_string - remove non-graphical chars from an INQUIRY 
> result string
> + * scsi_sanitize_inquiry_string - remove non-graphical chars from an
> + *INQUIRY result string
>  * @s: INQUIRY result string to sanitize
>  * @len: length of the string
>  *
> @@ -531,7 +532,7 @@ void scsi_target_reap(struct scsi_target *starget)
>  *string terminator, so all the following characters are set to
>  *spaces.
>  **/
> -static void sanitize_inquiry_string(unsigned char *s, int len)
> +void scsi_sanitize_inquiry_string(unsigned char *s, int len)
> {
>   int terminated = 0;
> 
> @@ -542,6 +543,7 @@ static void sanitize_inquiry_string(unsigned char *s, int 
> len)
>   *s = ' ';
>   }
> }
> +EXPORT_SYMBOL(scsi_sanitize_inquiry_string);
> 
> /**
>  * scsi_probe_lun - probe a single LUN using a SCSI INQUIRY
> @@ -627,9 +629,9 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
> unsigned char *inq_result,
>   }
> 
>   if (result == 0) {
> - sanitize_inquiry_string(&inq_result[8], 8);
> - sanitize_inquiry_string(&inq_result[16], 16);
> - sanitize_inquiry_string(&inq_result[32], 4);
> + scsi_sanitize_inquiry_string(&inq_result[8], 8);
> + scsi_sanitize_inquiry_string(&inq_result[16], 16);
> + scsi_sanitize_inquiry_string(&inq_result[32], 4);
> 
>   response_len = inq_result[4] + 5;
>   if (response_len > 255)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: NULL pointer dereference: IP: [] sr_runtime_suspend+0xc/0x20 [sr_mod]

[Alexandre Rossi]

Hi,

>> Could you please attach the debugging patch. Hopefully Alexandre, Erich,
>> or I will have some spare time to build an image from it.
>
> Actually, this patch is an attempt at a fix.  After looking more
> carefully at your log pictures, I realized what the problem must be.
>
> It's too bad nobody was able to capture a log where the error
> occurred in sr_runtime_suspend, though -- all the logs in the bug
> report show sd_runtime_resume.

I just tested the patch applied on top of 4.3.3 (4.3.3-6 in Debian).

It still crashes at boot, but the stacktrace is different : it happens
in blk_post_runtime_resume . Maybe I'm bit by a different bug or maybe
the I need to try with 4.4.

I'll post the captured log when I have access to a wired network. I'd
be happy to provide the logs of a debugging patch.

Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl

[Sebastian Herbszt]

Johannes Thumshirn wrote:
> This removes a redundant code block that will either be executed if the
> ENABLE_FCP_RING_POLLING flag is set in phba->cfg_poll or not. The code is just
> duplicated in both cases, hence we unify it again.
> 
> This probably is a left over from some sort of refactoring.
> 
> Signed-off-by: Johannes Thumshirn 

Reviewed-by: Sebastian Herbszt 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] qla2xxx: Fix warning reported by static checker

[Nicholas A. Bellinger]

Hi Himanshu,

On Wed, 2016-01-20 at 15:11 -0500, Himanshu Madhani wrote:
> This patch fixes following warning
> 
> drivers/scsi/qla2xxx/qla_target.c:3587 qlt_do_ctio_completion()
> warn: impossible condition '(logged_out == 41) => (0-1 == 41)'
> 
> drivers/scsi/qla2xxx/qla_target.c
>  3580  case CTIO_PORT_LOGGED_OUT:
>  3581  case CTIO_PORT_UNAVAILABLE:
>  3582  {
>  3583  bool logged_out = (status & 0x);
>  3584  ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
>  3585"qla_target(%d): CTIO with %s status %x "
>  3586  "received (state %x, se_cmd %p)\n", 
> vha->vp_idx,
>  3587  (logged_out == CTIO_PORT_LOGGED_OUT) ?
>   ^^
>   Bool cannot equal 0x26.
>  3588"PORT LOGGED OUT" : "PORT UNAVAILABLE",
> 
> Reported-by: Dan Carpenter 
> Signed-off-by: Himanshu Madhani 
> Signed-off-by: Giridhar Malavali 
> ---
>  drivers/scsi/qla2xxx/qla_target.c |7 ---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_target.c 
> b/drivers/scsi/qla2xxx/qla_target.c
> index 80651cd..829b534 100644
> --- a/drivers/scsi/qla2xxx/qla_target.c
> +++ b/drivers/scsi/qla2xxx/qla_target.c
> @@ -3597,12 +3597,13 @@ static void qlt_do_ctio_completion(struct 
> scsi_qla_host *vha, uint32_t handle,
>   case CTIO_PORT_LOGGED_OUT:
>   case CTIO_PORT_UNAVAILABLE:
>   {
> - bool logged_out = (status & 0x);
> + bool logged_out =
> + (status & 0x) == CTIO_PORT_LOGGED_OUT;
> +
>   ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
>   "qla_target(%d): CTIO with %s status %x "
>   "received (state %x, se_cmd %p)\n", vha->vp_idx,
> - (logged_out == CTIO_PORT_LOGGED_OUT) ?
> - "PORT LOGGED OUT" : "PORT UNAVAILABLE",
> + logged_out ? "PORT LOGGED OUT" : "PORT UNAVAILABLE",
>   status, cmd->state, se_cmd);
>  
>   if (logged_out && cmd->sess) {

I picked up Arnd's patch earlier this morning, and will include this
better version in the post -rc1 fixes PULL request.



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[GIT PULL] target updates for v4.5-rc1

[Nicholas A. Bellinger]

Hello Linus,

Here are target-pending updates for v4.5-rc1 code.

Please go ahead and PULL from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next

Note that Sagi + Jenny's series for iser target + initiator remote
invalidation is being merged via rdma.git, and Doug will be sending
in his rmda.git PULL request.

As per SFR, you'll (eventually) hit a minor iser-target merge conflict
vs. rdma.git once you PULL from Doug due to a ERR_PTR bug-fix:

http://marc.info/?l=linux-next&m=145222770105861&w=2

The highlights this round include:

 - Introduce configfs support for unlocked configfs_depend_item()
   (krzysztof + andrezej)
 - Conversion of usb-gadget target driver to new function registration 
   interface (andrzej + sebastian)
 - Enable qla2xxx FC target mode support for Extended Logins
   (himansu + giridhar)
 - Enable qla2xxx FC target mode support for Exchange Offload
   (himansu + giridhar)
 - Add qla2xxx FC target mode irq affinity notification +
   selective command queuing.  (quinn + himanshu)
 - Fix iscsi-target deadlock in se_node_acl configfs deletion
   (sagi + nab)
 - Convert se_node_acl configfs deletion + se_node_acl->queue_depth
   to proper se_session->sess_kref + target_get_session() usage.
   (hch + sagi + nab)
 - Fix long-standing race between se_node_acl->acl_kref get and
   get_initiator_node_acl() lookup.  (hch + nab)
 - Fix target/user block-size handling, and make sure netlink reaches
   all network namespaces (sheng + andy)

Note there is an outstanding bug-fix series for remote I_T nexus port
TMR LUN_RESET has been posted and still being tested, and will likely
become post -rc1 material at this point.

Thank you,

--nab

Alexei Potashnik (2):
  qla2xxx: Delete session if initiator is gone from FW
  qla2xxx: Wait for all conflicts before ack'ing PLOGI

Andrzej Pietrasiewicz (11):
  usb: gadget: tcm: split string definitions into function and device
  usb: gadget: tcm: follow naming conventions
  usb: gadget: tcm: use strtobool for a boolean value
  usb: gadget: tcm: simplify attribute store function
  usb: gadget: tcm: factor out f_tcm
  usb: gadget: f_tcm: convert to new function interface with backward
compatibility
  usb: gadget: tcm: convert to use new function registration interface
  usb: gadget: f_tcm: remove compatibility layer
  usb: gadget: f_tcm: remove redundant singleton
  usb: gadget: f_tcm: use usb_gstrings_attach
  usb: gadget: f_tcm: add configfs support

Andy Grover (1):
  target/fcoe: Add tag support to tcm_fc

Arnd Bergmann (1):
  scsi: qla2: avoid type mismatch in comparison

Bart Van Assche (9):
  target: Fix spelling + remove set-but-not-used variables
  iscsi-target: Fix indentation + spelling + unreachable code
  sbp-target: Remove a superfluous forward declaration
  target: Fix indentation in target_core_configfs.c
  target: Remove an unused variable
  usb/gadget: Remove set-but-not-used variables
  target: Support aborting tasks with a 64-bit tag
  target: Fix a memory leak in target_dev_lba_map_store()
  tcm_fc: Wait for command completion before freeing a session

Christophe Vu-Brugier (1):
  target: fix deprecated attribute names in dmesg

Dilip Kumar Uppugandla (1):
  qla2xxx: Check for online flag instead of active reset when
transmitting responses

Geliang Tang (1):
  target: use offset_in_page macro

Himanshu Madhani (4):
  qla2xxx: Enable Extended Logins support
  qla2xxx: Enable Exchange offload support.
  qla2xxx: Enable Target counters in DebugFS.
  qla2xxx: Added interface to send explicit LOGO.

Jamie Pocas (1):
  target/sbc: Add LBPRZ attribute + control CDB emulation

Krzysztof Opasiak (4):
  fs: configfs: Drop unused parameter from configfs_undepend_item()
  fs: configfs: Factor out configfs_do_depend_item()
  fs: configfs: Factor out configfs_find_subsys_dentry()
  fs: configfs: Add unlocked version of configfs_depend_item()

Nicholas Bellinger (9):
  tcm_usb_gadget: Don't strip off nexus WWPN prefix
  tcm_usb_gadget: Fix nexus leak
  tcm_usb_gadget: Fix enabled attribute failure
  iser-target: Fix non negative ERR_PTR isert_device_get usage
  tcm_fc: Convert acl lookup to modern get_initiator_node_acl usage
  ib_srpt: Convert acl lookup to modern get_initiator_node_acl usage
  iscsi-target: Fix potential dead-lock during node acl delete
  target: Convert ACL change queue_depth se_session reference usage
  target: Obtain se_node_acl->acl_kref during get_initiator_node_acl

Quinn Tran (8):
  qla2xxx: Add FW resource count in DebugFS.
  qla2xxx: Replace QLA_TGT_STATE_ABORTED with a bit.
  qla2xxx: Remove dependency on hardware_lock to reduce lock contention.
  qla2xxx: Add irq affinity notification
  qla2xxx: Add selective command queuing
  qla2xxx: Move atioq to a different lock to reduce lock contention
  qla2xxx: Disable ZIO at start time.
  qla2xxx: Set all queues to 4k

Sheng Yang (3):
  tcm_loop: Show address of tpg in configfs
  target/user: Allow user to set block size before enabling devi

[PATCH] qla2xxx: Fix warning reported by static checker

[Himanshu Madhani]

This patch fixes following warning

drivers/scsi/qla2xxx/qla_target.c:3587 qlt_do_ctio_completion()
warn: impossible condition '(logged_out == 41) => (0-1 == 41)'

drivers/scsi/qla2xxx/qla_target.c
 3580  case CTIO_PORT_LOGGED_OUT:
 3581  case CTIO_PORT_UNAVAILABLE:
 3582  {
 3583  bool logged_out = (status & 0x);
 3584  ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
 3585  "qla_target(%d): CTIO with %s status %x "
 3586  "received (state %x, se_cmd %p)\n", 
vha->vp_idx,
 3587  (logged_out == CTIO_PORT_LOGGED_OUT) ?
^^
Bool cannot equal 0x26.
 3588  "PORT LOGGED OUT" : "PORT UNAVAILABLE",

Reported-by: Dan Carpenter 
Signed-off-by: Himanshu Madhani 
Signed-off-by: Giridhar Malavali 
---
 drivers/scsi/qla2xxx/qla_target.c |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index 80651cd..829b534 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3597,12 +3597,13 @@ static void qlt_do_ctio_completion(struct scsi_qla_host 
*vha, uint32_t handle,
case CTIO_PORT_LOGGED_OUT:
case CTIO_PORT_UNAVAILABLE:
{
-   bool logged_out = (status & 0x);
+   bool logged_out =
+   (status & 0x) == CTIO_PORT_LOGGED_OUT;
+
ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
"qla_target(%d): CTIO with %s status %x "
"received (state %x, se_cmd %p)\n", vha->vp_idx,
-   (logged_out == CTIO_PORT_LOGGED_OUT) ?
-   "PORT LOGGED OUT" : "PORT UNAVAILABLE",
+   logged_out ? "PORT LOGGED OUT" : "PORT UNAVAILABLE",
status, cmd->state, se_cmd);
 
if (logged_out && cmd->sess) {
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH V3 8/9] aacraid: Fix character device re-initialization

[Raghava Aditya Renukunta]

Hello Tomas,

> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Wednesday, January 20, 2016 5:41 AM
> To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com;
> martin.peter...@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com;
> zzzDavid Carroll
> Subject: Re: [PATCH V3 8/9] aacraid: Fix character device re-initialization
> 
> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta 
> >
> > During EEH PCI hotplug activity kernel unloads and loads the driver,
> > causing character device to be unregistered(aac_remove_one).When the
> > driver is loaded back using aac_probe_one the character device needs
> > to be registered again for the AIF management tools to work.
> >
> > Fixed by adding code to register character device in aac_probe_one if
> > it is unregistered in aac_remove_one.
> >
> > Changes in V2:
> > Added macros to track character device state
> >
> > Changes in V3:
> > None
> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> > Reviewed-by: Shane Seymour 
> 
> Hi Raghava,
> when aacraid is loaded (modprobe) without an controller attached to the
> system
> the driver loads and creates the character device. Later when you hotplug a
> device and remove again we see the driver loaded but now without the
> char device. I'd prefer consistency here - either create the char device
> when the first controller is probed (preferred) or do not remove it
> until the driver exits.
> This is not a nack, just a wish that you changed it in next series.
> 
> --tm


Yes I will make the necessary changes  so that character device is created when
The controller is probed, and when the driver is removed 
(aac_remove_one),delete 
the character device. I will keep the character device during resume and 
suspend.

Do you want to do this in the next version of the patches or the next series of 
patches after this one is 
Accepted. ?
 
Regards,
Raghava Aditya


> > ---
> >  drivers/scsi/aacraid/aacraid.h |  7 +++
> >  drivers/scsi/aacraid/linit.c   | 21 ++---
> >  2 files changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 3473668..4b669ef 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -94,6 +94,13 @@ enum {
> >  #define aac_phys_to_logical(x)  ((x)+1)
> >  #define aac_logical_to_phys(x)  ((x)?(x)-1:0)
> >
> > +/*
> > + * These macros are for keeping track of
> > + * character device state.
> > + */
> > +#define AAC_CHARDEV_UNREGISTERED   (-1)
> > +#define AAC_CHARDEV_NEEDS_REINIT   (-2)
> > +
> >  /* #define AAC_DETAILED_STATUS_INFO */
> >
> >  struct diskparm
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index 27b3fcd..057c07c 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -80,7 +80,7 @@ MODULE_VERSION(AAC_DRIVER_FULL_VERSION);
> >
> >  static DEFINE_MUTEX(aac_mutex);
> >  static LIST_HEAD(aac_devices);
> > -static int aac_cfg_major = -1;
> > +static int aac_cfg_major = AAC_CHARDEV_UNREGISTERED;
> >  char aac_driver_version[] = AAC_DRIVER_FULL_VERSION;
> >
> >  /*
> > @@ -1125,6 +1125,13 @@ static void __aac_shutdown(struct aac_dev *
> aac)
> > else if (aac->max_msix > 1)
> > pci_disable_msix(aac->pdev);
> >  }
> > +static void aac_init_char(void)
> > +{
> > +   aac_cfg_major = register_chrdev(0, "aac", &aac_cfg_fops);
> > +   if (aac_cfg_major < 0) {
> > +   pr_err("aacraid: unable to register \"aac\" device.\n");
> > +   }
> > +}
> >
> >  static int aac_probe_one(struct pci_dev *pdev, const struct pci_device_id
> *id)
> >  {
> > @@ -1182,6 +1189,9 @@ static int aac_probe_one(struct pci_dev *pdev,
> const struct pci_device_id *id)
> > shost->max_cmd_len = 16;
> > shost->use_cmd_list = 1;
> >
> > +   if (aac_cfg_major == AAC_CHARDEV_NEEDS_REINIT)
> > +   aac_init_char();
> > +
> > aac = (struct aac_dev *)shost->hostdata;
> > aac->base_start = pci_resource_start(pdev, 0);
> > aac->scsi_host_ptr = shost;
> > @@ -1519,7 +1529,7 @@ static void aac_remove_one(struct pci_dev
> *pdev)
> > pci_disable_device(pdev);
> > if (list_empty(&aac_devices)) {
> > unregister_chrdev(aac_cfg_major, "aac");
> > -   aac_cfg_major = -1;
> > +   aac_cfg_major = AAC_CHARDEV_NEEDS_REINIT;
> > }
> >  }
> >
> > @@ -1681,11 +1691,8 @@ static int __init aac_init(void)
> > if (error < 0)
> > return error;
> >
> > -   aac_cfg_major = register_chrdev( 0, "aac", &aac_cfg_fops);
> > -   if (aac_cfg_major < 0) {
> > -   printk(KERN_WARNING
> > -   "aacraid: unable to register \"aac\" device.\n");
> > -   }
> > +   aac_init_char();
> > +
> >
> > return 0;
> >  }

--
To unsubscribe from this 

RE: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET

[Raghava Aditya Renukunta]

> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Tuesday, January 19, 2016 8:14 AM
> To: Raghava Aditya Renukunta; james.bottom...@hansenpartnership.com;
> martin.peter...@oracle.com; linux-scsi@vger.kernel.org
> Cc: Mahesh Rajashekhara; Murthy Bhat; Gana Sridaran; aacraid@pmc-
> sierra.com; Scott Benesh; jthumsh...@suse.de; shane.seym...@hpe.com;
> David Carroll
> Subject: Re: [PATCH V3 7/9] aacraid: Fix AIF triggered IOP_RESET
> 
> On 15.1.2016 08:16, Raghava Aditya Renukunta wrote:
> > From: Raghava Aditya Renukunta 
> >
> > while driver removal is in progress or PCI shutdown is invoked, driver
> > kills AIF aacraid thread, but IOCTL requests from the management tools
> > re-start AIF thread leading to IOP_RESET.
> >
> > Fixed by setting adapter_shutdown flag when PCI shutdown is invoked.
> >
> > Changes in V2:
> > Set adapter_shutdown flag before shutdown command is sent to \
> > controller
> >
> > Changes in V3:
> > Call aac_send_shut_shutdown first thing in __aac_shutdown
> > Convert adapter_shutdown to atomic_t variable to prevent \
> > SMP coherency issues(race conditions)
> 
> Hi,
> I don't think that an atomic variable can change it, imagine that
> thread just passed your test in aac_cfg_ioctl and another thread
> enter a bit later the adapter_shutdown so both - an I/O and your shutdown
> code
> will run in parallel.
> Also you need to fix your compat_ioctl path too.
> 
> --tm

Hello Tomas,
Yes that would not solve this problem.
I can think of 2 solutions

1.Place a delay after setting adapter_shutdown and before sending the actual
shutdown command in aac_send_shutdown. This way any impending commands will be 
processed before the adapter  actually receives the shutdown command. Since 
management 
commands are sync only , shutdown command will be sent last. This solution uses 
an
estimation of the delay

2.Since  commands are sync , place a check in aac_fib_send to block 
 commands once adapter_shutdown is set(only shutdown command will be sent thru)

I am more inclined to go with option 2.

Regards,
Raghava Aditya

> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> > ---
> >  drivers/scsi/aacraid/aacraid.h  |  2 +-
> >  drivers/scsi/aacraid/comminit.c |  6 +++---
> >  drivers/scsi/aacraid/linit.c| 15 ---
> >  3 files changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h
> > index 2916288..3473668 100644
> > --- a/drivers/scsi/aacraid/aacraid.h
> > +++ b/drivers/scsi/aacraid/aacraid.h
> > @@ -1234,7 +1234,7 @@ struct aac_dev
> > int msi_enabled;/* MSI/MSI-X enabled */
> > struct msix_entry   msixentry[AAC_MAX_MSIX];
> > struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */
> > -   u8  adapter_shutdown;
> > +   atomic_tadapter_shutdown;
> > u32 handle_pci_error;
> >  };
> >
> > diff --git a/drivers/scsi/aacraid/comminit.c
> b/drivers/scsi/aacraid/comminit.c
> > index 0e954e3..d361673 100644
> > --- a/drivers/scsi/aacraid/comminit.c
> > +++ b/drivers/scsi/aacraid/comminit.c
> > @@ -212,8 +212,9 @@ int aac_send_shutdown(struct aac_dev * dev)
> > return -ENOMEM;
> > aac_fib_init(fibctx);
> >
> > -   cmd = (struct aac_close *) fib_data(fibctx);
> > +   atomic_set(&dev->adapter_shutdown, 1);
> >
> > +   cmd = (struct aac_close *) fib_data(fibctx);
> > cmd->command = cpu_to_le32(VM_CloseAll);
> > cmd->cid = cpu_to_le32(0xfffe);
> >
> > @@ -229,7 +230,6 @@ int aac_send_shutdown(struct aac_dev * dev)
> > /* FIB should be freed only after getting the response from the F/W
> */
> > if (status != -ERESTARTSYS)
> > aac_fib_free(fibctx);
> > -   dev->adapter_shutdown = 1;
> > if ((dev->pdev->device == PMC_DEVICE_S7 ||
> >  dev->pdev->device == PMC_DEVICE_S8 ||
> >  dev->pdev->device == PMC_DEVICE_S9) &&
> > @@ -468,7 +468,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev
> *dev)
> > }
> > dev->max_msix = 0;
> > dev->msi_enabled = 0;
> > -   dev->adapter_shutdown = 0;
> > +   atomic_set(&dev->adapter_shutdown, 0);
> > if ((!aac_adapter_sync_cmd(dev,
> GET_COMM_PREFERRED_SETTINGS,
> >   0, 0, 0, 0, 0, 0,
> >   status+0, status+1, status+2, status+3, status+4))
> > diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> > index 6944560..27b3fcd 100644
> > --- a/drivers/scsi/aacraid/linit.c
> > +++ b/drivers/scsi/aacraid/linit.c
> > @@ -706,7 +706,7 @@ static long aac_cfg_ioctl(struct file *file,
> > int ret;
> > struct aac_dev *aac;
> > aac = (struct aac_dev *)file->private_data;
> > -   if (!capable(CAP_SYS_RAWIO) || aac->adapter_shutdown)
> > +   if (!capable(CAP_SYS_RAWIO) || atomic_read(&aac-
> >adapter_shutdown))
> > return -EPERM;
> > mutex_lock(&aac_mutex);
> > ret = aac_do_ioctl(file->private_data, cmd, (void __user *)arg);
> > @

Re: [PATCHv4 1/1] SCSI: hosts: update to use ida_simple for host_no management

[Lee Duncan]

On 01/05/2016 03:53 PM, Martin K. Petersen wrote:

"Lee" == Lee Duncan  writes:


Lee> Do you need me to resubmit this patch now that it's accepted?

Please resend.

Thanks!



Done, submitted against scsi tree, misc branch.
--
Lee Duncan

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] SCSI: usd ida for host number management

[Lee Duncan]

Update the SCSI hosts module to use ida to manage its
host_no index instead of an ATOMIC integer. This means
that the SCSI host number will now be reclaimable.

Use the ida "simple" mechanism, since there should be no
need for a separate spin lock current usage. Ida was chosen
over idr because the hosts module already has its
own instance and locking mechanisms that aren't easily
changed.

Changes from v1:
* First version used regular ida routines

Reviewed-by: Hannes Reinecke 
Reviewed-by: Johannes Thumshirn 
Signed-off-by: Lee Duncan 
---
 drivers/scsi/hosts.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 82ac1cd818ac..94025c5cf797 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -33,7 +33,7 @@
 #include 
 #include 
 #include 
-
+#include 
 #include 
 #include 
 #include 
@@ -42,7 +42,7 @@
 #include "scsi_logging.h"
 
 
-static atomic_t scsi_host_next_hn = ATOMIC_INIT(0);/* host_no for next new 
host */
+static DEFINE_IDA(host_index_ida);
 
 
 static void scsi_host_cls_release(struct device *dev)
@@ -355,6 +355,8 @@ static void scsi_host_dev_release(struct device *dev)
 
kfree(shost->shost_data);
 
+   ida_simple_remove(&host_index_ida, shost->host_no);
+
if (parent)
put_device(parent);
kfree(shost);
@@ -388,6 +390,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 {
struct Scsi_Host *shost;
gfp_t gfp_mask = GFP_KERNEL;
+   int index;
 
if (sht->unchecked_isa_dma && privsize)
gfp_mask |= __GFP_DMA;
@@ -406,11 +409,11 @@ struct Scsi_Host *scsi_host_alloc(struct 
scsi_host_template *sht, int privsize)
init_waitqueue_head(&shost->host_wait);
mutex_init(&shost->scan_mutex);
 
-   /*
-* subtract one because we increment first then return, but we need to
-* know what the next host number was before increment
-*/
-   shost->host_no = atomic_inc_return(&scsi_host_next_hn) - 1;
+   index = ida_simple_get(&host_index_ida, 0, 0, GFP_KERNEL);
+   if (index < 0)
+   goto fail_kfree;
+   shost->host_no = index;
+
shost->dma_channel = 0xff;
 
/* These three are default values which can be overridden */
@@ -495,7 +498,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
shost_printk(KERN_WARNING, shost,
"error handler thread failed to spawn, error = %ld\n",
PTR_ERR(shost->ehandler));
-   goto fail_kfree;
+   goto fail_index_remove;
}
 
shost->tmf_work_q = alloc_workqueue("scsi_tmf_%d",
@@ -511,6 +514,8 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template 
*sht, int privsize)
 
  fail_kthread:
kthread_stop(shost->ehandler);
+ fail_index_remove:
+   ida_simple_remove(&host_index_ida, shost->host_no);
  fail_kfree:
kfree(shost);
return NULL;
@@ -606,6 +611,7 @@ int scsi_init_hosts(void)
 void scsi_exit_hosts(void)
 {
class_unregister(&shost_class);
+   ida_destroy(&host_index_ida);
 }
 
 int scsi_is_host_device(const struct device *dev)
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: export function scsi_scan.c:sanitize_inquiry_string

[Don Brace]

The hpsa driver uses this function to cleanup inquiry
data. Our new pqi driver will also use this
function. This function was copied into both drivers.

This patch exports sanitize_inquiry_string so the hpsa
and the pqi drivers can use this function directly.

Hannes recommended the change in review:
https://www.marc.info/?l=linux-scsi&m=144619249003064&w=2
that using the existing function in scsi_scan would be
preferrable.

Suggested-by: Hannes Reinecke 
Reviewed-by: Kevin Barnett 
Reviewed-by: Justin Lindley 
Reviewed-by: Scott Teel 
Reviewed-by: Hannes Reinecke 
Signed-off-by: Don Brace 
---
 drivers/scsi/scsi_scan.c |   12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6a82066..1f02e84 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -518,7 +518,8 @@ void scsi_target_reap(struct scsi_target *starget)
 }
 
 /**
- * sanitize_inquiry_string - remove non-graphical chars from an INQUIRY result 
string
+ * scsi_sanitize_inquiry_string - remove non-graphical chars from an
+ *INQUIRY result string
  * @s: INQUIRY result string to sanitize
  * @len: length of the string
  *
@@ -531,7 +532,7 @@ void scsi_target_reap(struct scsi_target *starget)
  * string terminator, so all the following characters are set to
  * spaces.
  **/
-static void sanitize_inquiry_string(unsigned char *s, int len)
+void scsi_sanitize_inquiry_string(unsigned char *s, int len)
 {
int terminated = 0;
 
@@ -542,6 +543,7 @@ static void sanitize_inquiry_string(unsigned char *s, int 
len)
*s = ' ';
}
 }
+EXPORT_SYMBOL(scsi_sanitize_inquiry_string);
 
 /**
  * scsi_probe_lun - probe a single LUN using a SCSI INQUIRY
@@ -627,9 +629,9 @@ static int scsi_probe_lun(struct scsi_device *sdev, 
unsigned char *inq_result,
}
 
if (result == 0) {
-   sanitize_inquiry_string(&inq_result[8], 8);
-   sanitize_inquiry_string(&inq_result[16], 16);
-   sanitize_inquiry_string(&inq_result[32], 4);
+   scsi_sanitize_inquiry_string(&inq_result[8], 8);
+   scsi_sanitize_inquiry_string(&inq_result[16], 16);
+   scsi_sanitize_inquiry_string(&inq_result[32], 4);
 
response_len = inq_result[4] + 5;
if (response_len > 255)

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi update

[Don Brace]

This patch is based on Linus tree

The change is:
 - export sanitize_inquiry_string

---

Don Brace (1):
  scsi: export function scsi_scan.c:sanitize_inquiry_string


 drivers/scsi/scsi_scan.c |   12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

--
Signature
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

UFS: MPHY configuration issue

[Joao Pinto]

Hi,

I am trying to configure my MPHY through UNIPRO but I am experiencing a kernel
crash when executing a dme instruction:

As an example I am executing the following:

result = ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(PA_RXHSUNTERMCAP, 0), 1);

In ufshcd_dme_set_attr() I have the following data ready to execute:

set = dme-set
uic_cmd.command:2
uic_cmd.argument1 = 15a5
uic_cmd.argument2 = 0
uic_cmd.argument3 = 1

And then the ufshcd_send_uic_cmd() procedure originates a crash as you can see
below.

Crash log:

Path: /bin/busybox
CPU: 0 PID: 97 Comm: modprobe Not tainted 4.4.0-rc1 #26
task: 9f1f9c00 ti: 9f1ba000 task.ti: 9f1ba000

[ECR   ]: 0x00220200 => Invalid Write @ 0x by insn @ 0x80a8a8c2
[EFA   ]: 0x
[BLINK ]: ufshcd_release+0x14a2/0x1738 [ufshcd]
[ERET  ]: __mutex_lock_slowpath+0x46/0x100
[STAT32]: 0x080e : K   A1 E2 E1
BTA: 0x70054f4e  SP: 0x9f1bbc64  FP: 0x
LPS: 0x808765b8 LPE: 0x808765bc LPC: 0x
r00: 0x9f2b5b50 r01: 0x9f1bbcb4 r02: 0x
r03: 0x9f1bbc64 r04: 0x r05: 0x0002
r06: 0x6affd617 r07: 0x r08: 0x6affd617
r09: 0x r10: 0x0008 r11: 0x403f
r12: 0x

Stack Trace:
  __mutex_lock_slowpath+0x46/0x100
  ufshcd_release+0x14a2/0x1738 [ufshcd]

I would like to hear some opinions about this issue!

Thank you very much!

BR
Joao

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[Christian Borntraeger]

On 01/20/2016 05:01 PM, Martin K. Petersen wrote:
> Commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length
> limits") accidentally switched optimal I/O size reporting from bytes to
> block layer sectors.
> 
> Signed-off-by: Martin K. Petersen 
> Reported-by: Christian Borntraeger 
> ---
>  drivers/scsi/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 4e08d1cd704d..ec163d08f6c3 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2893,7 +2893,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>   sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
>   sdkp->opt_xfer_blocks * sdp->sector_size >= PAGE_CACHE_SIZE)
>   rw_max = q->limits.io_opt =
> - logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
> + sdkp->opt_xfer_blocks * sdp->sector_size;
>   else
>   rw_max = BLK_DEF_MAX_SECTORS;
> 

the error message is gone

Tested-by: Christian Borntraeger 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[Ewan Milne]

On Wed, 2016-01-20 at 11:01 -0500, Martin K. Petersen wrote:
> Commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length
> limits") accidentally switched optimal I/O size reporting from bytes to
> block layer sectors.
> 
> Signed-off-by: Martin K. Petersen 
> Reported-by: Christian Borntraeger 
> ---
>  drivers/scsi/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 4e08d1cd704d..ec163d08f6c3 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2893,7 +2893,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>   sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
>   sdkp->opt_xfer_blocks * sdp->sector_size >= PAGE_CACHE_SIZE)
>   rw_max = q->limits.io_opt =
> - logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
> + sdkp->opt_xfer_blocks * sdp->sector_size;
>   else
>   rw_max = BLK_DEF_MAX_SECTORS;
>  

Reviewed-by: Ewan D. Milne 


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[James Bottomley]

On Wed, 2016-01-20 at 11:40 -0500, Martin K. Petersen wrote:
> > > > > > "James" == James Bottomley <
> > > > > > james.bottom...@hansenpartnership.com> writes:
> 
> James> We should mark the commit causing the problems, which went
> into
> James> 4.4 if I remember correctly:
> 
> James> Fixes: ca369d51b3e1649be4a72addd6d6a168cfb3f537 Cc:
> James> sta...@vger.kernel.org # 4.4+ Reviewed-by: James E.J.
> Bottomley
> James> 
> 
> I'll add the tags. The reason I didn't explicitly put 4.4+ is that 
> the original commit has made its way quite far in various stable 
> trees by now.

It has?  It wasn't tagged for stable.  However, if it got applied to
stable trees, then we should certainly backport further.  I sort of
hope the stable process uses the Fixes: tag to decide when to backport
anyway, since the stable commit contains the original upstream sha256,
they can certainly identify it.

Greg, are we OK not to bother with qualifying the cc: stable tag if we
have a Fixes tag, or do you still want to see both?  Perhaps an
addition to stable_kernel_rules.txt mentioning Fixes: might be useful
as well.

Thanks,

James

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[Martin K. Petersen]

> "James" == James Bottomley  writes:

James> We should mark the commit causing the problems, which went into
James> 4.4 if I remember correctly:

James> Fixes: ca369d51b3e1649be4a72addd6d6a168cfb3f537 Cc:
James> sta...@vger.kernel.org # 4.4+ Reviewed-by: James E.J. Bottomley
James> 

I'll add the tags. The reason I didn't explicitly put 4.4+ is that the
original commit has made its way quite far in various stable trees by
now.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[Matthew R. Ochs]

Reviewed-by: Matthew R. Ochs  

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: Fix race on command completion

[Tomas Henzl]

On 15.1.2016 10:48, Hannes Reinecke wrote:
> Upon command completion the lpfc driver would call ->done()
> on the scsi command before taking the host lock and
> releasing the command internally.
> This opens up a race window there this command might be re-used
> after ->done(), leading to a double completion on the same command.

I agree that a driver should clean up the command before calling
->done, but this driver uses a list based system where a command 
can't be reused only until it was returned to the list,
so I don't understand how a 'done' before internal free could
cause an issue other than a failed lpfc_get_scsi_buf in .queuecommand.
Is your issue related to the abort_handler 
(maybe cmd->host_scribble = NULL; changes the abort handler flow)?

Tomas 

>
> This patch takes the host lock before accessing the scsi command,
> and disconnect the internal command under the same lock.
>
> Signed-off-by: Hannes Reinecke 
> ---
>  drivers/scsi/lpfc/lpfc_scsi.c | 17 -
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index 4679ed4..974af28 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -3908,9 +3908,16 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct 
> lpfc_iocbq *pIocbIn,
>   uint32_t logit = LOG_FCP;
>  
>   /* Sanity check on return of outstanding command */
> - if (!(lpfc_cmd->pCmd))
> + spin_lock_irqsave(&phba->hbalock, flags);
> + if (!(lpfc_cmd->pCmd)) {
> + spin_unlock_irqrestore(&phba->hbalock, flags);
>   return;
> + }
>   cmd = lpfc_cmd->pCmd;
> + cmd->host_scribble = NULL;
> + lpfc_cmd->pCmd = NULL;
> + spin_unlock_irqrestore(&phba->hbalock, flags);
> +
>   shost = cmd->device->host;
>  
>   lpfc_cmd->result = (pIocbOut->iocb.un.ulpWord[4] & IOERR_PARAM_MASK);
> @@ -4125,10 +4132,6 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct 
> lpfc_iocbq *pIocbIn,
>   cmd->scsi_done(cmd);
>  
>   if (phba->cfg_poll & ENABLE_FCP_RING_POLLING) {
> - spin_lock_irqsave(&phba->hbalock, flags);
> - lpfc_cmd->pCmd = NULL;
> - spin_unlock_irqrestore(&phba->hbalock, flags);
> -
>   /*
>* If there is a thread waiting for command completion
>* wake up the thread.
> @@ -4141,10 +4144,6 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct 
> lpfc_iocbq *pIocbIn,
>   return;
>   }
>  
> - spin_lock_irqsave(&phba->hbalock, flags);
> - lpfc_cmd->pCmd = NULL;
> - spin_unlock_irqrestore(&phba->hbalock, flags);
> -
>   /*
>* If there is a thread waiting for command completion
>* wake up the thread.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] SCSI: fix crashes in sd and sr runtime PM

[Alan Stern]

Runtime suspend during driver probe and removal can cause problems.
The driver's runtime_suspend or runtime_resume callbacks may invoked
before the driver has finished binding to the device or after the
driver has unbound from the device.

This problem shows up with the sd and sr drivers, and can cause disk
or CD/DVD drives to become unusable as a result.  The fix is simple.
The drivers store a pointer to the scsi_disk or scsi_cd structure as
their private device data when probing is finished, so we simply have
to be sure to clear the private data during removal and test it during
runtime suspend/resume.

This fixes .

Signed-off-by: Alan Stern 
Reported-by: Paul Menzel 
Reported-by: Erich Schubert 
Reported-by: Alexandre Rossi 
Tested-by: Paul Menzel 
CC: "James E.J. Bottomley" 
CC: Ben Hutchings 
CC: 

---


[as1795]


 drivers/scsi/sd.c |7 +--
 drivers/scsi/sr.c |4 
 2 files changed, 9 insertions(+), 2 deletions(-)

Index: usb-4.4/drivers/scsi/sd.c
===
--- usb-4.4.orig/drivers/scsi/sd.c
+++ usb-4.4/drivers/scsi/sd.c
@@ -3275,8 +3275,8 @@ static int sd_suspend_common(struct devi
struct scsi_disk *sdkp = dev_get_drvdata(dev);
int ret = 0;
 
-   if (!sdkp)
-   return 0;   /* this can happen */
+   if (!sdkp)  /* E.g.: runtime suspend following sd_remove() */
+   return 0;
 
if (sdkp->WCE && sdkp->media_present) {
sd_printk(KERN_NOTICE, sdkp, "Synchronizing SCSI cache\n");
@@ -3315,6 +3315,9 @@ static int sd_resume(struct device *dev)
 {
struct scsi_disk *sdkp = dev_get_drvdata(dev);
 
+   if (!sdkp)  /* E.g.: runtime resume at the start of sd_probe() */
+   return 0;
+
if (!sdkp->device->manage_start_stop)
return 0;
 
Index: usb-4.4/drivers/scsi/sr.c
===
--- usb-4.4.orig/drivers/scsi/sr.c
+++ usb-4.4/drivers/scsi/sr.c
@@ -144,6 +144,9 @@ static int sr_runtime_suspend(struct dev
 {
struct scsi_cd *cd = dev_get_drvdata(dev);
 
+   if (!cd)/* E.g.: runtime suspend following sr_remove() */
+   return 0;
+
if (cd->media_present)
return -EBUSY;
else
@@ -985,6 +988,7 @@ static int sr_remove(struct device *dev)
scsi_autopm_get_device(cd->device);
 
del_gendisk(cd->disk);
+   dev_set_drvdata(dev, NULL);
 
mutex_lock(&sr_ref_mutex);
kref_put(&cd->kref, sr_kref_release);

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[James Bottomley]

On Wed, 2016-01-20 at 11:01 -0500, Martin K. Petersen wrote:
> Commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length
> limits") accidentally switched optimal I/O size reporting from bytes
> to
> block layer sectors.
> 
> Signed-off-by: Martin K. Petersen 
> Reported-by: Christian Borntraeger 
> ---
>  drivers/scsi/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 4e08d1cd704d..ec163d08f6c3 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2893,7 +2893,7 @@ static int sd_revalidate_disk(struct gendisk
> *disk)
>   sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
>   sdkp->opt_xfer_blocks * sdp->sector_size >=
> PAGE_CACHE_SIZE)
>   rw_max = q->limits.io_opt =
> - logical_to_sectors(sdp, sdkp
> ->opt_xfer_blocks);
> + sdkp->opt_xfer_blocks * sdp->sector_size;
>   else
>   rw_max = BLK_DEF_MAX_SECTORS;
>  

We should mark the commit causing the problems, which went into 4.4 if
I remember correctly:

Fixes: ca369d51b3e1649be4a72addd6d6a168cfb3f537
Cc: sta...@vger.kernel.org # 4.4+
Reviewed-by: James E.J. Bottomley 

James
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl

[Matthew R. Ochs]

Reviewed-by: Matthew R. Ochs  

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Optimal I/O size is in bytes, not sectors

[Johannes Thumshirn]

On Wed, Jan 20, 2016 at 11:01:23AM -0500, Martin K. Petersen wrote:
> Commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length
> limits") accidentally switched optimal I/O size reporting from bytes to
> block layer sectors.
> 
> Signed-off-by: Martin K. Petersen 
> Reported-by: Christian Borntraeger 
> ---
>  drivers/scsi/sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 4e08d1cd704d..ec163d08f6c3 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2893,7 +2893,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>   sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
>   sdkp->opt_xfer_blocks * sdp->sector_size >= PAGE_CACHE_SIZE)
>   rw_max = q->limits.io_opt =
> - logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
> + sdkp->opt_xfer_blocks * sdp->sector_size;
>   else
>   rw_max = BLK_DEF_MAX_SECTORS;
>  
> -- 
> 2.5.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: qla2xxxx: avoid type mismatch in comparison

[Bart Van Assche]

On 01/20/2016 02:47 AM, Arnd Bergmann wrote:

A recent bug fix added code that does

 bool logged_out = (status & 0x);
 if (logged_out == CTIO_PORT_LOGGED_OUT)
...

This looks wrong because we are comparing a boolean with an
integer constant, ang gcc warns about it accordingly:

drivers/scsi/qla2xxx/qla_target.c: In function 'qlt_do_ctio_completion':
drivers/scsi/qla2xxx/qla_target.c:3587:20: warning: comparison of constant '41' 
with boolean expression is always false [-Wbool-compare]
 (logged_out == CTIO_PORT_LOGGED_OUT) ?

The correct fix is presumably to make that variable an 'int'.

Signed-off-by: Arnd Bergmann 
Fixes: 71cdc0796465 ("qla2xxx: Delete session if initiator is gone from FW")
---
The patch introducing this is currenly in linux-next through the 
target-updates/for-next
branch.

diff --git a/drivers/scsi/qla2xxx/qla_target.c 
b/drivers/scsi/qla2xxx/qla_target.c
index c7ab9e69c881..8075a4cdb45c 100644
--- a/drivers/scsi/qla2xxx/qla_target.c
+++ b/drivers/scsi/qla2xxx/qla_target.c
@@ -3580,7 +3580,7 @@ static void qlt_do_ctio_completion(struct scsi_qla_host 
*vha, uint32_t handle,
case CTIO_PORT_LOGGED_OUT:
case CTIO_PORT_UNAVAILABLE:
{
-   bool logged_out = (status & 0x);
+   int logged_out = (status & 0x);
ql_dbg(ql_dbg_tgt_mgt, vha, 0xf059,
"qla_target(%d): CTIO with %s status %x "
"received (state %x, se_cmd %p)\n", vha->vp_idx,



Hello Arnd,

Please read the e-mail thread that is available at 
http://thread.gmane.org/gmane.linux.scsi/108899/focus=108943. That 
thread namely makes it clear that the above patch is not the proper way 
to fix that code.


Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: scsi-debug regression with 4.5-rc?

[Martin K. Petersen]

> "Ewan" == Ewan Milne  writes:

Ewan> So I have a report from our test people that the optimal_io_size
Ewan> sysfs value is now different by a factor of 512 from what it used
Ewan> to be...

Yes, just prepared a patch this morning. I messed up sectors vs. bytes.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sd: Optimal I/O size is in bytes, not sectors

[Martin K. Petersen]

Commit ca369d51b3e1 ("block/sd: Fix device-imposed transfer length
limits") accidentally switched optimal I/O size reporting from bytes to
block layer sectors.

Signed-off-by: Martin K. Petersen 
Reported-by: Christian Borntraeger 
---
 drivers/scsi/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4e08d1cd704d..ec163d08f6c3 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2893,7 +2893,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS &&
sdkp->opt_xfer_blocks * sdp->sector_size >= PAGE_CACHE_SIZE)
rw_max = q->limits.io_opt =
-   logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
+   sdkp->opt_xfer_blocks * sdp->sector_size;
else
rw_max = BLK_DEF_MAX_SECTORS;
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl

[Tomas Henzl]

On 20.1.2016 16:08, Johannes Thumshirn wrote:
> This removes a redundant code block that will either be executed if the
> ENABLE_FCP_RING_POLLING flag is set in phba->cfg_poll or not. The code is just
> duplicated in both cases, hence we unify it again.
>
> This probably is a left over from some sort of refactoring.
>
> Signed-off-by: Johannes Thumshirn 

Reviewed-by: Tomas Henzl 

Tomas

> ---
>  drivers/scsi/lpfc/lpfc_scsi.c | 17 -
>  1 file changed, 17 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
> index 152b3c8..3bd0be6 100644
> --- a/drivers/scsi/lpfc/lpfc_scsi.c
> +++ b/drivers/scsi/lpfc/lpfc_scsi.c
> @@ -4139,23 +4139,6 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct 
> lpfc_iocbq *pIocbIn,
>   /* The sdev is not guaranteed to be valid post scsi_done upcall. */
>   cmd->scsi_done(cmd);
>  
> - if (phba->cfg_poll & ENABLE_FCP_RING_POLLING) {
> - spin_lock_irqsave(&phba->hbalock, flags);
> - lpfc_cmd->pCmd = NULL;
> - spin_unlock_irqrestore(&phba->hbalock, flags);
> -
> - /*
> -  * If there is a thread waiting for command completion
> -  * wake up the thread.
> -  */
> - spin_lock_irqsave(shost->host_lock, flags);
> - if (lpfc_cmd->waitq)
> - wake_up(lpfc_cmd->waitq);
> - spin_unlock_irqrestore(shost->host_lock, flags);
> - lpfc_release_scsi_buf(phba, lpfc_cmd);
> - return;
> - }
> -
>   spin_lock_irqsave(&phba->hbalock, flags);
>   lpfc_cmd->pCmd = NULL;
>   spin_unlock_irqrestore(&phba->hbalock, flags);

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support

[Tomas Henzl]

On 20.1.2016 16:08, Sumit Saxena wrote:
>> -Original Message-
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Wednesday, January 20, 2016 7:46 PM
>> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
>> martin.peter...@oracle.com
>> Cc: linux-scsi@vger.kernel.org; Kashyap Desai
>> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support
>>
>> On 20.1.2016 15:09, Sumit Saxena wrote:
 -Original Message-
 From: Tomas Henzl [mailto:the...@redhat.com]
 Sent: Wednesday, January 20, 2016 7:26 PM
 To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
 martin.peter...@oracle.com
 Cc: linux-scsi@vger.kernel.org; Kashyap Desai
 Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support

 On 19.1.2016 14:44, Sumit Saxena wrote:
>> -Original Message-
>> From: Tomas Henzl [mailto:the...@redhat.com]
>> Sent: Tuesday, January 19, 2016 7:04 PM
>> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
>> martin.peter...@oracle.com
>> Cc: linux-scsi@vger.kernel.org; kashyap.de...@avagotech.com
>> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support
>>
>> On 18.12.2015 14:27, Sumit Saxena wrote:
>>> This patch will add support for Dual Queue depth reported by
>>> firmware.
>>>
>>> Below are key points-
>>>
>>> 1. For iMR controllers, firmware will report two queue depths- 1.
> Controller
>> wide Queue depth 2. LDIO Queue depth(240).
>>> Ofcourse, Controller wide Queue depth will be greater among two.
>>> Using this new method, iMR can provide larger Queue depth(QD) for
>>> JBOD and limited QD for Virtual Disk(VD). This feature gives
>>> benefit for iMR
> product
>> which will be used for deployment with large number of JBOD and
>> limited number of VD on setup.
>>> 2. megaraid_sas driver will throttle Read write LDIOs based when
>>> RW
> LDIOs
>> reaches "LDIO Queue Depth".
>>> 3. This feature of dual queue depth can enabled/disabled via
>>> module
>> parameter. Default behavior is: Dual Queue depth is enabled.
>>> 4. Added sysfs parameter "ldio_outstanding" for user to read LDIO
> outstanding
>> at run time.
>>> Signed-off-by: Sumit Saxena 
>>> Signed-off-by: Kashyap Desai 
>>> ---
>>>  drivers/scsi/megaraid/megaraid_sas.h|9 +++
>>>  drivers/scsi/megaraid/megaraid_sas_base.c   |   20 ++-
>>>  drivers/scsi/megaraid/megaraid_sas_fusion.c |   89
>> ---
>>>  3 files changed, 108 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h
>>> b/drivers/scsi/megaraid/megaraid_sas.h
>>> index c539516..4595ef4 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas.h
>>> +++ b/drivers/scsi/megaraid/megaraid_sas.h
>>> @@ -1353,6 +1353,12 @@ enum DCMD_TIMEOUT_ACTION {
>>> KILL_ADAPTER = 1,
>>> IGNORE_TIMEOUT = 2,
>>>  };
>>> +
>>> +enum FW_BOOT_CONTEXT {
>>> +   PROBE_CONTEXT = 0,
>>> +   OCR_CONTEXT = 1,
>>> +};
>>> +
>>>  /* Frame Type */
>>>  #define IO_FRAME   0
>>>  #define PTHRU_FRAME1
>>> @@ -2038,6 +2044,8 @@ struct megasas_instance {
>>> u16 max_fw_cmds;
>>> u16 max_mfi_cmds;
>>> u16 max_scsi_cmds;
>>> +   u16 ldio_threshold;
>>> +   u16 cur_can_queue;
>>> u32 max_sectors_per_req;
>>> struct megasas_aen_event *ev;
>>>
>>> @@ -2068,6 +2076,7 @@ struct megasas_instance {
>>> u32 fw_support_ieee;
>>>
>>> atomic_t fw_outstanding;
>>> +   atomic_t ldio_outstanding;
>>> atomic_t fw_reset_no_pci_access;
>>>
>>> struct megasas_instance_template *instancet; diff --git
>>> a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> index 3454c5e..edc26fb 100644
>>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
>>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
>>> @@ -96,6 +96,10 @@ int rdpq_enable = 1;  module_param(rdpq_enable,
>>> int, S_IRUGO);  MODULE_PARM_DESC(rdpq_enable, " Allocate reply
>>> queue in chunks for large queue depth enable/disbale Default:
>>> disable(0)");
>>>
>>> +unsigned int dual_qdepth_disable;
>>> +module_param(dual_qdepth_disable, int, S_IRUGO);
>>> +MODULE_PARM_DESC(dual_qdepth_disable, "Disable dual queue depth
>>> +feature. Default: 0");
>>> +
>>>  MODULE_LICENSE("GPL");
>>>  MODULE_VERSION(MEGASAS_VERSION);
>>>  MODULE_AUTHOR("megaraidlinux@avagotech.com");
>>> @@ -1977,7 +1981,7 @@
 megasas_check_and_restore_queue_depth(struct
>> megasas_instance *instance)
>>> spin_lock_irqsave(instance->host->host_l

[PATCH RESEND] lpfc: Add lockdep assertions

[Johannes Thumshirn]

Several functions in lpfc have comments stating that the function must be
called with the hbalock (or hostlock, or ringlock) held. Add
lockdep_assert_held() annotations to these functions, so one can actually
verify the locks are held.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/lpfc/lpfc_hbadisc.c |  5 +
 drivers/scsi/lpfc/lpfc_sli.c | 43 
 2 files changed, 48 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_hbadisc.c b/drivers/scsi/lpfc/lpfc_hbadisc.c
index c37d72e..25b5dcd 100644
--- a/drivers/scsi/lpfc/lpfc_hbadisc.c
+++ b/drivers/scsi/lpfc/lpfc_hbadisc.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -1314,6 +1315,8 @@ __lpfc_update_fcf_record_pri(struct lpfc_hba *phba, 
uint16_t fcf_index,
 {
struct lpfc_fcf_pri *fcf_pri;
 
+   lockdep_assert_held(&phba->hbalock);
+
fcf_pri = &phba->fcf.fcf_pri[fcf_index];
fcf_pri->fcf_rec.fcf_index = fcf_index;
/* FCF record priority */
@@ -1398,6 +1401,8 @@ __lpfc_update_fcf_record(struct lpfc_hba *phba, struct 
lpfc_fcf_rec *fcf_rec,
   struct fcf_record *new_fcf_record, uint32_t addr_mode,
   uint16_t vlan_id, uint32_t flag)
 {
+   lockdep_assert_held(&phba->hbalock);
+
/* Copy the fields from the HBA's FCF record */
lpfc_copy_fcf_record(fcf_rec, new_fcf_record);
/* Update other fields of driver FCF record */
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 92dfd6a..2207726 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -576,6 +577,8 @@ __lpfc_sli_get_iocbq(struct lpfc_hba *phba)
struct list_head *lpfc_iocb_list = &phba->lpfc_iocb_list;
struct lpfc_iocbq * iocbq = NULL;
 
+   lockdep_assert_held(&phba->hbalock);
+
list_remove_head(lpfc_iocb_list, iocbq, struct lpfc_iocbq, list);
if (iocbq)
phba->iocb_cnt++;
@@ -797,6 +800,7 @@ int
 lpfc_test_rrq_active(struct lpfc_hba *phba, struct lpfc_nodelist *ndlp,
uint16_t  xritag)
 {
+   lockdep_assert_held(&phba->hbalock);
if (!ndlp)
return 0;
if (!ndlp->active_rrqs_xri_bitmap)
@@ -914,6 +918,8 @@ __lpfc_sli_get_sglq(struct lpfc_hba *phba, struct 
lpfc_iocbq *piocbq)
struct lpfc_nodelist *ndlp;
int found = 0;
 
+   lockdep_assert_held(&phba->hbalock);
+
if (piocbq->iocb_flag &  LPFC_IO_FCP) {
lpfc_cmd = (struct lpfc_scsi_buf *) piocbq->context1;
ndlp = lpfc_cmd->rdata->pnode;
@@ -1003,6 +1009,8 @@ __lpfc_sli_release_iocbq_s4(struct lpfc_hba *phba, struct 
lpfc_iocbq *iocbq)
unsigned long iflag = 0;
struct lpfc_sli_ring *pring = &phba->sli.ring[LPFC_ELS_RING];
 
+   lockdep_assert_held(&phba->hbalock);
+
if (iocbq->sli4_xritag == NO_XRI)
sglq = NULL;
else
@@ -1058,6 +1066,7 @@ __lpfc_sli_release_iocbq_s3(struct lpfc_hba *phba, struct 
lpfc_iocbq *iocbq)
 {
size_t start_clean = offsetof(struct lpfc_iocbq, iocb);
 
+   lockdep_assert_held(&phba->hbalock);
 
/*
 * Clean all volatile data fields, preserve iotag and node struct.
@@ -1080,6 +1089,8 @@ __lpfc_sli_release_iocbq_s3(struct lpfc_hba *phba, struct 
lpfc_iocbq *iocbq)
 static void
 __lpfc_sli_release_iocbq(struct lpfc_hba *phba, struct lpfc_iocbq *iocbq)
 {
+   lockdep_assert_held(&phba->hbalock);
+
phba->__lpfc_sli_release_iocbq(phba, iocbq);
phba->iocb_cnt--;
 }
@@ -1310,6 +1321,8 @@ static int
 lpfc_sli_ringtxcmpl_put(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
struct lpfc_iocbq *piocb)
 {
+   lockdep_assert_held(&phba->hbalock);
+
list_add_tail(&piocb->list, &pring->txcmplq);
piocb->iocb_flag |= LPFC_IO_ON_TXCMPLQ;
 
@@ -1344,6 +1357,8 @@ lpfc_sli_ringtx_get(struct lpfc_hba *phba, struct 
lpfc_sli_ring *pring)
 {
struct lpfc_iocbq *cmd_iocb;
 
+   lockdep_assert_held(&phba->hbalock);
+
list_remove_head((&pring->txq), cmd_iocb, struct lpfc_iocbq, list);
return cmd_iocb;
 }
@@ -1367,6 +1382,9 @@ lpfc_sli_next_iocb_slot (struct lpfc_hba *phba, struct 
lpfc_sli_ring *pring)
 {
struct lpfc_pgp *pgp = &phba->port_gp[pring->ringno];
uint32_t  max_cmd_idx = pring->sli.sli3.numCiocb;
+
+   lockdep_assert_held(&phba->hbalock);
+
if ((pring->sli.sli3.next_cmdidx == pring->sli.sli3.cmdidx) &&
   (++pring->sli.sli3.next_cmdidx >= max_cmd_idx))
pring->sli.sli3.next_cmdidx = 0;
@@ -1497,6 +1515,7 @@ static void
 lpfc_sli_submit_iocb(struct lpfc_hba *phba, struct lpfc_sli_ring *pring,
IOCB_t *iocb, struct lpfc_iocbq *nextiocb)
 {
+   lockdep_assert_held(&phba->hbalock);
/*
 * Set up an iotag
 *

Re: scsi-debug regression with 4.5-rc?

[Ewan Milne]

So I have a report from our test people that the optimal_io_size sysfs
value is now different by a factor of 512 from what it used to be...

>Here is what is executed:
>
>modprobe scsi_debug dev_size_mb=32 sector_size=4096 opt_blks=64
>num_tgts=1
>
>And here is what our test is capturing:
>
>/sys/dev/block/8:0/queue/optimal_io_size = 512 differs from expected
>value 
>1048576!
>
>
>(So with 64 it's 512,  with opt_blks=256 it's 2048,  but it used to be
>1048576)

This looks like it might be due to commit:

commit ca369d51b3e1649be4a72addd6d6a168cfb3f537
Author: Martin K. Petersen 
Date:   Fri Nov 13 16:46:48 2015 -0500

block/sd: Fix device-imposed transfer length limits

It would appear that this commit has changed the meaning of
the queue limits io_opt field to be 512-byte normalized
sectors instead of bytes.  Parts of the diff:

-   blk_queue_io_opt(sdkp->disk->queue,
-get_unaligned_be32(&buffer[12]) * sector_sz);

...

+   sdkp->opt_xfer_blocks = get_unaligned_be32(&buffer[12]);

...

+   if (sdkp->opt_xfer_blocks && sdkp->opt_xfer_blocks <= dev_max &&
+   sdkp->opt_xfer_blocks <= SD_DEF_XFER_BLOCKS)
+   rw_max = q->limits.io_opt =
+   logical_to_sectors(sdp, sdkp->opt_xfer_blocks);
+   else
+   rw_max = BLK_DEF_MAX_SECTORS;


---

Was this intentional?  I would have thought we would not want to change
the usermode meaning of this field.

-Ewan


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 09/15] megaraid_sas: Dual Queue depth support

[Sumit Saxena]

> -Original Message-
> From: Tomas Henzl [mailto:the...@redhat.com]
> Sent: Wednesday, January 20, 2016 7:46 PM
> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
> martin.peter...@oracle.com
> Cc: linux-scsi@vger.kernel.org; Kashyap Desai
> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support
>
> On 20.1.2016 15:09, Sumit Saxena wrote:
> >> -Original Message-
> >> From: Tomas Henzl [mailto:the...@redhat.com]
> >> Sent: Wednesday, January 20, 2016 7:26 PM
> >> To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
> >> martin.peter...@oracle.com
> >> Cc: linux-scsi@vger.kernel.org; Kashyap Desai
> >> Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support
> >>
> >> On 19.1.2016 14:44, Sumit Saxena wrote:
>  -Original Message-
>  From: Tomas Henzl [mailto:the...@redhat.com]
>  Sent: Tuesday, January 19, 2016 7:04 PM
>  To: Sumit Saxena; jbottom...@parallels.com; h...@infradead.org;
>  martin.peter...@oracle.com
>  Cc: linux-scsi@vger.kernel.org; kashyap.de...@avagotech.com
>  Subject: Re: [PATCH 09/15] megaraid_sas: Dual Queue depth support
> 
>  On 18.12.2015 14:27, Sumit Saxena wrote:
> > This patch will add support for Dual Queue depth reported by
> > firmware.
> >
> > Below are key points-
> >
> > 1. For iMR controllers, firmware will report two queue depths- 1.
> >>> Controller
>  wide Queue depth 2. LDIO Queue depth(240).
> > Ofcourse, Controller wide Queue depth will be greater among two.
> > Using this new method, iMR can provide larger Queue depth(QD) for
> > JBOD and limited QD for Virtual Disk(VD). This feature gives
> > benefit for iMR
> >>> product
>  which will be used for deployment with large number of JBOD and
>  limited number of VD on setup.
> > 2. megaraid_sas driver will throttle Read write LDIOs based when
> > RW
> >>> LDIOs
>  reaches "LDIO Queue Depth".
> > 3. This feature of dual queue depth can enabled/disabled via
> > module
>  parameter. Default behavior is: Dual Queue depth is enabled.
> > 4. Added sysfs parameter "ldio_outstanding" for user to read LDIO
> >>> outstanding
>  at run time.
> > Signed-off-by: Sumit Saxena 
> > Signed-off-by: Kashyap Desai 
> > ---
> >  drivers/scsi/megaraid/megaraid_sas.h|9 +++
> >  drivers/scsi/megaraid/megaraid_sas_base.c   |   20 ++-
> >  drivers/scsi/megaraid/megaraid_sas_fusion.c |   89
>  ---
> >  3 files changed, 108 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/scsi/megaraid/megaraid_sas.h
> > b/drivers/scsi/megaraid/megaraid_sas.h
> > index c539516..4595ef4 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas.h
> > +++ b/drivers/scsi/megaraid/megaraid_sas.h
> > @@ -1353,6 +1353,12 @@ enum DCMD_TIMEOUT_ACTION {
> > KILL_ADAPTER = 1,
> > IGNORE_TIMEOUT = 2,
> >  };
> > +
> > +enum FW_BOOT_CONTEXT {
> > +   PROBE_CONTEXT = 0,
> > +   OCR_CONTEXT = 1,
> > +};
> > +
> >  /* Frame Type */
> >  #define IO_FRAME   0
> >  #define PTHRU_FRAME1
> > @@ -2038,6 +2044,8 @@ struct megasas_instance {
> > u16 max_fw_cmds;
> > u16 max_mfi_cmds;
> > u16 max_scsi_cmds;
> > +   u16 ldio_threshold;
> > +   u16 cur_can_queue;
> > u32 max_sectors_per_req;
> > struct megasas_aen_event *ev;
> >
> > @@ -2068,6 +2076,7 @@ struct megasas_instance {
> > u32 fw_support_ieee;
> >
> > atomic_t fw_outstanding;
> > +   atomic_t ldio_outstanding;
> > atomic_t fw_reset_no_pci_access;
> >
> > struct megasas_instance_template *instancet; diff --git
> > a/drivers/scsi/megaraid/megaraid_sas_base.c
> > b/drivers/scsi/megaraid/megaraid_sas_base.c
> > index 3454c5e..edc26fb 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -96,6 +96,10 @@ int rdpq_enable = 1;  module_param(rdpq_enable,
> > int, S_IRUGO);  MODULE_PARM_DESC(rdpq_enable, " Allocate reply
> > queue in chunks for large queue depth enable/disbale Default:
> > disable(0)");
> >
> > +unsigned int dual_qdepth_disable;
> > +module_param(dual_qdepth_disable, int, S_IRUGO);
> > +MODULE_PARM_DESC(dual_qdepth_disable, "Disable dual queue depth
> > +feature. Default: 0");
> > +
> >  MODULE_LICENSE("GPL");
> >  MODULE_VERSION(MEGASAS_VERSION);
> >  MODULE_AUTHOR("megaraidlinux@avagotech.com");
> > @@ -1977,7 +1981,7 @@
> >> megasas_check_and_restore_queue_depth(struct
>  megasas_instance *instance)
> > spin_lock_irqsave(instance->host->host_lock, flags);
> > instance->fl

[PATCH] lpfc: Remove redundant code block in lpfc_scsi_cmd_iocb_cmpl

[Johannes Thumshirn]

This removes a redundant code block that will either be executed if the
ENABLE_FCP_RING_POLLING flag is set in phba->cfg_poll or not. The code is just
duplicated in both cases, hence we unify it again.

This probably is a left over from some sort of refactoring.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/lpfc/lpfc_scsi.c | 17 -
 1 file changed, 17 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_scsi.c b/drivers/scsi/lpfc/lpfc_scsi.c
index 152b3c8..3bd0be6 100644
--- a/drivers/scsi/lpfc/lpfc_scsi.c
+++ b/drivers/scsi/lpfc/lpfc_scsi.c
@@ -4139,23 +4139,6 @@ lpfc_scsi_cmd_iocb_cmpl(struct lpfc_hba *phba, struct 
lpfc_iocbq *pIocbIn,
/* The sdev is not guaranteed to be valid post scsi_done upcall. */
cmd->scsi_done(cmd);
 
-   if (phba->cfg_poll & ENABLE_FCP_RING_POLLING) {
-   spin_lock_irqsave(&phba->hbalock, flags);
-   lpfc_cmd->pCmd = NULL;
-   spin_unlock_irqrestore(&phba->hbalock, flags);
-
-   /*
-* If there is a thread waiting for command completion
-* wake up the thread.
-*/
-   spin_lock_irqsave(shost->host_lock, flags);
-   if (lpfc_cmd->waitq)
-   wake_up(lpfc_cmd->waitq);
-   spin_unlock_irqrestore(shost->host_lock, flags);
-   lpfc_release_scsi_buf(phba, lpfc_cmd);
-   return;
-   }
-
spin_lock_irqsave(&phba->hbalock, flags);
lpfc_cmd->pCmd = NULL;
spin_unlock_irqrestore(&phba->hbalock, flags);
-- 
2.7.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html