Re: [PATCH] scsi: ILLEGAL REQUEST + ASC==27 => target failure

[Martin K. Petersen]

Martin,

> ASC 0x27 is "WRITE PROTECTED". This error code is returned e.g.  by
> Fujitsu ETERNUS systems under certain conditions for WRITE SAME 16
> commands with UNMAP bit set. It should not be treated as a path
> error. In general, it makes sense to assume that being write protected
> is a target rather than a path property.

Applied to 4.14/scsi-fixes. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: libsas: remove unused variable sas_ha

[Martin K. Petersen]

Colin,

> Remove unused variable sas_ha to clean up build warning
> "unused variable ‘sas_ha’ [-Wunused-variable]"

Applied to 4.15/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: fix wrong command type of UTRD for UFSHCI v2.1

[Martin K. Petersen]

kehuanlin,

> Since the command type of UTRD in UFS 2.1 specification is the same with
> UFS 2.0. And it assumes the future UFS specification will follow the same
> definition.

Applied to 4.15/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: continue to boot even with Boot LUN is disabled

[Martin K. Petersen]

Huanlin,

> Several configurable fields of the Device Descriptor and the Unit
> Descriptors determine the Boot LUN status. The bBootEnable field and
> the bBootLunEn attribute is set to zero by default, so the Boot LUN is
> disabled by default.
>
> At which point the scsi device add for Boot LUN will fail, but we can
> continue to use the ufs device in fact. This failure shouldn't abort the
> device boot.

Applied to 4.15/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: add ufs a command complete time stamp

[Martin K. Petersen]

Zang,

Applied to 4.15/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: aacraid: Add a small delay after IOP reset

[Martin K. Petersen]

Dave,

>> 
> Previously we had used sleep to delay until the controller got its
> mind back, but early testing indicated it wasn't needed. I'm good with
> this.

Applied to 4.14/scsi-fixes. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: fix a pclint warning

[Martin K. Petersen]

Zang,

Applied to 4.15/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH] scsi: sd: Do not override max_sectors_kb sysfs setting

[Martin K. Petersen]

A user may lower the max_sectors_kb setting in sysfs to accommodate
certain workloads. Previously we would always set the max I/O size to
either the block layer default or the optional preferred I/O size
reported by the device.

Keep the current heuristics for the initial setting of max_sectors_kb.
For subsequent invocations, only update the current queue limit if it
exceeds the capabilities of the hardware.

Reported-by: Don Brace 
Signed-off-by: Martin K. Petersen 
---
 drivers/scsi/sd.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6549e5ce09ca..b18ba3235900 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3098,8 +3098,6 @@ static int sd_revalidate_disk(struct gendisk *disk)
sd_read_security(sdkp, buffer);
}
 
-   sdkp->first_scan = 0;
-
/*
 * We now have all cache related info, determine how we deal
 * with flush requests.
@@ -3114,7 +3112,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
q->limits.max_dev_sectors = logical_to_sectors(sdp, dev_max);
 
/*
-* Use the device's preferred I/O size for reads and writes
+* Determine the device's preferred I/O size for reads and writes
 * unless the reported value is unreasonably small, large, or
 * garbage.
 */
@@ -3128,8 +3126,19 @@ static int sd_revalidate_disk(struct gendisk *disk)
rw_max = min_not_zero(logical_to_sectors(sdp, dev_max),
  (sector_t)BLK_DEF_MAX_SECTORS);
 
-   /* Combine with controller limits */
-   q->limits.max_sectors = min(rw_max, queue_max_hw_sectors(q));
+   /* Do not exceed controller limit */
+   rw_max = min(rw_max, queue_max_hw_sectors(q));
+
+   /*
+* Only update max_sectors if previously unset or if the current value
+* exceeds the capabilities of the hardware.
+*/
+   if (sdkp->first_scan ||
+   q->limits.max_sectors > q->limits.max_dev_sectors ||
+   q->limits.max_sectors > q->limits.max_hw_sectors)
+   q->limits.max_sectors = rw_max;
+
+   sdkp->first_scan = 0;
 
set_capacity(disk, logical_to_sectors(sdp, sdkp->capacity));
sd_config_write_same(sdkp);
-- 
2.14.1

Re: [PATCH] sd: preserve sysfs updates to max_sectors_kb

[Martin K. Petersen]

Martin,

> Could you please explain why you think Don's patch is wrong? User
> settings being discarded because of a BLKRRPART ioctl violates the
> principle of least surprise. With Don's patch, that won't happen any
> more. If hardware limits change, whether they increase or decrease, the
> patch will also do the Right Thing, AFAICS. Increasing hw limits will
> not automatically increase the sw limit, but IMO that's actually
> expected.

I have been mulling over this for a while.

I suggest the following...

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH] scsi: sd: Implement blacklist option for WRITE SAME w/ UNMAP

[Martin K. Petersen]

SBC-4 states:

  "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates the
   maximum number of LBAs that may be unmapped by an UNMAP command"

  "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value indicates
   the maximum number of contiguous logical blocks that the device server
   allows to be unmapped or written in a single WRITE SAME command."

Despite the spec being clear on the topic, some devices incorrectly
expect WRITE SAME commands with the UNMAP bit set to be limited to the
value reported in MAXIMUM UNMAP LBA COUNT in the Block Limits VPD.

Implement a blacklist option that can be used to accommodate devices
with this behavior.

Reported-by: Bill Kuzeja 
Reported-by: Ewan D. Milne 
Signed-off-by: Martin K. Petersen 
---
 drivers/scsi/scsi_scan.c|  3 +++
 drivers/scsi/sd.c   | 16 
 include/scsi/scsi_device.h  |  1 +
 include/scsi/scsi_devinfo.h |  1 +
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e7818afeda2b..15590a063ad9 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -956,6 +956,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned 
char *inq_result,
if (*bflags & BLIST_NO_DIF)
sdev->no_dif = 1;
 
+   if (*bflags & BLIST_UNMAP_LIMIT_WS)
+   sdev->unmap_limit_for_ws = 1;
+
sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT;
 
if (*bflags & BLIST_TRY_VPD_PAGES)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b18ba3235900..347be7580181 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -715,13 +715,21 @@ static void sd_config_discard(struct scsi_disk *sdkp, 
unsigned int mode)
break;
 
