2013-09-13 12:58:35

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 0/4] Hyper-V TRIM support

tl;dr -- enable TRIM support for Hyper-V emulated disks.

The Hyper-V hypervisor can support TRIM for its devices, advertising this
via the appropriate VPD pages. However the emulated disks only claim
to be SPC-2 devices. According to the specs VPD pages (in general) did
exist at SPC-2 but the specific pages we interogate for the TRIM support
did not until SPC-3 therefore the kernel avoids reading the relevant pages
for SPC-2 devices and prevents TRIM from being offered for these devices.
Additionally at SPC-2 we prefer ReadCapacity10 over ReadCapacity16 and
unless we use RC16 we will not identify the device as TRIM capable also
preventing TRIM being offered.

As the VPD page zero does list which pages are valid for each device, it
could be argued that we could simply attempt to use these pages for all
devices which claim to be SPC-2 and above. While this seems valid the
code documents a number of devices which take badly to having even VPD
page 0 interogated even when supposedly supported. Therefore it seems
appropriate to add a scsi device flag to allow a device to request SPC-3
VPD pages be used when only at SPC-2.

Similarly for the ReadCapacity selection it seems dangerous to invert
the order for all SPC-2 devices. So it seems appropriate to add a scsi
device flag to request we try RC16 before RC10 (mirroring the existing
flag for the opposite).

The following four patches add the two scsi device flags and select those
flags for the Hyper-V emulated disks.

Patches against v3.11.

Comments?

-apw

Andy Whitcroft (4):
scsi: add scsi device flag to request VPD pages be used at SPC-2
scsi: add scsi device flag to request READ CAPACITY (16) be preferred
scsi: hyper-v storage -- mark as VPD capable at SPC-2
scsi: hyper-v storage -- mark as preferring READ CAPACITY (16) at
SPC-2

drivers/scsi/sd.c | 8 +++++++-
drivers/scsi/storvsc_drv.c | 2 ++
include/scsi/scsi_device.h | 2 ++
3 files changed, 11 insertions(+), 1 deletion(-)

--
1.8.1.2


2013-09-13 12:58:44

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 2/4] scsi: add scsi device flag to request READ CAPACITY (16) be preferred

BugLink: http://bugs.launchpad.net/bugs/1223499
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/scsi/sd.c | 2 ++
include/scsi/scsi_device.h | 1 +
2 files changed, 3 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 5a8a04d..eba4d6c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2104,6 +2104,8 @@ static int sd_try_rc16_first(struct scsi_device *sdp)
return 1;
if (scsi_device_protection(sdp))
return 1;
+ if (sdp->try_rc_16_first)
+ return 1;
return 0;
}

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9b7fdb6..df3ed43 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -165,6 +165,7 @@ struct scsi_device {
unsigned no_read_disc_info:1; /* Avoid READ_DISC_INFO cmds */
unsigned no_read_capacity_16:1; /* Avoid READ_CAPACITY_16 cmds */
unsigned try_rc_10_first:1; /* Try READ_CAPACACITY_10 first */
+ unsigned try_rc_16_first:1; /* Try READ_CAPACACITY_16 first */
unsigned is_visible:1; /* is the device visible in sysfs */
unsigned wce_default_on:1; /* Cache is ON by default */
unsigned no_dif:1; /* T10 PI (DIF) should be disabled */
--
1.8.1.2

2013-09-13 12:58:41

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 1/4] scsi: add scsi device flag to request VPD pages be used at SPC-2

Under Hyper-V the disk devices support the trim extensions advertising them
via the appropriate VPD pages, it however reports itself as SPC-2 only.
The relevant pages were added in SPC-3 and later, so we do not even
attempt to see if they are present; the VPD page 0 lists which other
pages are present.

Add a scsi quirk bit to allow those devices to hint we can and should
try these VPD pages.

