[PATCH 1/2] xhci: Add broken-streams quirk for Fresco Logic FL1000G xhci controllers
Streams do not work reliabe on Fresco Logic FL1000G xhci controllers, trying to use them results in errors like this: 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 0500 01078001 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 0500 01038001 As always I've ordered a pci-e addon card with a Fresco Logic controller for myself to see if I can come up with a better fix then the big hammer, in the mean time this will make uas devices work again (in usb-storage mode) for FL1000G users. Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede --- drivers/usb/host/xhci-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 9a69b1f..e1071ce 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -82,6 +82,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) "must be suspended extra slowly", pdev->revision); } + if (pdev->device == PCI_DEVICE_ID_FRESCO_LOGIC_PDK) + xhci->quirks |= XHCI_BROKEN_STREAMS; /* Fresco Logic confirms: all revisions of this chip do not * support MSI, even though some of them claim to in their PCI * capabilities. -- 2.1.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
[PATCH 2/2] uas: Add US_FL_NO_ATA_1X for Seagate devices with usb-id 0bc2:a013
This is yet another Seagate device which needs the US_FL_NO_ATA_1X quirk Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.16 Signed-off-by: Hans de Goede --- drivers/usb/storage/unusual_uas.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/usb/storage/unusual_uas.h b/drivers/usb/storage/unusual_uas.h index 18a283d..2918376 100644 --- a/drivers/usb/storage/unusual_uas.h +++ b/drivers/usb/storage/unusual_uas.h @@ -68,6 +68,13 @@ UNUSUAL_DEV(0x0bc2, 0xa003, 0x, 0x, USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_NO_ATA_1X), +/* Reported-by: Marcin Zajączkowski */ +UNUSUAL_DEV(0x0bc2, 0xa013, 0x, 0x, + "Seagate", + "Backup Plus", + USB_SC_DEVICE, USB_PR_DEVICE, NULL, + US_FL_NO_ATA_1X), + /* https://bbs.archlinux.org/viewtopic.php?id=183190 */ UNUSUAL_DEV(0x0bc2, 0xab20, 0x, 0x, "Seagate", -- 2.1.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 1/2] xhci: Add broken-streams quirk for Fresco Logic FL1000G xhci controllers
Hello. On 12/5/2014 1:11 PM, Hans de Goede wrote: Streams do not work reliabe on Fresco Logic FL1000G xhci controllers, s/reliabe/reliably/? trying to use them results in errors like this: 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3570 9067b000 0500 01078001 21:37:33 kernel: xhci_hcd :04:00.0: ERROR Transfer event for disabled endpoint or incorrect stream ring 21:37:33 kernel: xhci_hcd :04:00.0: @368b3580 9067b400 0500 01038001 As always I've ordered a pci-e addon card with a Fresco Logic controller for myself to see if I can come up with a better fix then the big hammer, in the mean time this will make uas devices work again (in usb-storage mode) for FL1000G users. Reported-by: Marcin Zajączkowski Cc: sta...@vger.kernel.org # 3.15 Signed-off-by: Hans de Goede WBR, Sergei -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
The commit 7985090aa0201(sd: Disable discard_zeroes_data for UNMAP)
introduces regresion for QEMU SCSI.
QEMU SCSI device claims to support UNMAP, WRITE SAME and WRITE SAME
16 in LBP VPD page, but only provides "Maximum unmap LBA count"
in block limits VPD page, and "Maximum write same length" isn't set.
The default max_discard_sectors(SD_MAX_WS16_BLOCKS) can't work at
all since it is much bigger than the actual Maximum unmap LBA count.
This patch trys to fix the regression by setting 'max_ws_blocks'
as 'max_unmap_blocks' when block limits VPD page doesn't provide
"Maximum write same length" under the situation. This approach is
reasonable because device server supports the use of the WRITE
SAME or WRITE SAME 10 command to unmap LBAs when LBPWS or LBPWS10
is set in LBP VPD page.
Cc: Martin K. Petersen
Cc: Paolo Bonzini
Cc: Christoph Hellwig
Signed-off-by: Ming Lei
---
drivers/scsi/sd.c |8
1 file changed, 8 insertions(+)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index fedab3c..2a69fee 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2624,6 +2624,14 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
} else {/* LBP VPD page tells us what to use */
+ /*
+* Borrow max_unmap_blocks if max_ws_blocks isn't
+* provided from block limits VPD page
+*/
+ if ((sdkp->lbpws10 || sdkp->lbpws) &&
+ !sdkp->max_ws_blocks)
+ sdkp->max_ws_blocks = sdkp->max_unmap_blocks;
+
if (sdkp->lbpws)
sd_config_discard(sdkp, SD_LBP_WS16);
else if (sdkp->lbpws10)
--
1.7.9.5
--
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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
> "Ming" == Ming Lei writes: Ming, Ming> QEMU SCSI device claims to support UNMAP, WRITE SAME and WRITE Ming> SAME 16 in LBP VPD page, but only provides "Maximum unmap LBA Ming> count" in block limits VPD page, and "Maximum write same length" Ming> isn't set. That really sounds like a problem that should be fixed in QEMU SCSI. Ming> The default max_discard_sectors(SD_MAX_WS16_BLOCKS) can't work at Ming> all since it is much bigger than the actual Maximum unmap LBA Ming> count. There is absolutely no correlation between max write same blocks and max unmap blocks. They are two entirely different commands. If QEMU SCSI advertises support for WRITE SAME(16) and does not report a cap in the block limits VPD then we must expect that it supports the maximum number of blocks that can be expressed by the command. -- 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] SCSI: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On Fri, Dec 5, 2014 at 8:56 PM, Martin K. Petersen wrote: >> "Ming" == Ming Lei writes: > > Ming, > > Ming> QEMU SCSI device claims to support UNMAP, WRITE SAME and WRITE > Ming> SAME 16 in LBP VPD page, but only provides "Maximum unmap LBA > Ming> count" in block limits VPD page, and "Maximum write same length" > Ming> isn't set. > > That really sounds like a problem that should be fixed in QEMU SCSI. It can be fixed, but there are lots of old QEMU in production. > > Ming> The default max_discard_sectors(SD_MAX_WS16_BLOCKS) can't work at > Ming> all since it is much bigger than the actual Maximum unmap LBA > Ming> count. > > There is absolutely no correlation between max write same blocks and max > unmap blocks. They are two entirely different commands. If QEMU SCSI Do you have any better idea for the problem? > advertises support for WRITE SAME(16) and does not report a cap in the > block limits VPD then we must expect that it supports the maximum number > of blocks that can be expressed by the command. Unfortunately it doesn't support that, see below log: [ 50.112885] sd 0:0:1:0: [sda] FAILED Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [ 50.113859] sd 0:0:1:0: [sda] Sense Key : Illegal Request [current] [ 50.113859] sd 0:0:1:0: [sda] Add. Sense: Invalid field in cdb [ 50.113859] sd 0:0:1:0: [sda] CDB: [ 50.113859] Write same(16): 93 08 00 00 00 00 00 00 80 00 00 40 00 00 00 00 [ 50.113859] blk_update_request: critical target error, dev sda, sector 32768 Thanks, Ming Lei -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
> "Ming" == Ming Lei writes: >> There is absolutely no correlation between max write same blocks and >> max unmap blocks. They are two entirely different commands. If QEMU >> SCSI Ming> Do you have any better idea for the problem? Does QEMU SCSI set LBPRZ? -- 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] SCSI: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On Fri, Dec 5, 2014 at 9:22 PM, Martin K. Petersen wrote: >> "Ming" == Ming Lei writes: > >>> There is absolutely no correlation between max write same blocks and >>> max unmap blocks. They are two entirely different commands. If QEMU >>> SCSI > > Ming> Do you have any better idea for the problem? > > Does QEMU SCSI set LBPRZ? It isn't set in LBP VPG page. Thanks, -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
> "Ming" == Ming Lei writes: Ming> It isn't set in LBP VPG page. What about in READ CAPACITY(16)? -- 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] SCSI: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On Fri, Dec 5, 2014 at 9:38 PM, Martin K. Petersen wrote: >> "Ming" == Ming Lei writes: > > Ming> It isn't set in LBP VPG page. > > What about in READ CAPACITY(16)? It isn't set too. Thanks, Ming Lei -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
> "Ming" == Ming Lei writes:
>> What about in READ CAPACITY(16)?
Ming> It isn't set too.
Please try the following patch:
[SCSI] sd: Tweak discard heuristics to work around QEMU SCSI issue
7985090aa020 changed the discard heuristics to give preference to the
WRITE SAME commands that (unlike UNMAP) guarantee deterministic results.
Ming Lei discovered that QEMU SCSI's WRITE SAME implementation
internally relied on limits that were only communicated for the UNMAP
case. And therefore discard commands backed by WRITE SAME would fail.
Tweak the heuristics so we still pick UNMAP in the LBPRZ=0 case and only
prefer the WRITE SAME variants if the device has the LBPRZ flag set.
Reported-by: Ming Lei
Signed-off-by: Martin K. Petersen
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 95bfb7bfbb9d..76c5d1388417 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2623,8 +2623,9 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
sd_config_discard(sdkp, SD_LBP_WS16);
} else {/* LBP VPD page tells us what to use */
-
- if (sdkp->lbpws)
+ if (sdkp->lbpu && sdkp->max_unmap_blocks &&
!sdkp->lbprz)
+ sd_config_discard(sdkp, SD_LBP_UNMAP);
+ else if (sdkp->lbpws)
sd_config_discard(sdkp, SD_LBP_WS16);
else if (sdkp->lbpws10)
sd_config_discard(sdkp, SD_LBP_WS10);
--
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] SCSI: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On Fri, Dec 5, 2014 at 9:58 PM, Martin K. Petersen
wrote:
>> "Ming" == Ming Lei writes:
>
>>> What about in READ CAPACITY(16)?
>
> Ming> It isn't set too.
>
> Please try the following patch:
>
>
> [SCSI] sd: Tweak discard heuristics to work around QEMU SCSI issue
>
> 7985090aa020 changed the discard heuristics to give preference to the
> WRITE SAME commands that (unlike UNMAP) guarantee deterministic results.
>
> Ming Lei discovered that QEMU SCSI's WRITE SAME implementation
> internally relied on limits that were only communicated for the UNMAP
I just found it should be one QEMU bug for emulating write same, but
limiting write same sectors number as max_unmap_blocks can
workaround the issue.
> case. And therefore discard commands backed by WRITE SAME would fail.
>
> Tweak the heuristics so we still pick UNMAP in the LBPRZ=0 case and only
> prefer the WRITE SAME variants if the device has the LBPRZ flag set.
>
> Reported-by: Ming Lei
> Signed-off-by: Martin K. Petersen
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 95bfb7bfbb9d..76c5d1388417 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2623,8 +2623,9 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
> sd_config_discard(sdkp, SD_LBP_WS16);
>
> } else {/* LBP VPD page tells us what to use */
> -
> - if (sdkp->lbpws)
> + if (sdkp->lbpu && sdkp->max_unmap_blocks &&
> !sdkp->lbprz)
> + sd_config_discard(sdkp, SD_LBP_UNMAP);
> + else if (sdkp->lbpws)
> sd_config_discard(sdkp, SD_LBP_WS16);
> else if (sdkp->lbpws10)
> sd_config_discard(sdkp, SD_LBP_WS10);
Looks it does work.
Thanks,
Ming Lei
--
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 v4] scsi: ufs: add support of generic PHY and ICE in Qualcomm chips
On Thursday 04 December 2014 09:24 PM, Christoph Hellwig wrote: > On Thu, Nov 27, 2014 at 05:59:58PM +0200, Yaniv Gardi wrote: >> In this change we add support to the generic PHY framework. >> Two UFS phys are implemented: >> qmp-20nm and qmp-28nm. >> >> Also, the files in this change implement the UFS HW (controller & PHY) >> specific behavior in Qualcomm chips. >> Relocation of a few header files is needed in order to expose routines >> and data structures between PHY driver and UFS driver. >> >> Also, this change include the implementation of Inline Crypto Engine (ICE) >> in Qualcomm chips. > > This whole patch is a mess. It does way to many things in one patch, > and it doesn't explain enough of it. > > Please explain why you need it. Especially as the PHY API is a generic > phy abstraction, so having to share defintions between the provider and > consumer seems wrong. Even if you need some shared bits keep them to an > absolute minium insted of moving so much out of the driver directory. > Also if at all possible keep the shared data in a single header under > include/linux instead of having lots of global headers in a deep > directory structure. > > Second split this into patches that do a single things, and explain why > you're doing each: > > 1) header move if/as needed > 2) add 20nm phy driver > 3) add 28nm phy driver > 4) add ufs-qcom driver > 5) add ufs-qcom-ice support > > and so on. +1 -Kishon -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On 05/12/2014 14:58, Martin K. Petersen wrote:
>> "Ming" == Ming Lei writes:
>
>>> What about in READ CAPACITY(16)?
>
> Ming> It isn't set too.
>
> Please try the following patch:
>
>
> [SCSI] sd: Tweak discard heuristics to work around QEMU SCSI issue
>
> 7985090aa020 changed the discard heuristics to give preference to the
> WRITE SAME commands that (unlike UNMAP) guarantee deterministic results.
>
> Ming Lei discovered that QEMU SCSI's WRITE SAME implementation
> internally relied on limits that were only communicated for the UNMAP
> case. And therefore discard commands backed by WRITE SAME would fail.
>
> Tweak the heuristics so we still pick UNMAP in the LBPRZ=0 case and only
> prefer the WRITE SAME variants if the device has the LBPRZ flag set.
>
> Reported-by: Ming Lei
> Signed-off-by: Martin K. Petersen
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 95bfb7bfbb9d..76c5d1388417 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2623,8 +2623,9 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
> sd_config_discard(sdkp, SD_LBP_WS16);
>
> } else {/* LBP VPD page tells us what to use */
> -
> - if (sdkp->lbpws)
> + if (sdkp->lbpu && sdkp->max_unmap_blocks &&
> !sdkp->lbprz)
> + sd_config_discard(sdkp, SD_LBP_UNMAP);
> + else if (sdkp->lbpws)
> sd_config_discard(sdkp, SD_LBP_WS16);
> else if (sdkp->lbpws10)
> sd_config_discard(sdkp, SD_LBP_WS10);
>
This is the right fix. Ming, how do you reproduce the QEMU bug?
Acked-by: Paolo Bonzini
--
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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On 05/12/2014 14:05, Ming Lei wrote: > > [ 50.112885] sd 0:0:1:0: [sda] FAILED Result: hostbyte=DID_OK > driverbyte=DRIVER_SENSE > [ 50.113859] sd 0:0:1:0: [sda] Sense Key : Illegal Request [current] > [ 50.113859] sd 0:0:1:0: [sda] Add. Sense: Invalid field in cdb > [ 50.113859] sd 0:0:1:0: [sda] CDB: > [ 50.113859] Write same(16): 93 08 00 00 00 00 00 00 80 00 00 40 00 00 00 00 > [ 50.113859] blk_update_request: critical target error, dev sda, sector > 32768 So this command is zeroing 2^22 sectors (2GB) starting from sector 128. How did you run QEMU and what command produced this request? Paolo -- 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: SD: set max_ws_blocks as max_unmap_blocks if it isn't provided
On Fri, Dec 5, 2014 at 11:49 PM, Paolo Bonzini wrote: > > > On 05/12/2014 14:05, Ming Lei wrote: >> >> [ 50.112885] sd 0:0:1:0: [sda] FAILED Result: hostbyte=DID_OK >> driverbyte=DRIVER_SENSE >> [ 50.113859] sd 0:0:1:0: [sda] Sense Key : Illegal Request [current] >> [ 50.113859] sd 0:0:1:0: [sda] Add. Sense: Invalid field in cdb >> [ 50.113859] sd 0:0:1:0: [sda] CDB: >> [ 50.113859] Write same(16): 93 08 00 00 00 00 00 00 80 00 00 40 00 00 00 >> 00 >> [ 50.113859] blk_update_request: critical target error, dev sda, sector >> 32768 > > So this command is zeroing 2^22 sectors (2GB) starting from sector 128. > How did you run QEMU and what command produced this request? It was found in xfstests, and turns out it is a QEMU INT_MAX bug. Thanks -- 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/3] libata: Whitelist SSDs that are known to properly return zeroes after TRIM
On 07/11/2014 06:08, Martin K. Petersen wrote:
> The whitelist is only meant as a starting point and is by no means
> comprehensive:
>
>- All intel SSD models except for 510
>- Micron M5*
>- Samsung SSDs
>- Seagate SSDs
>
> Signed-off-by: Martin K. Petersen
> ---
> drivers/ata/libata-core.c | 18 ++
> drivers/ata/libata-scsi.c | 10 ++
> include/linux/libata.h| 1 +
> 3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index c5ba15af87d3..f41f24a8bc21 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -4225,10 +4225,20 @@ static const struct ata_blacklist_entry
> ata_device_blacklist [] = {
> { "PIONEER DVD-RW DVR-216D", NULL, ATA_HORKAGE_NOSETXFER },
>
> /* devices that don't properly handle queued TRIM commands */
> - { "Micron_M500*", NULL, ATA_HORKAGE_NO_NCQ_TRIM, },
> - { "Crucial_CT???M500SSD*", NULL, ATA_HORKAGE_NO_NCQ_TRIM, },
> - { "Micron_M550*", NULL, ATA_HORKAGE_NO_NCQ_TRIM, },
> - { "Crucial_CT*M550SSD*",NULL, ATA_HORKAGE_NO_NCQ_TRIM, },
> + { "Micron_M5?0*", NULL, ATA_HORKAGE_NO_NCQ_TRIM |
> + ATA_HORKAGE_ZERO_AFTER_TRIM, },
> + { "Crucial_CT???M5?0SSD*", NULL, ATA_HORKAGE_NO_NCQ_TRIM, },
I have a Crucial_CT256MX1 (i.e. MX100) and it does reliably zero.
BTW. it's the same hardware as the M550, so probably the same set of
quirks should apply to both.
Paolo
> +
> + /*
> + * DRAT/RZAT are weak guarantees. Explicitly black/whitelist
> + * SSDs that provide reliable zero after TRIM.
> + */
> + { "INTEL*SSDSC2MH*",NULL, 0, }, /* Blacklist intel 510 */
> + { "INTEL*SSD*", NULL, ATA_HORKAGE_ZERO_AFTER_TRIM, },
> + { "SSD*INTEL*", NULL, ATA_HORKAGE_ZERO_AFTER_TRIM, },
> + { "Samsung*SSD*", NULL, ATA_HORKAGE_ZERO_AFTER_TRIM, },
> + { "SAMSUNG*SSD*", NULL, ATA_HORKAGE_ZERO_AFTER_TRIM, },
> + { "ST[1248][0248]0[FH]*", NULL, ATA_HORKAGE_ZERO_AFTER_TRIM, },
>
> /*
>* Some WD SATA-I drives spin up and down erratically when the link
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 0586f66d70fa..deaa6e34ed4d 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2515,13 +2515,15 @@ static unsigned int ata_scsiop_read_cap(struct
> ata_scsi_args *args, u8 *rbuf)
> rbuf[15] = lowest_aligned;
>
> if (ata_id_has_trim(args->id)) {
> - rbuf[14] |= 0x80; /* TPE */
> + rbuf[14] |= 0x80; /* LBPME */
>
> - if (ata_id_has_zero_after_trim(args->id))
> - rbuf[14] |= 0x40; /* TPRZ */
> + if (ata_id_has_zero_after_trim(args->id) &&
> + dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
> + ata_dev_warn(dev, "Enabling
> discard_zeroes_data\n");
> + rbuf[14] |= 0x40; /* LBPRZ */
> + }
> }
> }
> -
> return 0;
> }
>
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index bd5fefeaf548..45ac825b8366 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -421,6 +421,7 @@ enum {
> ATA_HORKAGE_NO_NCQ_TRIM = (1 << 19),/* don't use queued TRIM */
> ATA_HORKAGE_NOLPM = (1 << 20),/* don't use LPM */
> ATA_HORKAGE_WD_BROKEN_LPM = (1 << 21), /* some WDs have broken LPM */
> + ATA_HORKAGE_ZERO_AFTER_TRIM = (1 << 22),/* guarantees zero after trim */
>
>/* DMA mask for user DMA control: User visible values; DO NOT
> renumber */
>
--
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 2/4] [SCSI] Blacklist RSOC for Microsoft iSCSI target devices
On 12/4/14, 8:17 PM, Martin K. Petersen wrote: "Mike" == Mike Christie writes: Mike> In case other people test this patch, I wanted to warn people that Mike> the MS iSCSI target does the same sequence (sends reject PDU then Mike> drops the connection on us) for any command it does not Mike> support. We end up seeing the same problem for other commands. Ick. Have you pinged MS about this again recently? Doesn't sound like we'd be the only ones tripping over something like this. I have not. Until the other day in this thread, I thought they fixed it in the newer target versions. I just noticed this behavior was the general error handling behavior for all unknown commands yesterday when testing your patch. Trying to write up an analysis about why they should return a ILLEGAL REQUEST and not drop the connection for unsupported commands. -- 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_scan: Send TEST UNIT READY to LUN0 before LUN scanning
On 12/05/2014 12:43 AM, Sebastian Herbszt wrote:
> Hannes Reinecke wrote:
>> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
>> index 49014a1..4f446e3 100644
>> --- a/drivers/scsi/scsi_devinfo.c
>> +++ b/drivers/scsi/scsi_devinfo.c
>> @@ -166,6 +166,7 @@ static struct {
>> {"easyRAID", "X6P", NULL, BLIST_NOREPORTLUN},
>> {"easyRAID", "F8", NULL, BLIST_NOREPORTLUN},
>> {"FSC", "CentricStor", "*", BLIST_SPARSELUN | BLIST_LARGELUN},
>> +{"FUJITSU", "ETERNUS_DX", "*", BLIST_TEST_LUN0 },
>> {"Generic", "USB SD Reader", "1.00", BLIST_FORCELUN | BLIST_INQUIRY_36},
>> {"Generic", "USB Storage-SMC", "0180", BLIST_FORCELUN |
>> BLIST_INQUIRY_36},
>> {"Generic", "USB Storage-SMC", "0207", BLIST_FORCELUN |
>> BLIST_INQUIRY_36},
>
> Does this apply to all ETERNUS DX models and generations?
>
So I've been given to understand. Matthias Roessler will have details.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (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] scsi_scan: Send TEST UNIT READY to LUN0 before LUN scanning
On 12/05/2014 01:19 AM, James Bottomley wrote:
> On Thu, 2014-12-04 at 17:39 +0100, Hannes Reinecke wrote:
> [...]
>> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
>> index 0af7133..ca39e32 100644
>> --- a/drivers/scsi/scsi_scan.c
>> +++ b/drivers/scsi/scsi_scan.c
>> @@ -119,6 +119,13 @@ MODULE_PARM_DESC(inq_timeout,
>> "Timeout (in seconds) waiting for devices to answer INQUIRY."
>> " Default is 20. Some devices may need more; most need less.");
>>
>> +static unsigned int scsi_scan_timeout = SCSI_TIMEOUT/HZ + 58;
>> +
>> +module_param_named(scan_timeout, scsi_scan_timeout, uint, S_IRUGO|S_IWUSR);
>> +MODULE_PARM_DESC(scan_timeout,
>> + "Timeout (in seconds) waiting for devices to become ready"
>> + " after INQUIRY. Default is 60.");
>> +
>> /* This lock protects only this list */
>> static DEFINE_SPINLOCK(async_scan_lock);
>> static LIST_HEAD(scanning_hosts);
>> @@ -719,19 +726,6 @@ static int scsi_probe_lun(struct scsi_device *sdev,
>> unsigned char *inq_result,
>> }
>>
>> /*
>> - * Related to the above issue:
>> - *
>> - * XXX Devices (disk or all?) should be sent a TEST UNIT READY,
>> - * and if not ready, sent a START_STOP to start (maybe spin up) and
>> - * then send the INQUIRY again, since the INQUIRY can change after
>> - * a device is initialized.
>
> You're not sending a start_stop unit, so why is this being deleted? Any
> unstarted unit will still return initialization command required and may
> or may not return lun data.
>
Hmm. Okay.
>> - *
>> - * Ideally, start a device if explicitly asked to do so. This
>> - * assumes that a device is spun up on power on, spun down on
>> - * request, and then spun up on request.
>> - */
>> -
>> -/*
>> * The scanning code needs to know the scsi_level, even if no
>> * device is attached at LUN 0 (SCSI_SCAN_TARGET_PRESENT) so
>> * non-zero LUNs can be scanned.
>> @@ -756,6 +750,65 @@ static int scsi_probe_lun(struct scsi_device *sdev,
>> unsigned char *inq_result,
>> }
>>
>> /**
>> + * scsi_test_lun - waiting for a LUN to become ready
>> + * @sdev: scsi_device to test
>> + *
>> + * Description:
>> + * Wait for the lun associated with @sdev to become ready
>> + *
>> + * Send a TEST UNIT READY to detect any unit attention conditions.
>> + * Retry TEST UNIT READY for up to @scsi_scan_timeout if the
>> + * returned sense key is 02/04/01 (Not ready, Logical Unit is
>> + * in process of becoming ready)
>> + **/
>> +static int
>> +scsi_test_lun(struct scsi_device *sdev)
>> +{
>> +struct scsi_sense_hdr sshdr;
>> +int res = SCSI_SCAN_TARGET_PRESENT;
>> +int tur_result;
>> +unsigned long tur_timeout = jiffies + scsi_scan_timeout * HZ;
>> +
>> +/* Skip for older devices */
>> +if (sdev->scsi_level <= SCSI_3)
>> +return SCSI_SCAN_LUN_PRESENT;
>> +
>> +/*
>> + * Wait for the device to become ready.
>> + *
>> + * Some targets take some time before the firmware is
>> + * fully initialized, during which time they might not
>> + * be able to fill out any REPORT_LUN command correctly.
>> + * And as we're not capable of handling the
>> + * INQUIRY DATA CHANGED unit attention correctly we'd
>> + * rather wait here.
>
> Can't we just fix that? All you need is rescan to re-read the inquiry
> data.
>
In theory, yes.
However, there is this nasty issue with UA preference, so a
Power-On/Reset UA might have obscured this one.
Plus we would need to enable it in general, in which case it would
also be run if someone unmapped LUNs on the array.
At which point we would have to _delete_ scsi devices on the fly,
something I'm not really comfortable with.
At least _not_ without quite some testing.
Besides, correct UA handling would be a good topic for LSF.
There _are_ areas where we could improve.
>> + */
>> +do {
>> +tur_result = scsi_test_unit_ready(sdev, SCSI_TIMEOUT,
>> + 3, &sshdr);
>> +if (!tur_result) {
>> +res = SCSI_SCAN_LUN_PRESENT;
>> +break;
>> +}
>> +if ((driver_byte(tur_result) & DRIVER_SENSE) &&
>> +scsi_sense_valid(&sshdr)) {
>> +SCSI_LOG_SCAN_BUS(3, sdev_printk(KERN_INFO, sdev,
>> +"scsi_scan: tur returned %02x/%02x/%02x\n",
>> +sshdr.sense_key, sshdr.asc, sshdr.ascq));
>
> If we're waiting 60s and coming in here every 100ms we'll get 600 of
> these in the log ... that's going to drown out anything that came before
> it.
>
Ok, I'll be switching to _ratelimit here.
>> +if (sshdr.sense_key == NOT_READY &&
>> +sshdr.asc == 0x04 && sshdr.ascq == 0x01) {
>> +/* Logical Unit is in process
>> +
[PATCH V5 0/3] scsi: Configure number of LUs reported by 'report-luns'
This patch set retrieves the number of LUs available on a target using the report-luns command by re-sizing the returned data buffer and retrying report luns. A minor bug fix is included. scsi_mod parameter max_report_luns is no longer used and is removed. Changes from previous posting: - simplify implementation by removing max_report_luns parameter and removing 2nd kmalloc failure handling - remove patch to fix return value on first kmalloc failing - drop do_scsi_report_luns encapsulation as it isn't required with new implementation Rob Evers (3): scsi: Avoid unnecessary GFP_ATOMIC allocation in scsi_report_lun_scan scsi: Use set/get_unaligned_be32 in report_luns scsi: Retry report-luns when reported LU count requres more memory drivers/scsi/scsi_scan.c | 54 ++-- 1 file changed, 15 insertions(+), 39 deletions(-) -- 1.8.3.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
[PATCH V5 1/3] scsi: Avoid unnecessary GFP_ATOMIC allocation in scsi_report_lun_scan
Signed-off-by: Rob Evers
---
drivers/scsi/scsi_scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index ba3f1e8..e460b35 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1408,7 +1408,7 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
* prevent us from finding any LUNs on this target.
*/
length = (max_scsi_report_luns + 1) * sizeof(struct scsi_lun);
- lun_data = kmalloc(length, GFP_ATOMIC |
+ lun_data = kmalloc(length, GFP_KERNEL |
(sdev->host->unchecked_isa_dma ? __GFP_DMA : 0));
if (!lun_data) {
printk(ALLOC_FAILURE_MSG, __func__);
--
1.8.3.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
[PATCH V5 2/3] scsi: Use set/get_unaligned_be32 in report_luns
---
drivers/scsi/scsi_scan.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e460b35..8db1f6f 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include
#include
@@ -1359,7 +1360,6 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
unsigned int retries;
int result;
struct scsi_lun *lunp, *lun_data;
- u8 *data;
struct scsi_sense_hdr sshdr;
struct scsi_device *sdev;
struct Scsi_Host *shost = dev_to_shost(&starget->dev);
@@ -1425,10 +1425,7 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
/*
* bytes 6 - 9: length of the command.
*/
- scsi_cmd[6] = (unsigned char) (length >> 24) & 0xff;
- scsi_cmd[7] = (unsigned char) (length >> 16) & 0xff;
- scsi_cmd[8] = (unsigned char) (length >> 8) & 0xff;
- scsi_cmd[9] = (unsigned char) length & 0xff;
+ put_unaligned_be32(length, &scsi_cmd[6]);
scsi_cmd[10] = 0; /* reserved */
scsi_cmd[11] = 0; /* control */
@@ -1476,9 +1473,7 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
/*
* Get the length from the first four bytes of lun_data.
*/
- data = (u8 *) lun_data->scsi_lun;
- length = ((data[0] << 24) | (data[1] << 16) |
- (data[2] << 8) | (data[3] << 0));
+ length = get_unaligned_be32(lun_data->scsi_lun);
num_luns = (length / sizeof(struct scsi_lun));
if (num_luns > max_scsi_report_luns) {
--
1.8.3.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
[PATCH V5 3/3] scsi: Retry report-luns when reported LU count requres more memory
Update scsi_report_lun_scan to initially always report up to 511 LUs,
as the previous default max_report_luns did. Retry in a loop if not
enough memory is available for the number of LUs reported. Parameter
max_report_luns is removed as it is no longer used.
---
drivers/scsi/scsi_scan.c | 43 ---
1 file changed, 12 insertions(+), 31 deletions(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 8db1f6f..ecd8703 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -80,6 +80,7 @@
static const char *scsi_null_device_strs = "nullnullnullnull";
+#define INITIAL_MAX_SCSI_REPORT_LUNS 511
#define MAX_SCSI_LUNS 512
static u64 max_scsi_luns = MAX_SCSI_LUNS;
@@ -99,20 +100,6 @@ char scsi_scan_type[6] = SCSI_SCAN_TYPE_DEFAULT;
module_param_string(scan, scsi_scan_type, sizeof(scsi_scan_type), S_IRUGO);
MODULE_PARM_DESC(scan, "sync, async or none");
-/*
- * max_scsi_report_luns: the maximum number of LUNS that will be
- * returned from the REPORT LUNS command. 8 times this value must
- * be allocated. In theory this could be up to an 8 byte value, but
- * in practice, the maximum number of LUNs suppored by any device
- * is about 16k.
- */
-static unsigned int max_scsi_report_luns = 511;
-
-module_param_named(max_report_luns, max_scsi_report_luns, uint,
S_IRUGO|S_IWUSR);
-MODULE_PARM_DESC(max_report_luns,
-"REPORT LUNS maximum number of LUNS received (should be"
-" between 1 and 16384)");
-
static unsigned int scsi_inq_timeout = SCSI_TIMEOUT/HZ + 18;
module_param_named(inq_timeout, scsi_inq_timeout, uint, S_IRUGO|S_IWUSR);
@@ -1399,15 +1386,10 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
/*
* Allocate enough to hold the header (the same size as one scsi_lun)
-* plus the max number of luns we are requesting.
-*
-* Reallocating and trying again (with the exact amount we need)
-* would be nice, but then we need to somehow limit the size
-* allocated based on the available memory and the limits of
-* kmalloc - we don't want a kmalloc() failure of a huge value to
-* prevent us from finding any LUNs on this target.
+* plus the number of luns we are requesting.
*/
- length = (max_scsi_report_luns + 1) * sizeof(struct scsi_lun);
+ length = (INITIAL_MAX_SCSI_REPORT_LUNS + 1) * sizeof(struct scsi_lun);
+retry:
lun_data = kmalloc(length, GFP_KERNEL |
(sdev->host->unchecked_isa_dma ? __GFP_DMA : 0));
if (!lun_data) {
@@ -1473,17 +1455,16 @@ static int scsi_report_lun_scan(struct scsi_target
*starget, int bflags,
/*
* Get the length from the first four bytes of lun_data.
*/
- length = get_unaligned_be32(lun_data->scsi_lun);
+ if (get_unaligned_be32(lun_data->scsi_lun) +
+ sizeof(struct scsi_lun) > length) {
+ length = get_unaligned_be32(lun_data->scsi_lun) +
+sizeof(struct scsi_lun);
+ kfree(lun_data);
+ goto retry;
+ } else
+ length = get_unaligned_be32(lun_data->scsi_lun);
num_luns = (length / sizeof(struct scsi_lun));
- if (num_luns > max_scsi_report_luns) {
- sdev_printk(KERN_WARNING, sdev,
- "Only %d (max_scsi_report_luns)"
- " of %d luns reported, try increasing"
- " max_scsi_report_luns.\n",
- max_scsi_report_luns, num_luns);
- num_luns = max_scsi_report_luns;
- }
SCSI_LOG_SCAN_BUS(3, sdev_printk (KERN_INFO, sdev,
"scsi scan: REPORT LUN scan\n"));
--
1.8.3.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 1/3] libata: Whitelist SSDs that are known to properly return zeroes after TRIM
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Martin K. Petersen > Sent: Thursday, 06 November, 2014 11:08 PM > To: linux-scsi@vger.kernel.org; linux-...@vger.kernel.org; linux- > fsde...@vger.kernel.org; ne...@suse.de > Cc: Martin K. Petersen > Subject: [PATCH 1/3] libata: Whitelist SSDs that are known to properly return > zeroes after TRIM > > As defined, the DRAT (Deterministic Read After Trim) and RZAT (Return > Zero After Trim) flags in the ATA Command Set are unreliable in the > sense that they only define what happens if the device successfully > executed the DSM TRIM command. TRIM is only advisory, however, and the > device is free to silently ignore all or parts of the request. > > In practice this renders the DRAT and RZAT flags completely useless and > because the results are unpredictable we decided to disable discard in > MD for 3.18 to avoid the risk of data corruption. > > Hardware vendors in the real world obviously need better guarantees than > what the standards bodies provide. Unfortuntely those guarantees are > encoded in product requirements documents rather than somewhere we can > key off of them programatically. So we are compelled to disabling > discard_zeroes_data for all devices unless we explicitly have data to > support whitelisting them. > > This patch whitelists SSDs from a few of the main vendors. None of the > whitelists are based on written guarantees. They are purely based on > empirical evidence collected from internal and external users that have > tested or qualified these drives in RAID deployments. > > The whitelist is only meant as a starting point and is by no means > comprehensive: > >- All intel SSD models except for 510 >- Micron M5* >- Samsung SSDs >- Seagate SSDs > That description and Paolo's reply: > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Paolo Bonzini > Sent: Friday, 05 December, 2014 10:45 AM ... > I have a Crucial_CT256MX1 (i.e. MX100) and it does reliably zero. make me concerned about this whitelist approach. I think you need a manufacturer assertion that this is indeed the design intent; you cannot be certain based on observation from outside. Since the SCSI and ATA standards allow ignoring the hint, it might be honored most of the time, but ignored in some rare cases (e.g., drive firmware has a malloc() failure that only happens when the drive is handling an overtemperature condition and six other problems at the same time). Maybe there should be two levels: * vendor asserts the drive is designed to always honor the hint * community observes the drive always seems to honor the hint and a sysfs flag for users to select the level at which they feel safe. A user running 3 replicas of the data in different sites might be more trusting than a user for which this is the only copy of the data. --- Rob ElliottHP Server Storage -- 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 RESEND 2/5] SES: generate KOBJ_CHANGE on enclosure attach
From: Dan Williams In support of a /dev/disk/by-slot populated with data from the enclosure and ses modules udev needs notification when the new interface files/links are available. Otherwise, any udev rules specified for the disk cannot assume that the enclosure topology has settled. Signed-off-by: Dan Williams Signed-off-by: Song Liu Reviewed-by: Jens Axboe Reviewed-by: Hannes Reinecke --- drivers/scsi/ses.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index c52fd98..87cf970b 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -349,7 +349,8 @@ static int ses_enclosure_find_by_addr(struct enclosure_device *edev, if (scomp->addr != efd->addr) continue; - enclosure_add_device(edev, i, efd->dev); + if (enclosure_add_device(edev, i, efd->dev) == 0) + kobject_uevent(&efd->dev->kobj, KOBJ_CHANGE); return 1; } return 0; -- 1.8.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
[PATCH RESEND 3/5] SES: add enclosure logical id
From: Dan Williams
Export the NAA logical id for the enclosure. This is optionally available from
the sas_transport_class, but it is really a property of the enclosure.
Signed-off-by: Dan Williams
Signed-off-by: Song Liu
Reviewed-by: Jens Axboe
Cc: Hannes Reinecke
---
drivers/misc/enclosure.c | 13 +
drivers/scsi/ses.c| 9 +
include/linux/enclosure.h | 1 +
3 files changed, 23 insertions(+)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
15faf61..18b87de 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -395,8 +395,21 @@ static ssize_t components_show(struct device *cdev, }
static DEVICE_ATTR_RO(components);
+static ssize_t id_show(struct device *cdev,
+struct device_attribute *attr,
+char *buf)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev);
+
+ if (edev->cb->show_id)
+ return edev->cb->show_id(edev, buf);
+ return -EINVAL;
+}
+static DEVICE_ATTR_RO(id);
+
static struct attribute *enclosure_class_attrs[] = {
&dev_attr_components.attr,
+ &dev_attr_id.attr,
NULL,
};
ATTRIBUTE_GROUPS(enclosure_class);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 87cf970b..696d5d8
100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -258,6 +258,14 @@ static int ses_set_active(struct enclosure_device *edev,
return ses_set_page2_descriptor(edev, ecomp, desc); }
+static int ses_show_id(struct enclosure_device *edev, char *buf) {
+ struct ses_device *ses_dev = edev->scratch;
+ unsigned long long id = get_unaligned_be64(ses_dev->page1+8+4);
+
+ return sprintf(buf, "%#llx\n", id);
+}
+
static struct enclosure_component_callbacks ses_enclosure_callbacks = {
.get_fault = ses_get_fault,
.set_fault = ses_set_fault,
@@ -265,6 +273,7 @@ static struct enclosure_component_callbacks
ses_enclosure_callbacks = {
.get_locate = ses_get_locate,
.set_locate = ses_set_locate,
.set_active = ses_set_active,
+ .show_id= ses_show_id,
};
struct ses_host_edev {
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index
a835d33..807622b 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -79,6 +79,7 @@ struct enclosure_component_callbacks {
int (*set_locate)(struct enclosure_device *,
struct enclosure_component *,
enum enclosure_component_setting);
+ int (*show_id)(struct enclosure_device *, char *buf);
};
--
1.8.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
[PATCH RESEND 4/5] SES: add reliable slot attribute
From: Dan Williams
The name provided by firmware is in a vendor specific format, publish the slot
number to have a reliable mechanism for identifying slots across firmware
implementations. If the enclosure does not provide a slot number fallback to
the component number which is guaranteed unique, and usually mirrors the slot
number.
Cleaned up the unused ses_component.desc in the process.
Signed-off-by: Dan Williams
Signed-off-by: Song Liu
Reviewed-by: Jens Axboe
Reviewed-by: Hannes Reinecke
---
drivers/misc/enclosure.c | 20 +++-
drivers/scsi/ses.c| 17 -
include/linux/enclosure.h | 1 +
3 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
18b87de..2e3eafa 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -145,8 +145,10 @@ enclosure_register(struct device *dev, const char *name,
int components,
if (err)
goto err;
- for (i = 0; i < components; i++)
+ for (i = 0; i < components; i++) {
edev->component[i].number = -1;
+ edev->component[i].slot = -1;
+ }
mutex_lock(&container_list_lock);
list_add_tail(&edev->node, &container_list); @@ -552,6 +554,20 @@
static ssize_t get_component_type(struct device *cdev,
return snprintf(buf, 40, "%s\n", enclosure_type[ecomp->type]); }
+static ssize_t get_component_slot(struct device *cdev,
+ struct device_attribute *attr, char *buf) {
+ struct enclosure_component *ecomp = to_enclosure_component(cdev);
+ int slot;
+
+ /* if the enclosure does not override then use 'number' as a stand-in */
+ if (ecomp->slot >= 0)
+ slot = ecomp->slot;
+ else
+ slot = ecomp->number;
+
+ return snprintf(buf, 40, "%d\n", slot); }
static DEVICE_ATTR(fault, S_IRUGO | S_IWUSR, get_component_fault,
set_component_fault);
@@ -562,6 +578,7 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR,
get_component_active, static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR,
get_component_locate,
set_component_locate);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL);
+static DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
static struct attribute *enclosure_component_attrs[] = {
&dev_attr_fault.attr,
@@ -569,6 +586,7 @@ static struct attribute *enclosure_component_attrs[] = {
&dev_attr_active.attr,
&dev_attr_locate.attr,
&dev_attr_type.attr,
+ &dev_attr_slot.attr,
NULL
};
ATTRIBUTE_GROUPS(enclosure_component);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 696d5d8..3e59a5d
100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -47,7 +47,6 @@ struct ses_device {
struct ses_component {
u64 addr;
- unsigned char *desc;
};
static int ses_probe(struct device *dev) @@ -307,19 +306,26 @@ static void
ses_process_descriptor(struct enclosure_component *ecomp,
int invalid = desc[0] & 0x80;
enum scsi_protocol proto = desc[0] & 0x0f;
u64 addr = 0;
+ int slot = -1;
struct ses_component *scomp = ecomp->scratch;
unsigned char *d;
- scomp->desc = desc;
-
if (invalid)
return;
switch (proto) {
+ case SCSI_PROTOCOL_FCP:
+ if (eip) {
+ d = desc + 4;
+ slot = d[3];
+ }
+ break;
case SCSI_PROTOCOL_SAS:
- if (eip)
+ if (eip) {
+ d = desc + 4;
+ slot = d[3];
d = desc + 8;
- else
+ } else
d = desc + 4;
/* only take the phy0 addr */
addr = (u64)d[12] << 56 |
@@ -335,6 +341,7 @@ static void ses_process_descriptor(struct
enclosure_component *ecomp,
/* FIXME: Need to add more protocols than just SAS */
break;
}
+ ecomp->slot = slot;
scomp->addr = addr;
}
diff --git a/include/linux/enclosure.h b/include/linux/enclosure.h index
807622b..0f826c1 100644
--- a/include/linux/enclosure.h
+++ b/include/linux/enclosure.h
@@ -92,6 +92,7 @@ struct enclosure_component {
int fault;
int active;
int locate;
+ int slot;
enum enclosure_status status;
};
--
1.8.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
[PATCH RESEND 0/5] Feature enhancements for ses module
These patches include a few enhancements to ses module: 1: close potential race condition by at enclosure race condition 2,3,4: add enclosure id and device slot, so we can create symlink in /dev/disk/by-slot: # ls -d /dev/disk/by-slot/* /dev/disk/by-slot/enclosure-0x5000ae41fc1310ff-slot0 5: add ability to power on/off device with power_status file in sysfs. Due to the complexity of SES standard, the module is not to replace implement all features (which is the goal of sg_ses from sg3_utils). Patch 5 and existing features for device element and array device elements control of HDDs. It is helpful to handle some HDD related fields in the kernel, as the kernel can generate mapping between a device to the SES device element (or array device element): /sys/block/sdc/device/enclosure_deviceXXX/ With patch 5, we can easily power off a running HDD by echo off > /sys/block/sdc/device/enclosure_deviceXXX/power_status This is very useful for systems like Cold Storage, where HDDs are being powered \ on/off frequently Dan Williams (4): SES: close potential registration race SES: generate KOBJ_CHANGE on enclosure attach SES: add enclosure logical id SES: add reliable slot attribute Song Liu (1): SES: Add power_status to SES enclosure component drivers/misc/enclosure.c | 107 + drivers/scsi/ses.c| 148 +++--- include/linux/enclosure.h | 13 +++- 3 files changed, 232 insertions(+), 36 deletions(-) -- 1.8.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
[PATCH RESEND 5/5] SES: Add power_status to SES enclosure component
Add power_status to SES enclosure component, so we can power on/off the HDDs
behind the enclosure.
Check firmware status in ses_set_* before sending control pages to firmware.
Signed-off-by: Song Liu
Acked-by: Dan Williams
Reviewed-by: Jens Axboe
Cc: Hannes Reinecke
---
drivers/misc/enclosure.c | 38 ++
drivers/scsi/ses.c| 98 ++-
include/linux/enclosure.h | 6 +++
3 files changed, 133 insertions(+), 9 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
2e3eafa..819f2f2 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -148,6 +148,7 @@ enclosure_register(struct device *dev, const char *name,
int components,
for (i = 0; i < components; i++) {
edev->component[i].number = -1;
edev->component[i].slot = -1;
+ edev->component[i].power_status = 1;
}
mutex_lock(&container_list_lock);
@@ -546,6 +547,40 @@ static ssize_t set_component_locate(struct device *cdev,
return count;
}
+static ssize_t get_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev->parent);
+ struct enclosure_component *ecomp = to_enclosure_component(cdev);
+
+ if (edev->cb->get_power_status)
+ edev->cb->get_power_status(edev, ecomp);
+ return snprintf(buf, 40, "%s\n", ecomp->power_status ? "on" : "off");
+}
+
+static ssize_t set_component_power_status(struct device *cdev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct enclosure_device *edev = to_enclosure_device(cdev->parent);
+ struct enclosure_component *ecomp = to_enclosure_component(cdev);
+ int val;
+
+ if (strncmp(buf, "on", 2) == 0 &&
+ (buf[2] == '\n' || buf[2] == '\0'))
+ val = 1;
+ else if (strncmp(buf, "off", 3) == 0 &&
+ (buf[3] == '\n' || buf[3] == '\0'))
+ val = 0;
+ else
+ return -EINVAL;
+
+ if (edev->cb->set_power_status)
+ edev->cb->set_power_status(edev, ecomp, val);
+ return count;
+}
+
static ssize_t get_component_type(struct device *cdev,
struct device_attribute *attr, char *buf) {
@@ -577,6 +612,8 @@ static DEVICE_ATTR(active, S_IRUGO | S_IWUSR,
get_component_active,
set_component_active);
static DEVICE_ATTR(locate, S_IRUGO | S_IWUSR, get_component_locate,
set_component_locate);
+static DEVICE_ATTR(power_status, S_IRUGO | S_IWUSR, get_component_power_status,
+ set_component_power_status);
static DEVICE_ATTR(type, S_IRUGO, get_component_type, NULL); static
DEVICE_ATTR(slot, S_IRUGO, get_component_slot, NULL);
@@ -585,6 +622,7 @@ static struct attribute *enclosure_component_attrs[] = {
&dev_attr_status.attr,
&dev_attr_active.attr,
&dev_attr_locate.attr,
+ &dev_attr_power_status.attr,
&dev_attr_type.attr,
&dev_attr_slot.attr,
NULL
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 3e59a5d..8ba3c78
100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -67,6 +67,20 @@ static int ses_probe(struct device *dev) #define
SES_TIMEOUT (30 * HZ) #define SES_RETRIES 3
+static void init_device_slot_control(unsigned char *dest_desc,
+struct enclosure_component *ecomp,
+unsigned char *status)
+{
+ memcpy(dest_desc, status, 4);
+ dest_desc[0] = 0;
+ /* only clear byte 1 for ENCLOSURE_COMPONENT_DEVICE */
+ if (ecomp->type == ENCLOSURE_COMPONENT_DEVICE)
+ dest_desc[1] = 0;
+ dest_desc[2] &= 0xde;
+ dest_desc[3] &= 0x3c;
+}
+
+
static int ses_recv_diag(struct scsi_device *sdev, int page_code,
void *buf, int bufflen)
{
@@ -178,14 +192,22 @@ static int ses_set_fault(struct enclosure_device *edev,
struct enclosure_component *ecomp,
enum enclosure_component_setting val) {
- unsigned char desc[4] = {0 };
+ unsigned char desc[4];
+ unsigned char *desc_ptr;
+
+ desc_ptr = ses_get_page2_descriptor(edev, ecomp);
+
+ if (!desc_ptr)
+ return -EIO;
+
+ init_device_slot_control(desc, ecomp, desc_ptr);
switch (val) {
case ENCLOSURE_SETTING_DISABLED:
- /* zero is disabled */
+ desc[3] &= 0xdf;
break;
case ENCLOSURE_SETTING_ENABLED:
- desc[3] = 0x20;
+ desc[3] |= 0x20;
break;
default:
/* SES doesn't d
[PATCH RESEND 1/5] SES: close potential registration race
From: Dan Williams
The slot and address fields have a small window of instability when userspace
can read them before initialization. Separate enclosure_component allocation
from registration.
Signed-off-by: Dan Williams
Signed-off-by: Song Liu
Reviewed-by: Jens Axboe
Cc: Hannes Reinecke
---
drivers/misc/enclosure.c | 36 +---
drivers/scsi/ses.c| 21 ++---
include/linux/enclosure.h | 5 +++--
3 files changed, 42 insertions(+), 20 deletions(-)
diff --git a/drivers/misc/enclosure.c b/drivers/misc/enclosure.c index
2cf2bbc..15faf61 100644
--- a/drivers/misc/enclosure.c
+++ b/drivers/misc/enclosure.c
@@ -249,27 +249,25 @@ static void enclosure_component_release(struct device
*dev) static const struct attribute_group *enclosure_component_groups[];
/**
- * enclosure_component_register - add a particular component to an enclosure
+ * enclosure_component_alloc - prepare a new enclosure component
* @edev: the enclosure to add the component
* @num: the device number
* @type: the type of component being added
* @name: an optional name to appear in sysfs (leave NULL if none)
*
- * Registers the component. The name is optional for enclosures that
- * give their components a unique name. If not, leave the field NULL
- * and a name will be assigned.
+ * The name is optional for enclosures that give their components a
+ unique
+ * name. If not, leave the field NULL and a name will be assigned.
*
* Returns a pointer to the enclosure component or an error.
*/
struct enclosure_component *
-enclosure_component_register(struct enclosure_device *edev,
-unsigned int number,
-enum enclosure_component_type type,
-const char *name)
+enclosure_component_alloc(struct enclosure_device *edev,
+ unsigned int number,
+ enum enclosure_component_type type,
+ const char *name)
{
struct enclosure_component *ecomp;
struct device *cdev;
- int err;
if (number >= edev->components)
return ERR_PTR(-EINVAL);
@@ -291,14 +289,30 @@ enclosure_component_register(struct enclosure_device
*edev,
cdev->release = enclosure_component_release;
cdev->groups = enclosure_component_groups;
+ return ecomp;
+}
+EXPORT_SYMBOL_GPL(enclosure_component_alloc);
+
+/**
+ * enclosure_component_register - publishes an initialized enclosure component
+ * @ecomp: component to add
+ *
+ * Returns 0 on successful registration, releases the component
+otherwise */ int enclosure_component_register(struct
+enclosure_component *ecomp) {
+ struct device *cdev;
+ int err;
+
+ cdev = &ecomp->cdev;
err = device_register(cdev);
if (err) {
ecomp->number = -1;
put_device(cdev);
- return ERR_PTR(err);
+ return err;
}
- return ecomp;
+ return 0;
}
EXPORT_SYMBOL_GPL(enclosure_component_register);
diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 80bfece..c52fd98
100644
--- a/drivers/scsi/ses.c
+++ b/drivers/scsi/ses.c
@@ -423,16 +423,23 @@ static void ses_enclosure_data_process(struct
enclosure_device *edev,
type_ptr[0] == ENCLOSURE_COMPONENT_ARRAY_DEVICE) {
if (create)
- ecomp =
enclosure_component_register(edev,
-
components++,
-
type_ptr[0],
-
name);
+ ecomp = enclosure_component_alloc(
+ edev,
+ components++,
+ type_ptr[0],
+ name);
else
ecomp = &edev->component[components++];
- if (!IS_ERR(ecomp) && addl_desc_ptr)
- ses_process_descriptor(ecomp,
- addl_desc_ptr);
+ if (!IS_ERR(ecomp)) {
+ if (addl_desc_ptr)
+ ses_process_descriptor(
+ ecomp,
+ addl_desc_ptr);
+ if (create)
+ enclosure_component_register(
+ ecomp);
+
drivers:scsi:storvsc: Fix a bug in handling ring buffer failures that may result in I/O freeze
When ring buffer returns an error indicating retry, storvsc may not return a
proper error code to SCSI when bounce buffer is not used. This has introduced
I/O freeze on RAID running atop storvsc devices. This patch fixes it by always
returning a proper error code.
Signed-off-by: Long Li
Reviewed-by: K. Y. Srinivasan
---
drivers/scsi/storvsc_drv.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index e3ba251..4cff0dd 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1688,13 +1688,12 @@ static int storvsc_queuecommand(struct Scsi_Host *host,
struct scsi_cmnd *scmnd)
if (ret == -EAGAIN) {
/* no more space */
- if (cmd_request->bounce_sgl_count) {
+ if (cmd_request->bounce_sgl_count)
destroy_bounce_buffer(cmd_request->bounce_sgl,
cmd_request->bounce_sgl_count);
- ret = SCSI_MLQUEUE_DEVICE_BUSY;
- goto queue_error;
- }
+ ret = SCSI_MLQUEUE_DEVICE_BUSY;
+ goto queue_error;
}
return 0;
--
2.1.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: drivers:scsi:storvsc: Fix a bug in handling ring buffer failures that may result in I/O freeze
> -Original Message-
> From: Long Li [mailto:lon...@microsoft.com]
> Sent: Friday, December 5, 2014 7:38 PM
> To: linux-scsi@vger.kernel.org
> Cc: KY Srinivasan; Haiyang Zhang; jbottom...@parallels.com;
> de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; Long Li
> Subject: drivers:scsi:storvsc: Fix a bug in handling ring buffer failures
> that may
> result in I/O freeze
>
> When ring buffer returns an error indicating retry, storvsc may not return a
> proper error code to SCSI when bounce buffer is not used. This has
> introduced I/O freeze on RAID running atop storvsc devices. This patch fixes
> it by always returning a proper error code.
>
> Signed-off-by: Long Li
> Reviewed-by: K. Y. Srinivasan
Signed-off-by: K. Y. Srinivasan
cc: sta...@vger.kernel.org
> ---
> drivers/scsi/storvsc_drv.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index
> e3ba251..4cff0dd 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1688,13 +1688,12 @@ static int storvsc_queuecommand(struct
> Scsi_Host *host, struct scsi_cmnd *scmnd)
> if (ret == -EAGAIN) {
> /* no more space */
>
> - if (cmd_request->bounce_sgl_count) {
> + if (cmd_request->bounce_sgl_count)
> destroy_bounce_buffer(cmd_request->bounce_sgl,
> cmd_request->bounce_sgl_count);
>
> - ret = SCSI_MLQUEUE_DEVICE_BUSY;
> - goto queue_error;
> - }
> + ret = SCSI_MLQUEUE_DEVICE_BUSY;
> + goto queue_error;
> }
>
> return 0;
> --
> 2.1.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