case SD_LBP_WS16:
-   max_blocks = min_not_zero(sdkp->max_ws_blocks,
- (u32)SD_MAX_WS16_BLOCKS);
+   if (sdkp->device->unmap_limit_for_ws)
+   max_blocks = sdkp->max_unmap_blocks;
+   else
+   max_blocks = sdkp->max_ws_blocks;
+
+   max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS16_BLOCKS);
break;
 
case SD_LBP_WS10:
-   max_blocks = min_not_zero(sdkp->max_ws_blocks,
- (u32)SD_MAX_WS10_BLOCKS);
+   if (sdkp->device->unmap_limit_for_ws)
+   max_blocks = sdkp->max_unmap_blocks;
+   else
+   max_blocks = sdkp->max_ws_blocks;
+
+   max_blocks = min_not_zero(max_blocks, (u32)SD_MAX_WS10_BLOCKS);
break;
 
case SD_LBP_ZERO:
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 82e93ee94708..67c5a9f223f7 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -192,6 +192,7 @@ struct scsi_device {
unsigned no_dif:1;  /* T10 PI (DIF) should be disabled */
unsigned broken_fua:1;  /* Don't set FUA bit */
unsigned lun_in_cdb:1;  /* Store LUN bits in CDB[1] */
+   unsigned unmap_limit_for_ws:1;  /* Use the UNMAP limit for WRITE SAME */
 
atomic_t disk_events_disable_depth; /* disable depth for disk events */
 
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 9592570e092a..36b03013d629 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -29,5 +29,6 @@
 #define BLIST_TRY_VPD_PAGES0x1000 /* Attempt to read VPD pages */
 #define BLIST_NO_RSOC  0x2000 /* don't try to issue RSOC */
 #define BLIST_MAX_1024 0x4000 /* maximum 1024 sector cdb length */
+#define BLIST_UNMAP_LIMIT_WS   0x8000 /* Use UNMAP limit for WRITE SAME */
 
 #endif
-- 
2.14.1

Re: [PATCH] sd: Limit WRITE SAME / WRITE SAME(16) w/UNMAP length for certain devices

[Martin K. Petersen]

Ewan,

>> The spec is crystal clear. The device needs to be fixed. We can
>> blacklist older firmware revs.
>
> Yes, I know that is what SBC-4 says, and I agree that the devices
> are not conforming.  Unfortunately, I've come across 3 different
> arrays now from 3 different manufacturers that exhibit this behavior.

My main beef with your approach is that it penalizes devices that
actually implement the spec correctly.

I'll send a proposed patch.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: ufs: add ufs a command complete time stamp

[Subhash Jadavani]

On 2017-09-26 19:06, Zang Leigang wrote:

Signed-off-by: Zang Leigang 

diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 794a460..7e8d583 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -385,6 +385,8 @@ void ufshcd_print_trs(struct ufs_hba *hba,
unsigned long bitmap, bool pr_prdt)

dev_err(hba->dev, "UPIU[%d] - issue time %lld us\n",
tag, ktime_to_us(lrbp->issue_time_stamp));
+   dev_err(hba->dev, "UPIU[%d] - complete time %lld us\n",
+   tag, ktime_to_us(lrbp->compl_time_stamp));
dev_err(hba->dev,
"UPIU[%d] - Transfer Request Descriptor phys@0x%llx\n",
tag, (u64)lrbp->utrd_dma_addr);
@@ -1746,6 +1748,7 @@ static inline
 void ufshcd_send_command(struct ufs_hba *hba, unsigned int task_tag)
 {
hba->lrb[task_tag].issue_time_stamp = ktime_get();
+   hba->lrb[task_tag].compl_time_stamp = ktime_set(0, 0);
ufshcd_clk_scaling_start_busy(hba);
__set_bit(task_tag, >outstanding_reqs);
ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
@@ -4627,6 +4630,8 @@ static void __ufshcd_transfer_req_compl(struct
ufs_hba *hba,
}
if (ufshcd_is_clkscaling_supported(hba))
hba->clk_scaling.active_reqs--;
+
+   lrbp->compl_time_stamp = ktime_get();
}

/* clear corresponding bits of completed commands */
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index cdc8bd0..40ea475 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -166,6 +166,7 @@ struct ufs_pm_lvl_states {
  * @lun: LUN of the command
  * @intr_cmd: Interrupt command (doesn't participate in interrupt 
aggregation)

  * @issue_time_stamp: time stamp for debug purposes
+ * @compl_time_stamp: time stamp for statistics
  * @req_abort_skip: skip request abort task flag
  */
 struct ufshcd_lrb {
@@ -189,6 +190,7 @@ struct ufshcd_lrb {
u8 lun; /* UPIU LUN id field is only 8-bit wide */
bool intr_cmd;
ktime_t issue_time_stamp;
+   ktime_t compl_time_stamp;

bool req_abort_skip;
 };


Looks good to me.
Reviewed-by: Subhash Jadavani 

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH 3/3] smartpqi: update driver version to 1.1.2-126

[Don Brace]

Reviewed-by: Gerry Morong 
Reviewed-by: Scott Benesh 
Reviewed-by: Scott Teel 
Signed-off-by: Kevin Barnett 
Signed-off-by: Don Brace 
---
 drivers/scsi/smartpqi/smartpqi_init.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
b/drivers/scsi/smartpqi/smartpqi_init.c
index be83d92..8fe9183 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -40,11 +40,11 @@
 #define BUILD_TIMESTAMP
 #endif
 
-#define DRIVER_VERSION "1.1.2-125"
+#define DRIVER_VERSION "1.1.2-126"
 #define DRIVER_MAJOR   1
 #define DRIVER_MINOR   1
 #define DRIVER_RELEASE 2
-#define DRIVER_REVISION125
+#define DRIVER_REVISION126
 
 #define DRIVER_NAME"Microsemi PQI Driver (v" \
DRIVER_VERSION BUILD_TIMESTAMP ")"

[PATCH 2/3] smartpqi: cleanup raid map warning message

[Don Brace]

From: Kevin Barnett 

Fix a small cosmetic bug in a very rarely encountered
error message that can occur when a LD has a corrupted
raid map.

Reviewed-by: Scott Benesh 
Signed-off-by: Kevin Barnett 
Signed-off-by: Don Brace 
---
 drivers/scsi/smartpqi/smartpqi_init.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
b/drivers/scsi/smartpqi/smartpqi_init.c
index 677b88e..be83d92 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -1078,9 +1078,9 @@ static int pqi_validate_raid_map(struct pqi_ctrl_info 
*ctrl_info,
 
 bad_raid_map:
dev_warn(_info->pci_dev->dev,
-   "scsi %d:%d:%d:%d %s\n",
-   ctrl_info->scsi_host->host_no,
-   device->bus, device->target, device->lun, err_msg);
+   "logical device %08x%08x %s\n",
+   *((u32 *)>scsi3addr),
+   *((u32 *)>scsi3addr[4]), err_msg);
 
return -EINVAL;
 }

[PATCH 1/3] smartpqi: update controller ids

[Don Brace]

From: Kevin Barnett 

Update the driver’s PCI IDs

Reviewed-by: Scott Benesh 
Reviewed-by: Scott Teel 
Signed-off-by: Kevin Barnett 
Signed-off-by: Don Brace 
---
 drivers/scsi/smartpqi/smartpqi_init.c |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c 
b/drivers/scsi/smartpqi/smartpqi_init.c
index 83bdbd8..677b88e 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -6926,6 +6926,14 @@ static const struct pci_device_id pqi_pci_id_table[] = {
},
{
PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+  PCI_VENDOR_ID_ADAPTEC2, 0x1302)
+   },
+   {
+   PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
+  PCI_VENDOR_ID_ADAPTEC2, 0x1303)
+   },
+   {
+   PCI_DEVICE_SUB(PCI_VENDOR_ID_ADAPTEC2, 0x028f,
   PCI_VENDOR_ID_ADAPTEC2, 0x1380)
},
{

[PATCH 0/3] smartpqi updates

[Don Brace]

These patches are based on Linus's tree

The changes are:
 - update list of controllers
 - cleanup warning message
 - change driver version to 1.1.2-126
---

Don Brace (1):
  smartpqi: update driver version to 1.1.2-126

Kevin Barnett (2):
  smartpqi: update controller ids
  smartpqi: cleanup raid map warning message


 drivers/scsi/smartpqi/smartpqi_init.c |   18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

--
Signature

RE: [PATCH] scsi: aacraid: Add a small delay after IOP reset

[Dave Carroll]

> 
> 
> Guilherme,
> 
> > James/Martin, am I expected to send a v2 with some change? Perhaps
> > with Dave's ack?  Sorry to annoy, thanks in advance for any advice!
> 
> I was just about to mail Dave and ask for confirmation that your 
> interpretation
> of the controller behavior is correct.
> 
> Dave?
> 
 Hi Martin,

Previously we had used sleep to delay until the controller got its mind back, 
but early testing indicated it wasn't needed. I'm  good with this.

-Dave

[PATCH] scsi: sd: add check for changing allow_restart

[weiping zhang]

/sys/class/scsi_disk/0:2:0:0/allow_restart can be changed to 0 unexpectly by
writing invalid string, like following:

echo asdf > /sys/class/scsi_disk/0:2:0:0/allow_restart

Signed-off-by: weiping zhang 
---
 drivers/scsi/sd.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3ef2214..d18639f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -253,6 +253,7 @@ static ssize_t
 allow_restart_store(struct device *dev, struct device_attribute *attr,
const char *buf, size_t count)
 {
+   int err, v;
struct scsi_disk *sdkp = to_scsi_disk(dev);
struct scsi_device *sdp = sdkp->device;
 
@@ -262,7 +263,11 @@ allow_restart_store(struct device *dev, struct 
device_attribute *attr,
if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
return -EINVAL;
 
-   sdp->allow_restart = simple_strtoul(buf, NULL, 10);
+   err = kstrtoint(buf, 10, );
+   if (err || (v != 0 && v != 1))
+   return -EINVAL;
+
+   sdp->allow_restart = v;
 
return count;
 }
-- 
2.9.4

RE: [PATCH] sd: Limit WRITE SAME / WRITE SAME(16) w/UNMAP length for certain devices

[Knight, Frederick]

I agree that it is disappointing that so many vendors seem to have trouble 
reading the spec.  This case is pretty clear.

The best the T10 committee could do is add a bit to indicate that the device 
uses the length from MAXIMUM UNMAP LBA COUNT field for the length of unmaps via 
the WRITE SAME w/UNMAP=1 rather than the MAXIMUM WRITE SAME LENGTH field.  BUT, 
I'll be very clear that the setting of any such new bit will be bit=0 is 
backward compatible for COMPLIANT devices, and bit=1 will be the new setting 
for "backwards" devices - which means they would STILL require a firmware 
change to tell you they are backwards, and you'd STILL need a blacklist for 
their older revisions.  And this would just makes the hosts job all that much 
harder!

Once a device is broken (violates the spec), there is not very much we can do 
in the spec to fix it (they have to fix their broken device).

Fred

-Original Message-
From: Ewan D. Milne [mailto:emi...@redhat.com] 
Sent: Wednesday, September 27, 2017 12:28 PM
To: Martin K. Petersen 
Cc: linux-scsi@vger.kernel.org; Knight, Frederick 
Subject: Re: [PATCH] sd: Limit WRITE SAME / WRITE SAME(16) w/UNMAP length for 
certain devices

On Mon, 2017-09-25 at 21:46 -0400, Martin K. Petersen wrote:
> Ewan,
> 
> > Some devices do not support a WRITE SAME / WRITE SAME(16) with the
> > UNMAP bit set up to the length specified in the MAXIMUM WRITE SAME
> > LENGTH field in the block limits VPD page (or, the field is zero,
> > indicating there is no limit).  Limit the length by the MAXIMUM UNMAP
> > LBA COUNT value.  Otherwise the command might be rejected.
> 
> From SBC4:
> 
>   "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates the
>   maximum number of LBAs that may be unmapped by an UNMAP command"
> 
> Note that it explicitly states "UNMAP command" and not "unmap
> operation".
> 
>   "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value indicates
>   the maximum number of contiguous logical blocks that the device server
>   allows to be unmapped or written in a single WRITE SAME command."
> 
> It says "unmapped or written" and "WRITE SAME command".
> 
> The spec is crystal clear. The device needs to be fixed. We can
> blacklist older firmware revs.
> 

Yes, I know that is what SBC-4 says, and I agree that the devices
are not conforming.  Unfortunately, I've come across 3 different
arrays now from 3 different manufacturers that exhibit this behavior.

cc: Fred Knight for his opinion on this (NetApp was not one of the
arrays that I've run into, though).

-Ewan

Re: [PATCH] sd: Limit WRITE SAME / WRITE SAME(16) w/UNMAP length for certain devices

[Ewan D. Milne]

On Mon, 2017-09-25 at 21:46 -0400, Martin K. Petersen wrote:
> Ewan,
> 
> > Some devices do not support a WRITE SAME / WRITE SAME(16) with the
> > UNMAP bit set up to the length specified in the MAXIMUM WRITE SAME
> > LENGTH field in the block limits VPD page (or, the field is zero,
> > indicating there is no limit).  Limit the length by the MAXIMUM UNMAP
> > LBA COUNT value.  Otherwise the command might be rejected.
> 
> From SBC4:
> 
>   "A MAXIMUM UNMAP LBA COUNT field set to a non-zero value indicates the
>   maximum number of LBAs that may be unmapped by an UNMAP command"
> 
> Note that it explicitly states "UNMAP command" and not "unmap
> operation".
> 
>   "A MAXIMUM WRITE SAME LENGTH field set to a non-zero value indicates
>   the maximum number of contiguous logical blocks that the device server
>   allows to be unmapped or written in a single WRITE SAME command."
> 
> It says "unmapped or written" and "WRITE SAME command".
> 
> The spec is crystal clear. The device needs to be fixed. We can
> blacklist older firmware revs.
> 

Yes, I know that is what SBC-4 says, and I agree that the devices
are not conforming.  Unfortunately, I've come across 3 different
arrays now from 3 different manufacturers that exhibit this behavior.

cc: Fred Knight for his opinion on this (NetApp was not one of the
arrays that I've run into, though).

-Ewan

Re: [PATCH] scsi: ILLEGAL REQUEST + ASC==27 => target failure

[Lee Duncan]

On 09/27/2017 05:44 AM, Martin Wilck wrote:
> ASC 0x27 is "WRITE PROTECTED". This error code is returned e.g.
> by Fujitsu ETERNUS systems under certain conditions for
> WRITE SAME 16 commands with UNMAP bit set. It should not be
> treated as a path error. In general, it makes sense to assume
> that being write protected is a target rather than a path
> property.
> 
> Signed-off-by: Martin Wilck 
> ---
>  drivers/scsi/scsi_error.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 38942050b265..dab876c65473 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -580,7 +580,8 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
>   if (sshdr.asc == 0x20 || /* Invalid command operation code */
>   sshdr.asc == 0x21 || /* Logical block address out of range 
> */
>   sshdr.asc == 0x24 || /* Invalid field in cdb */
> - sshdr.asc == 0x26) { /* Parameter value invalid */
> + sshdr.asc == 0x26 || /* Parameter value invalid */
> + sshdr.asc == 0x27) { /* Write protected */
>   set_host_byte(scmd, DID_TARGET_FAILURE);
>   }
>   return SUCCESS;
> 

Looks good to me.

Acked-by: Lee Duncan 

-- 
Lee-Man

Re: [PATCH] scsi: ioctl reset should wait for IOs to complete

[Lee Duncan]

On 09/26/2017 11:58 PM, Hannes Reinecke wrote:
> On 09/26/2017 08:24 PM, Bart Van Assche wrote:
>> On Tue, 2017-09-26 at 10:22 -0700, Lee Duncan wrote:
>>> The SCSI ioctl reset path is smart enough to set the
>>> flag tmf_in_progress when a user-requested reset is
>>> processed, but it does not wait for IO that is in
>>> flight. This can result in lost IOs and hung
>>> processes. We should wait for a reasonable amount
>>> of time for either the IOs to complete or to fail
>>> the request.
>>
>> Hello Lee,
>>
>> I'm using this functionality all the time to test how SCSI target code 
>> handles
>> TMFs while SCSI commands are in progress. So I would regret if the SCSI reset
>> ioctl code would be modified such that it waits for outstanding requests.
>> Isn't the behavior you described a SCSI LLD bug? Shouldn't such bugs be fixed
>> instead of implementing a work-around in the SCSI core?
>>
> Well, thing is that there is an asymmetry here; originally all SCSI EH
> functions were supposed to run with no I/O in flight.
> (I've modified that with the asynchronous ABORT TASK TMF, but still).
> But when called with sg_reset this is no longer true, we're disallowing
> new requests, but do not wait for the in-flight I/O to complete.
> And we've had a customer report where calling sg_reset -t on an iSCSI
> device caused I/O to become stuck as the in-flight I/O was terminated by
> the target reset, but the iSCSI stack never sent a completion for that I/O.
> 
> However, we could also defer this problem until my SCSI EH rework goes
> in; that clears up the sg_reset path and might clarify things a bit.
> 
> Cheers,
> 
> Hannes
> 

I will wait and see if the problem still exists there, and address it if
it does.

Thank you.
-- 
Lee Duncan
SUSE Labs

[PATCH] scsi: ILLEGAL REQUEST + ASC==27 => target failure

[Martin Wilck]

ASC 0x27 is "WRITE PROTECTED". This error code is returned e.g.
by Fujitsu ETERNUS systems under certain conditions for
WRITE SAME 16 commands with UNMAP bit set. It should not be
treated as a path error. In general, it makes sense to assume
that being write protected is a target rather than a path
property.

Signed-off-by: Martin Wilck 
---
 drivers/scsi/scsi_error.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 38942050b265..dab876c65473 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -580,7 +580,8 @@ int scsi_check_sense(struct scsi_cmnd *scmd)
if (sshdr.asc == 0x20 || /* Invalid command operation code */
sshdr.asc == 0x21 || /* Logical block address out of range 
*/
sshdr.asc == 0x24 || /* Invalid field in cdb */
-   sshdr.asc == 0x26) { /* Parameter value invalid */
+   sshdr.asc == 0x26 || /* Parameter value invalid */
+   sshdr.asc == 0x27) { /* Write protected */
set_host_byte(scmd, DID_TARGET_FAILURE);
}
return SUCCESS;
-- 
2.14.0

Re: [PATCH V6 6/6] SCSI: set block queue at preempt only when SCSI device is put into quiesce

[Ming Lei]

On Wed, Sep 27, 2017 at 09:54:09AM +, Bart Van Assche wrote:
> On Wed, 2017-09-27 at 13:48 +0800, Ming Lei wrote:
> > @@ -2928,12 +2929,28 @@ scsi_device_quiesce(struct scsi_device *sdev)
> >  {
> > int err;
> >  
> > +   /*
> > +* Simply quiesing SCSI device isn't safe, it is easy
> > +* to use up requests because all these allocated requests
> > +* can't be dispatched when device is put in QIUESCE.
> > +* Then no request can be allocated and we may hang
> > +* somewhere, such as system suspend/resume.
> > +*
> > +* So we set block queue in preempt only first, no new
> > +* normal request can enter queue any more, and all pending
> > +* requests are drained once blk_set_preempt_only()
> > +* returns. Only RQF_PREEMPT is allowed in preempt only mode.
> > +*/
> > +   blk_set_preempt_only(sdev->request_queue, true);
> > +
> > mutex_lock(>state_mutex);
> > err = scsi_device_set_state(sdev, SDEV_QUIESCE);
> > mutex_unlock(>state_mutex);
> >  
> > -   if (err)
> > +   if (err) {
> > +   blk_set_preempt_only(sdev->request_queue, false);
> > return err;
> > +   }
> >  
> > scsi_run_queue(sdev->request_queue);
> > while (atomic_read(>device_busy)) {
> > @@ -2964,6 +2981,8 @@ void scsi_device_resume(struct scsi_device *sdev)
> > scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
> > scsi_run_queue(sdev->request_queue);
> > mutex_unlock(>state_mutex);
> > +
> > +   blk_set_preempt_only(sdev->request_queue, false);
> 
> You should have realized yourself that this code is racy. If a request is
> allocated just before scsi_device_quiesce() is called and dispatched just
> after the device state has been changed into SDEV_QUIESCE then the loop that

That won't happen, any requests allocated before blk_set_preempt_only(true)
will be drained. Any normal requests are prevented from being entering
queue after blk_set_preempt_only(true) returns.

Please look at blk_set_preempt_only():

+void blk_set_preempt_only(struct request_queue *q, bool preempt_only)
+{
+   blk_mq_freeze_queue(q);
+   if (preempt_only)
+   queue_flag_set_unlocked(QUEUE_FLAG_PREEMPT_ONLY, q);
+   else
+   queue_flag_clear_unlocked(QUEUE_FLAG_PREEMPT_ONLY, q);
+   blk_mq_unfreeze_queue(q);
+}
+EXPORT_SYMBOL(blk_set_preempt_only);

blk_set_preempt_only(true) is called before scsi_device_set_state(sdev, 
SDEV_QUIESCE),
then any requests will be drained by blk_mq_freeze_queue() inside
blk_set_preempt_only(), meantime new normal requests are prevented from
being entering queue.

Once blk_set_preempt_only() returns, only RQF_PREEMPT is allowed to
enter queue.


-- 
Ming

Re: [PATCH V6 6/6] SCSI: set block queue at preempt only when SCSI device is put into quiesce

[Bart Van Assche]

On Wed, 2017-09-27 at 13:48 +0800, Ming Lei wrote:
> @@ -2928,12 +2929,28 @@ scsi_device_quiesce(struct scsi_device *sdev)
>  {
>   int err;
>  
> + /*
> +  * Simply quiesing SCSI device isn't safe, it is easy
> +  * to use up requests because all these allocated requests
> +  * can't be dispatched when device is put in QIUESCE.
> +  * Then no request can be allocated and we may hang
> +  * somewhere, such as system suspend/resume.
> +  *
> +  * So we set block queue in preempt only first, no new
> +  * normal request can enter queue any more, and all pending
> +  * requests are drained once blk_set_preempt_only()
> +  * returns. Only RQF_PREEMPT is allowed in preempt only mode.
> +  */
> + blk_set_preempt_only(sdev->request_queue, true);
> +
>   mutex_lock(>state_mutex);
>   err = scsi_device_set_state(sdev, SDEV_QUIESCE);
>   mutex_unlock(>state_mutex);
>  
> - if (err)
> + if (err) {
> + blk_set_preempt_only(sdev->request_queue, false);
>   return err;
> + }
>  
>   scsi_run_queue(sdev->request_queue);
>   while (atomic_read(>device_busy)) {
> @@ -2964,6 +2981,8 @@ void scsi_device_resume(struct scsi_device *sdev)
>   scsi_device_set_state(sdev, SDEV_RUNNING) == 0)
>   scsi_run_queue(sdev->request_queue);
>   mutex_unlock(>state_mutex);
> +
> + blk_set_preempt_only(sdev->request_queue, false);

You should have realized yourself that this code is racy. If a request is
allocated just before scsi_device_quiesce() is called and dispatched just
after the device state has been changed into SDEV_QUIESCE then the loop that
waits for all commands to complete will wait forever due to the SCSI prep
function returning BLKPREP_DEFER.

Bart.

Re: [PATCH V6 0/6] block/scsi: safe SCSI quiescing

[Ming Lei]

On Wed, Sep 27, 2017 at 04:27:51PM +0800, Ming Lei wrote:
> On Wed, Sep 27, 2017 at 09:57:37AM +0200, Martin Steigerwald wrote:
> > Hi Ming.
> > 
> > Ming Lei - 27.09.17, 13:48:
> > > Hi,
> > > 
> > > The current SCSI quiesce isn't safe and easy to trigger I/O deadlock.
> > > 
> > > Once SCSI device is put into QUIESCE, no new request except for
> > > RQF_PREEMPT can be dispatched to SCSI successfully, and
> > > scsi_device_quiesce() just simply waits for completion of I/Os
> > > dispatched to SCSI stack. It isn't enough at all.
> > > 
> > > Because new request still can be comming, but all the allocated
> > > requests can't be dispatched successfully, so request pool can be
> > > consumed up easily.
> > > 
> > > Then request with RQF_PREEMPT can't be allocated and wait forever,
> > > meantime scsi_device_resume() waits for completion of RQF_PREEMPT,
> > > then system hangs forever, such as during system suspend or
> > > sending SCSI domain alidation.
> > > 
> > > Both IO hang inside system suspend[1] or SCSI domain validation
> > > were reported before.
> > > 
> > > This patch introduces preempt only mode, and solves the issue
> > > by allowing RQF_PREEMP only during SCSI quiesce.
> > > 
> > > Both SCSI and SCSI_MQ have this IO deadlock issue, this patch fixes
> > > them all.
> > > 
> > > V6:
> > >   - borrow Bart's idea of preempt only, with clean
> > > implementation(patch 5/patch 6)
> > >   - needn't any external driver's dependency, such as MD's
> > >   change
> > 
> > Do you want me to test with v6 of the patch set? If so, it would be nice if 
> > you´d make a v6 branch in your git repo.
> 
> Hi Martin,
> 
> I appreciate much if you may run V6 and provide your test result,
> follows the branch:
> 
> https://github.com/ming1/linux/tree/blk_safe_scsi_quiesce_V6
> 
> https://github.com/ming1/linux.git #blk_safe_scsi_quiesce_V6
> 

Also follows the branch against V4.13:

https://github.com/ming1/linux/tree/v4.13-safe-scsi-quiesce_V6_for_test

https://github.com/ming1/linux.git #v4.13-safe-scsi-quiesce_V6_for_test

-- 
Ming

Re: [PATCH V6 0/6] block/scsi: safe SCSI quiescing

[Ming Lei]

On Wed, Sep 27, 2017 at 09:57:37AM +0200, Martin Steigerwald wrote:
> Hi Ming.
> 
> Ming Lei - 27.09.17, 13:48:
> > Hi,
> > 
> > The current SCSI quiesce isn't safe and easy to trigger I/O deadlock.
> > 
> > Once SCSI device is put into QUIESCE, no new request except for
> > RQF_PREEMPT can be dispatched to SCSI successfully, and
> > scsi_device_quiesce() just simply waits for completion of I/Os
> > dispatched to SCSI stack. It isn't enough at all.
> > 
> > Because new request still can be comming, but all the allocated
> > requests can't be dispatched successfully, so request pool can be
> > consumed up easily.
> > 
> > Then request with RQF_PREEMPT can't be allocated and wait forever,
> > meantime scsi_device_resume() waits for completion of RQF_PREEMPT,
> > then system hangs forever, such as during system suspend or
> > sending SCSI domain alidation.
> > 
> > Both IO hang inside system suspend[1] or SCSI domain validation
> > were reported before.
> > 
> > This patch introduces preempt only mode, and solves the issue
> > by allowing RQF_PREEMP only during SCSI quiesce.
> > 
> > Both SCSI and SCSI_MQ have this IO deadlock issue, this patch fixes
> > them all.
> > 
> > V6:
> > - borrow Bart's idea of preempt only, with clean
> >   implementation(patch 5/patch 6)
> > - needn't any external driver's dependency, such as MD's
> > change
> 
> Do you want me to test with v6 of the patch set? If so, it would be nice if 
> you´d make a v6 branch in your git repo.

Hi Martin,

I appreciate much if you may run V6 and provide your test result,
follows the branch:

https://github.com/ming1/linux/tree/blk_safe_scsi_quiesce_V6

https://github.com/ming1/linux.git #blk_safe_scsi_quiesce_V6


> 
> After an uptime of almost 6 days I am pretty confident that the V5 one fixes 
> the 
> issue for me. So
> 
> Tested-by: Martin Steigerwald 
> 
> for V5.

Thanks for your test!


-- 
Ming

Re: [PATCH V6 0/6] block/scsi: safe SCSI quiescing

[Martin Steigerwald]

Hi Ming.

Ming Lei - 27.09.17, 13:48:
> Hi,
> 
> The current SCSI quiesce isn't safe and easy to trigger I/O deadlock.
> 
> Once SCSI device is put into QUIESCE, no new request except for
> RQF_PREEMPT can be dispatched to SCSI successfully, and
> scsi_device_quiesce() just simply waits for completion of I/Os
> dispatched to SCSI stack. It isn't enough at all.
> 
> Because new request still can be comming, but all the allocated
> requests can't be dispatched successfully, so request pool can be
> consumed up easily.
> 
> Then request with RQF_PREEMPT can't be allocated and wait forever,
> meantime scsi_device_resume() waits for completion of RQF_PREEMPT,
> then system hangs forever, such as during system suspend or
> sending SCSI domain alidation.
> 
> Both IO hang inside system suspend[1] or SCSI domain validation
> were reported before.
> 
> This patch introduces preempt only mode, and solves the issue
> by allowing RQF_PREEMP only during SCSI quiesce.
> 
> Both SCSI and SCSI_MQ have this IO deadlock issue, this patch fixes
> them all.
> 
> V6:
>   - borrow Bart's idea of preempt only, with clean
> implementation(patch 5/patch 6)
>   - needn't any external driver's dependency, such as MD's
>   change

Do you want me to test with v6 of the patch set? If so, it would be nice if 
you´d make a v6 branch in your git repo.

After an uptime of almost 6 days I am pretty confident that the V5 one fixes 
the 
issue for me. So

Tested-by: Martin Steigerwald 

for V5.

Thanks,
Martin

> V5:
>   - fix one tiny race by introducing blk_queue_enter_preempt_freeze()
>   given this change is small enough compared with V4, I added
>   tested-by directly
> 
> V4:
>   - reorganize patch order to make it more reasonable
>   - support nested preempt freeze, as required by SCSI transport spi
>   - check preempt freezing in slow path of of blk_queue_enter()
>   - add "SCSI: transport_spi: resume a quiesced device"
>   - wake up freeze queue in setting dying for both blk-mq and legacy
>   - rename blk_mq_[freeze|unfreeze]_queue() in one patch
>   - rename .mq_freeze_wq and .mq_freeze_depth
>   - improve comment
> 
> V3:
>   - introduce q->preempt_unfreezing to fix one bug of preempt freeze
>   - call blk_queue_enter_live() only when queue is preempt frozen
>   - cleanup a bit on the implementation of preempt freeze
>   - only patch 6 and 7 are changed
> 
> V2:
>   - drop the 1st patch in V1 because percpu_ref_is_dying() is
>   enough as pointed by Tejun
>   - introduce preempt version of blk_[freeze|unfreeze]_queue
>   - sync between preempt freeze and normal freeze
>   - fix warning from percpu-refcount as reported by Oleksandr
> 
> 
> [1] https://marc.info/?t=150340250100013=3=2
> 
> 
> Thanks,
> Ming
> 
> Ming Lei (6):
>   blk-mq: only run hw queues for blk-mq
>   block: tracking request allocation with q_usage_counter
>   block: pass flags to blk_queue_enter()
>   block: prepare for passing RQF_PREEMPT to request allocation
>   block: support PREEMPT_ONLY
>   SCSI: set block queue at preempt only when SCSI device is put into
> quiesce
> 
>  block/blk-core.c| 62
> ++--- block/blk-mq.c  |
> 14 ---
>  block/blk-timeout.c |  2 +-
>  drivers/scsi/scsi_lib.c | 25 +---
>  fs/block_dev.c  |  4 ++--
>  include/linux/blk-mq.h  |  7 +++---
>  include/linux/blkdev.h  | 27 ++---
>  7 files changed, 106 insertions(+), 35 deletions(-)


-- 
Martin

mptsas driver cannot detect hotplugging disk with the LSI SCSI SAS1068 controller in Ubuntu guest on VMware

[Gavin Guo]

There is a problem in the latest upstream kernel with the device:

$ grep -i lsi lspci
03:00.0 Serial Attached SCSI controller [0107]: LSI Logic / Symbios
Logic SAS1068 PCI-X Fusion-MPT SAS [1000:0054] (rev 01)

The device is simulated by the VMware ESXi 5.5

When hotplugging a new disk to the Guest Ubuntu OS, the latest kernel
cannot automatically probe the disk. However, on the v3.19.0-80.88
kernel, the disk can be dynamically probed and show the following
info message:

mptsas: ioc0: attaching ssp device: fw_channel 0, fw_id 1, phy 1,
sas_addr 0x5000c29a6bdae0f5
scsi 2:0:1:0: Direct-Access VMware   Virtual disk 1.0  PQ: 0
ANSI: 2
sd 2:0:1:0: Attached scsi generic sg2 type 0
sd 2:0:1:0: [sdb] 2097152 512-byte logical blocks: (1.07 GB/1.00 GiB)
sd 2:0:1:0: [sdb] Write Protect is off
sd 2:0:1:0: [sdb] Mode Sense: 61 00 00 00
sd 2:0:1:0: [sdb] Cache data unavailable
sd 2:0:1:0: [sdb] Assuming drive cache: write through
 sdb: unknown partition table
sd 2:0:1:0: [sdb] Attached SCSI disk

After looking up the message:
mptsas: ioc0: attaching ssp device: fw_channel 0, fw_id 1, phy 1,
sas_addr 0x5000c29a6bdae0f5

I found it comes from the path:
mptsas_firmware_event_work -> mptsas_send_sas_event ->
mptsas_hotplug_work -> mptsas_add_end_device

I'll appreciate if anyone can give the idea: If it's possible that the
irq from the simulated LSI SAS controller didn't come in to trigger
the event? However, it can work on the v3.19 kernel so if there is
any driver implementation issue in the latest kernel.

Re: [PATCH] scsi: ioctl reset should wait for IOs to complete

[Hannes Reinecke]

On 09/26/2017 08:24 PM, Bart Van Assche wrote:
> On Tue, 2017-09-26 at 10:22 -0700, Lee Duncan wrote:
>> The SCSI ioctl reset path is smart enough to set the
>> flag tmf_in_progress when a user-requested reset is
>> processed, but it does not wait for IO that is in
>> flight. This can result in lost IOs and hung
>> processes. We should wait for a reasonable amount
>> of time for either the IOs to complete or to fail
>> the request.
> 
> Hello Lee,
> 
> I'm using this functionality all the time to test how SCSI target code handles
> TMFs while SCSI commands are in progress. So I would regret if the SCSI reset
> ioctl code would be modified such that it waits for outstanding requests.
> Isn't the behavior you described a SCSI LLD bug? Shouldn't such bugs be fixed
> instead of implementing a work-around in the SCSI core?
> 
Well, thing is that there is an asymmetry here; originally all SCSI EH
functions were supposed to run with no I/O in flight.
(I've modified that with the asynchronous ABORT TASK TMF, but still).
But when called with sg_reset this is no longer true, we're disallowing
new requests, but do not wait for the in-flight I/O to complete.
And we've had a customer report where calling sg_reset -t on an iSCSI
device caused I/O to become stuck as the in-flight I/O was terminated by
the target reset, but the iSCSI stack never sent a completion for that I/O.

However, we could also defer this problem until my SCSI EH rework goes
in; that clears up the sg_reset path and might clarify things a bit.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.com  +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: [PATCH 4/5] scsi/ufs: qcom: Set phy mode based on the controllers HS MODE

[Vivek Gautam]

On 09/27/2017 04:14 AM, Subhash Jadavani wrote:

On 2017-08-03 23:48, Vivek Gautam wrote:

Set the phy mode based on the UFS HS PA mode. This lets the
controller let phy know the mode in which the PHY Adapter is
running and set the phy rates accordingly.

Signed-off-by: Vivek Gautam 
---
 drivers/scsi/ufs/ufs-qcom.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c
index c87d770b519a..44c21d5818ee 100644
--- a/drivers/scsi/ufs/ufs-qcom.c
+++ b/drivers/scsi/ufs/ufs-qcom.c
@@ -273,6 +273,9 @@ static int ufs_qcom_power_up_sequence(struct 
ufs_hba *hba)

 bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B)
 ? true : false;

+if (is_rate_B)
+phy_set_mode(phy, PHY_MODE_UFS_HS_B);
+
 /* Assert PHY reset and apply PHY calibration values */
 ufs_qcom_assert_reset(hba);
 /* provide 1ms delay to let the reset pulse propagate */


Looks good to me.
Reviewed-by: Subhash Jadavani 


Thanks for reviewing Subhash.

BRs
Vivek

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 5/5] ufs/phy: qcom: Refactor to use phy_init call

[Vivek Gautam]

Hi Subhash,


On Wed, Sep 27, 2017 at 4:43 AM, Subhash Jadavani
 wrote:
> Hi Vivek,
>
> Please find one comment inline below, rest look good.
>
> Regards,
> Subhash
>
>
> On 2017-08-03 23:48, Vivek Gautam wrote:
>>
>> Refactor ufs_qcom_power_up_sequence() to get rid of ugly
>> exported phy APIs and use the phy_init() and phy_power_on()
>> to do the phy initialization.
>>
>> Signed-off-by: Vivek Gautam 
>> ---
>>  drivers/phy/qualcomm/phy-qcom-ufs-i.h|  2 --
>>  drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c |  9 +--
>>  drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c |  9 +--
>>  drivers/phy/qualcomm/phy-qcom-ufs.c  | 38
>> 
>>  drivers/scsi/ufs/ufs-qcom.c  | 36
>> ++
>>  include/linux/phy/phy-qcom-ufs.h |  3 ---
>>  6 files changed, 38 insertions(+), 59 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> index 94326ed107c3..495fd5941231 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-i.h
>> @@ -123,7 +123,6 @@ struct ufs_qcom_phy {
>>   * struct ufs_qcom_phy_specific_ops - set of pointers to functions which
>> have a
>>   * specific implementation per phy. Each UFS phy, should implement
>>   * those functions according to its spec and requirements
>> - * @calibrate_phy: pointer to a function that calibrate the phy
>>   * @start_serdes: pointer to a function that starts the serdes
>>   * @is_physical_coding_sublayer_ready: pointer to a function that
>>   * checks pcs readiness. returns 0 for success and non-zero for error.
>> @@ -132,7 +131,6 @@ struct ufs_qcom_phy {
>>   * and writes to QSERDES_RX_SIGDET_CNTRL attribute
>>   */
>>  struct ufs_qcom_phy_specific_ops {
>> -   int (*calibrate_phy)(struct ufs_qcom_phy *phy, bool is_rate_B);
>> void (*start_serdes)(struct ufs_qcom_phy *phy);
>> int (*is_physical_coding_sublayer_ready)(struct ufs_qcom_phy
>> *phy);
>> void (*set_tx_lane_enable)(struct ufs_qcom_phy *phy, u32 val);
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> index af65785230b5..c39440b56b6d 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
>> @@ -44,7 +44,13 @@ void ufs_qcom_phy_qmp_14nm_advertise_quirks(struct
>> ufs_qcom_phy *phy_common)
>>
>>  static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy)
>>  {
>> -   return 0;
>> +   struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
>> +   bool is_rate_B = false;
>> +
>> +   if (phy_common->mode == PHY_MODE_UFS_HS_B)
>> +   is_rate_B = true;
>> +
>> +   return ufs_qcom_phy_qmp_14nm_phy_calibrate(phy_common, is_rate_B);
>>  }
>>
>>  static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)
>> @@ -120,7 +126,6 @@ static int
>> ufs_qcom_phy_qmp_14nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
>>  };
>>
>>  static struct ufs_qcom_phy_specific_ops phy_14nm_ops = {
>> -   .calibrate_phy  = ufs_qcom_phy_qmp_14nm_phy_calibrate,
>> .start_serdes   = ufs_qcom_phy_qmp_14nm_start_serdes,
>> .is_physical_coding_sublayer_ready =
>> ufs_qcom_phy_qmp_14nm_is_pcs_ready,
>> .set_tx_lane_enable =
>> ufs_qcom_phy_qmp_14nm_set_tx_lane_enable,
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> index 5c18c41dbdb4..5705a2d4c6d2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
>> @@ -63,7 +63,13 @@ void ufs_qcom_phy_qmp_20nm_advertise_quirks(struct
>> ufs_qcom_phy *phy_common)
>>
>>  static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy)
>>  {
>> -   return 0;
>> +   struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
>> +   bool is_rate_B = false;
>> +
>> +   if (phy_common->mode == PHY_MODE_UFS_HS_B)
>> +   is_rate_B = true;
>> +
>> +   return ufs_qcom_phy_qmp_20nm_phy_calibrate(phy_common, is_rate_B);
>>  }
>>
>>  static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)
>> @@ -178,7 +184,6 @@ static int
>> ufs_qcom_phy_qmp_20nm_is_pcs_ready(struct ufs_qcom_phy *phy_common)
>>  };
>>
>>  static struct ufs_qcom_phy_specific_ops phy_20nm_ops = {
>> -   .calibrate_phy  = ufs_qcom_phy_qmp_20nm_phy_calibrate,
>> .start_serdes   = ufs_qcom_phy_qmp_20nm_start_serdes,
>> .is_physical_coding_sublayer_ready =
>> ufs_qcom_phy_qmp_20nm_is_pcs_ready,
>> .set_tx_lane_enable =
>> ufs_qcom_phy_qmp_20nm_set_tx_lane_enable,
>> diff --git a/drivers/phy/qualcomm/phy-qcom-ufs.c
>> b/drivers/phy/qualcomm/phy-qcom-ufs.c
>> index 43865ef340e2..1febe3294fe3 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-ufs.c
>> +++ 

Re: [PATCH V6 2/6] block: tracking request allocation with q_usage_counter

[Hannes Reinecke]

On 09/27/2017 07:48 AM, Ming Lei wrote:
> This usage is basically same with blk-mq, so that we can
> support to freeze legacy queue easily.
> 
> Also 'wake_up_all(>mq_freeze_wq)' has to be moved
> into blk_set_queue_dying() since both legacy and blk-mq
> may wait on the wait queue of .mq_freeze_wq.
> 
> Signed-off-by: Ming Lei 
> ---
>  block/blk-core.c | 14 ++
>  block/blk-mq.c   |  7 ---
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
As indicated in the other (similar) patch from Bart, we have a customer
report running into a q_usage_counter underflow with legacy-sq.
So this patch is actually a bugfix.

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)