BugLink: http://bugs.launchpad.net/bugs/1223499
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/scsi/sd.c | 6 +++++-
include/scsi/scsi_device.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b58e8f8..5a8a04d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2667,8 +2667,12 @@ static int sd_try_extended_inquiry(struct scsi_device *sdp)
* Although VPD inquiries can go to SCSI-2 type devices,
* some USB ones crash on receiving them, and the pages
* we currently ask for are for SPC-3 and beyond
+ * Allow devices to request use of VPD inquiry at SPC-2
+ * as some devices implement these extensions (inc Hyper-V)
*/
- if (sdp->scsi_level > SCSI_SPC_2 && !sdp->skip_vpd_pages)
+ if ((sdp->scsi_level > SCSI_SPC_2 ||
+ (sdp->use_vpd_spc2 && sdp->scsi_level >= SCSI_SPC_2)) &&
+ !sdp->skip_vpd_pages)
return 1;
return 0;
}
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index d65fbec..9b7fdb6 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -149,6 +149,7 @@ struct scsi_device {
unsigned skip_ms_page_8:1; /* do not use MODE SENSE page 0x08 */
unsigned skip_ms_page_3f:1; /* do not use MODE SENSE page 0x3f */
unsigned skip_vpd_pages:1; /* do not read VPD pages */
+ unsigned use_vpd_spc2:1; /* do use VPD pages at SPC-2 */
unsigned use_192_bytes_for_3f:1; /* ask for 192 bytes from page 0x3f */
unsigned no_start_on_add:1; /* do not issue start on add */
unsigned allow_restart:1; /* issue START_UNIT in error handler */
--
1.8.1.2

2013-09-13 12:58:57

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 4/4] scsi: hyper-v storage -- mark as preferring READ CAPACITY (16) at SPC-2

BugLink: http://bugs.launchpad.net/bugs/1223499
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/scsi/storvsc_drv.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 14ba8fd..25e7dd5 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1438,6 +1438,7 @@ static int storvsc_device_configure(struct scsi_device *sdevice)

sdevice->no_write_same = 1;
sdevice->use_vpd_spc2 = 1;
+ sdevice->try_rc_16_first = 1;

return 0;
}
--
1.8.1.2

2013-09-13 12:59:24

by Andy Whitcroft

[permalink] [raw]
Subject: [PATCH 3/4] scsi: hyper-v storage -- mark as VPD capable at SPC-2

BugLink: http://bugs.launchpad.net/bugs/1223499
Signed-off-by: Andy Whitcroft <[email protected]>
---
drivers/scsi/storvsc_drv.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 1a28f56..14ba8fd 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1437,6 +1437,7 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
blk_queue_rq_timeout(sdevice->request_queue, (storvsc_timeout * HZ));

sdevice->no_write_same = 1;
+ sdevice->use_vpd_spc2 = 1;

return 0;
}
--
1.8.1.2

2013-09-13 14:58:02

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH 0/4] Hyper-V TRIM support

On Fri, 2013-09-13 at 13:58 +0100, Andy Whitcroft wrote:
> tl;dr -- enable TRIM support for Hyper-V emulated disks.
>
> The Hyper-V hypervisor can support TRIM for its devices, advertising this
> via the appropriate VPD pages. However the emulated disks only claim
> to be SPC-2 devices. According to the specs VPD pages (in general) did
> exist at SPC-2 but the specific pages we interogate for the TRIM support
> did not until SPC-3 therefore the kernel avoids reading the relevant pages
> for SPC-2 devices and prevents TRIM from being offered for these devices.
> Additionally at SPC-2 we prefer ReadCapacity10 over ReadCapacity16 and
> unless we use RC16 we will not identify the device as TRIM capable also
> preventing TRIM being offered.
>
> As the VPD page zero does list which pages are valid for each device, it
> could be argued that we could simply attempt to use these pages for all
> devices which claim to be SPC-2 and above. While this seems valid the
> code documents a number of devices which take badly to having even VPD
> page 0 interogated even when supposedly supported. Therefore it seems
> appropriate to add a scsi device flag to allow a device to request SPC-3
> VPD pages be used when only at SPC-2.
>
> Similarly for the ReadCapacity selection it seems dangerous to invert
> the order for all SPC-2 devices. So it seems appropriate to add a scsi
> device flag to request we try RC16 before RC10 (mirroring the existing
> flag for the opposite).
>
> The following four patches add the two scsi device flags and select those
> flags for the Hyper-V emulated disks.
>
> Patches against v3.11.
>
> Comments?

This is an awful lot of contortions (which don't seem to have any other
users on the horizon) to support a device that's not standards
compliant. What about this, it's simple, it does the right thing and
it's contained in the driver of the problem device.

James

---

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 83ec1aa..bd86a4b 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1438,6 +1438,12 @@ static int storvsc_device_configure(struct scsi_device *sdevice)

sdevice->no_write_same = 1;

+ /*
+ * hyper-v is stupid and lies about its capabilities
+ * If we pretend to be SPC-3, we send RC16 which activates trim
+ */
+ sdevice->scsi_level = SCSI_SPC_3;
+
return 0;
}


2013-09-13 15:41:04

by Douglas Gilbert

[permalink] [raw]
Subject: Re: [PATCH 0/4] Hyper-V TRIM support

On 13-09-13 08:58 AM, Andy Whitcroft wrote:
> tl;dr -- enable TRIM support for Hyper-V emulated disks.
>
> The Hyper-V hypervisor can support TRIM for its devices, advertising this
> via the appropriate VPD pages. However the emulated disks only claim
> to be SPC-2 devices. According to the specs VPD pages (in general) did
> exist at SPC-2 but the specific pages we interogate for the TRIM support

VPD pages are found in SPC (ANSI INCITS 301-1997) and many of its
drafts. By SPC-2 (ANSI INCITS 351-2001) the "supported VPD pages"
VPD page (0x0) ** and the "device identification" VPD page (0x83)
were mandatory (in SPC those pages were optional). So that is
approaching 20 years for manufacturers to get used to VPD pages.

TRIM is a T13 term (ATA/SATA) that was introduced after 2001
(i.e. after SBC-2). The corresponding SCSI term is now
"Logical Block Provisioning" (LBP). This covers the SCSI
UNMAP command (closest thing to TRIM) and the SCSI WRITE SAME
command which contains LBP options.

LBP capability was originally reported in the SCSI READ
CAPACITY(16) command (but not the more common READ
CAPACITY(10) command); namely the LBPME and LBPRZ bits. Those
bits have been renamed *** during the lifetime of SBC-3 drafts.
Those bits and a lot of additional LBP information are now
found in two VPD pages: "Block Limits" (0xb0) and "Logical
Block Provisioning" (0xb2).


After a 36 byte standard INQUIRY response, unless the
compliance is stone age (e.g. SCSI-2 or earlier) then
a 36 byte INQUIRY with the EVPD bit set and page=0
should be pretty safe. Check the response carefully
as USB devices will often ignore the EVPD bit and respond
with a standard INQUIRY response. Forget any such devices.
Now look for either of those LBP supporting VPD pages.
There should not be too many devices that support neither
and do LBP.

Doug Gilbert


** some vendors do not include their own vendor specific VPD
pages in the "Supported VPD pages" VPD page. Grrr

*** those bits were previously named TPE and TPRZ

> did not until SPC-3 therefore the kernel avoids reading the relevant pages
> for SPC-2 devices and prevents TRIM from being offered for these devices.
> Additionally at SPC-2 we prefer ReadCapacity10 over ReadCapacity16 and
> unless we use RC16 we will not identify the device as TRIM capable also
> preventing TRIM being offered.
>
> As the VPD page zero does list which pages are valid for each device, it
> could be argued that we could simply attempt to use these pages for all
> devices which claim to be SPC-2 and above. While this seems valid the
> code documents a number of devices which take badly to having even VPD
> page 0 interogated even when supposedly supported. Therefore it seems
> appropriate to add a scsi device flag to allow a device to request SPC-3
> VPD pages be used when only at SPC-2.
>
> Similarly for the ReadCapacity selection it seems dangerous to invert
> the order for all SPC-2 devices. So it seems appropriate to add a scsi
> device flag to request we try RC16 before RC10 (mirroring the existing
> flag for the opposite).
>
> The following four patches add the two scsi device flags and select those
> flags for the Hyper-V emulated disks.




2013-09-13 15:40:59

by Andy Whitcroft

[permalink] [raw]
Subject: Re: [PATCH 0/4] Hyper-V TRIM support

On Fri, Sep 13, 2013 at 07:57:58AM -0700, James Bottomley wrote:

> This is an awful lot of contortions (which don't seem to have any other
> users on the horizon) to support a device that's not standards
> compliant. What about this, it's simple, it does the right thing and
> it's contained in the driver of the problem device.
>
> James
>
> ---
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 83ec1aa..bd86a4b 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1438,6 +1438,12 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
>
> sdevice->no_write_same = 1;
>
> + /*
> + * hyper-v is stupid and lies about its capabilities
> + * If we pretend to be SPC-3, we send RC16 which activates trim
> + */
> + sdevice->scsi_level = SCSI_SPC_3;
> +
> return 0;
> }

I will get that tested and see if there are any other negative side effects.

-apw