Add blist flags to permit the reading of the VPD pages even when
the target may claim SPC-2 compliance. MSFT targets currently
claim SPC-2 compliance while they implement post SPC-2 features.
With this patch we can correctly handle WRITE_SAME_16 issues.
Signed-off-by: K. Y. Srinivasan <[email protected]>
---
drivers/scsi/storvsc_drv.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 29d0329..15ba695 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -327,6 +327,8 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
*/
static int storvsc_timeout = 180;
+static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
+
#define STORVSC_MAX_IO_REQUESTS 200
static void storvsc_on_channel_callback(void *context);
@@ -1449,6 +1451,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
sdevice->no_write_same = 1;
+ /*
+ * Add blist flags to permit the reading of the VPD pages even when
+ * the target may claim SPC-2 compliance. MSFT targets currently
+ * claim SPC-2 compliance while they implement post SPC-2 features.
+ * With this patch we can correctly handle WRITE_SAME_16 issues.
+ */
+ sdevice->sdev_bflags |= msft_blist_flags;
+
return 0;
}
--
1.7.4.1
On Mon, Jul 21, 2014 at 04:06:01PM -0700, K. Y. Srinivasan wrote:
> Add blist flags to permit the reading of the VPD pages even when
> the target may claim SPC-2 compliance. MSFT targets currently
> claim SPC-2 compliance while they implement post SPC-2 features.
> With this patch we can correctly handle WRITE_SAME_16 issues.
OK I've just seen this as I was about to post a similar patch to get
discard going on Hyper-V. Will your patches handle Hyper-V pass through devices
that support discard? The SSD I have here reports the following in the Linux
VM:
# sg_vpd -p lbpv /dev/sdc
Logical block provisioning VPD page (SBC):
Unmap command supported (LBPU): 1
Write same (16) with unmap bit supported (LBWS): 0
Write same (10) with unmap bit supported (LBWS10): 0
Logical block provisioning read zeros (LBPRZ): 0
Anchored LBAs supported (ANC_SUP): 1
Threshold exponent: 0
Descriptor present (DP): 0
Provisioning type: 0
# sg_readcap -l /dev/sdc
Read Capacity results:
Protection: prot_en=0, p_type=0, p_i_exponent=0
Logical block provisioning: lbpme=0, lbprz=0
Last logical block address=234441647 (0xdf94baf), Number of logical blocks=234441648
Logical block length=512 bytes
Logical blocks per physical block exponent=0
Lowest aligned logical block address=0
Hence:
Device size: 120034123776 bytes, 114473.5 MiB, 120.03 GB
--
Sitsofe | http://sucs.org/~sits/
On Wed, Jul 23, 2014 at 11:04:48AM +0100, Sitsofe Wheeler wrote:
> OK I've just seen this as I was about to post a similar patch to get
> discard going on Hyper-V. Will your patches handle Hyper-V pass through devices
> that support discard? The SSD I have here reports the following in the Linux
> VM:
I didn't know hyperv also supports pass through, but it should work fine.
The only thing I'm worried about is real SCSI-2 devices or crappy USB
devices that don't support EVPD pages and might cause problems. But
let's ignore that problem for now..
On Wed, Jul 23, 2014 at 04:51:28AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 23, 2014 at 11:04:48AM +0100, Sitsofe Wheeler wrote:
> > OK I've just seen this as I was about to post a similar patch to get
> > discard going on Hyper-V. Will your patches handle Hyper-V pass through devices
> > that support discard? The SSD I have here reports the following in the Linux
> > VM:
>
> I didn't know hyperv also supports pass through, but it should work fine.
> The only thing I'm worried about is real SCSI-2 devices or crappy USB
> devices that don't support EVPD pages and might cause problems. But
> let's ignore that problem for now..
That's good to know (I was worried the device would not be detected as
supporting discard because it doesn't report lbpme and doesn't declare a
conformance version (see below)). Is there a tree with all these updates
in or do they need to be pieced together from this email thread?
Additionally is it worth quirking sd_try_rc16_first on when
BLIST_TRY_VPD_PAGES is present?
# sg_inq /dev/sdc
standard INQUIRY:
PQual=0 Device_type=0 RMB=0 version=0x00 [no conformance claimed]
[AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2
SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
[RelAdr=0] WBus16=0 Sync=0 Linked=0 [TranDis=0] CmdQue=1
[SPI: Clocking=0x0 QAS=0 IUS=0]
length=60 (0x3c) Peripheral device type: disk
Vendor identification: ADATA
Product identification: SSD S510 120GB
Product revision level: 5.2.
Unit serial number: 03205115500300002076
--
Sitsofe | http://sucs.org/~sits/
On Wed, Jul 23, 2014 at 01:54:43PM +0100, Sitsofe Wheeler wrote:
> That's good to know (I was worried the device would not be detected as
> supporting discard because it doesn't report lbpme and doesn't declare a
> conformance version (see below)).
Ok, that makes things worse - you might be able to force it through
sysfs, though.
> Is there a tree with all these updates
> in or do they need to be pieced together from this email thread?
I'm picking this up for my scsi-queue tree. Still need a review
for the patch we're replying to before that one can go in, though.
> Additionally is it worth quirking sd_try_rc16_first on when
> BLIST_TRY_VPD_PAGES is present?
Sounds reasonable to me.
On Mon, Jul 21, 2014 at 04:06:01PM -0700, K. Y. Srinivasan wrote:
> Add blist flags to permit the reading of the VPD pages even when
> the target may claim SPC-2 compliance. MSFT targets currently
> claim SPC-2 compliance while they implement post SPC-2 features.
> With this patch we can correctly handle WRITE_SAME_16 issues.
>
> static void storvsc_on_channel_callback(void *context);
> @@ -1449,6 +1451,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
>
> sdevice->no_write_same = 1;
>
> + /*
> + * Add blist flags to permit the reading of the VPD pages even when
> + * the target may claim SPC-2 compliance. MSFT targets currently
> + * claim SPC-2 compliance while they implement post SPC-2 features.
> + * With this patch we can correctly handle WRITE_SAME_16 issues.
> + */
> + sdevice->sdev_bflags |= msft_blist_flags;
> +
I'm not sure this alone will work - won't sdev_bflags/bflags have
already been built at this point? If the setting of this has to stay in
this function why don't you (also) set sdevice->try_vpd_pages in a
similar way to no_write_same?
--
Sitsofe | http://sucs.org/~sits/
On Wed, Jul 23, 2014 at 03:10:28PM +0100, Sitsofe Wheeler wrote:
> I'm not sure this alone will work - won't sdev_bflags/bflags have
> already been built at this point?
They've been built up, but we can still or new values into it. It looks
fine to me from review, but if you can test it on an actualy hypverv
setup that would be valueable feedback.
> If the setting of this has to stay in
> this function why don't you (also) set sdevice->try_vpd_pages in a
> similar way to no_write_same?
We could probably do that. I don't actually like this whole split
between the blacklist flags, and the bitfields. I'll see if we can
just get rid of those bitfields an always just use the blacklist
flags directly, now that we have precedence of setting them in drivers
and not just the table keyed by the inquiry data. But that's a longer
term project, this late in the 3.17 cycle I'd just like to merge
something that gets discards on hyperv to work.
On Wed, Jul 23, 2014 at 07:10:14AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 23, 2014 at 01:54:43PM +0100, Sitsofe Wheeler wrote:
> > That's good to know (I was worried the device would not be detected as
> > supporting discard because it doesn't report lbpme and doesn't declare a
> > conformance version (see below)).
>
> Ok, that makes things worse - you might be able to force it through
> sysfs, though.
I've got a hack that seems to be working for me - see below.
> > Is there a tree with all these updates
> > in or do they need to be pieced together from this email thread?
>
> I'm picking this up for my scsi-queue tree. Still need a review
> for the patch we're replying to before that one can go in, though.
OK.
> > Additionally is it worth quirking sd_try_rc16_first on when
> > BLIST_TRY_VPD_PAGES is present?
>
> Sounds reasonable to me.
OK did you want me to post a set of changes to your patch? I've
basically changed the patches from bypassing the SCSI level check (to
allow scanning of VPD pages) to also try READ_CAPACITY(16) first and
bypass lbpme checks. Now it's more of a case of BLIST_TRY_LBP...
--
Sitsofe | http://sucs.org/~sits/
> -----Original Message-----
> From: Sitsofe Wheeler [mailto:[email protected]]
> Sent: Wednesday, July 23, 2014 3:05 AM
> To: KY Srinivasan
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
>
> On Mon, Jul 21, 2014 at 04:06:01PM -0700, K. Y. Srinivasan wrote:
> > Add blist flags to permit the reading of the VPD pages even when the
> > target may claim SPC-2 compliance. MSFT targets currently claim SPC-2
> > compliance while they implement post SPC-2 features.
> > With this patch we can correctly handle WRITE_SAME_16 issues.
>
> OK I've just seen this as I was about to post a similar patch to get discard
> going on Hyper-V. Will your patches handle Hyper-V pass through devices
> that support discard? The SSD I have here reports the following in the Linux
It should. Hyper-V does support pass through devices.
K. Y
> VM:
>
> # sg_vpd -p lbpv /dev/sdc
> Logical block provisioning VPD page (SBC):
> Unmap command supported (LBPU): 1
> Write same (16) with unmap bit supported (LBWS): 0
> Write same (10) with unmap bit supported (LBWS10): 0
> Logical block provisioning read zeros (LBPRZ): 0
> Anchored LBAs supported (ANC_SUP): 1
> Threshold exponent: 0
> Descriptor present (DP): 0
> Provisioning type: 0
>
> # sg_readcap -l /dev/sdc
> Read Capacity results:
> Protection: prot_en=0, p_type=0, p_i_exponent=0
> Logical block provisioning: lbpme=0, lbprz=0
> Last logical block address=234441647 (0xdf94baf), Number of logical
> blocks=234441648
> Logical block length=512 bytes
> Logical blocks per physical block exponent=0
> Lowest aligned logical block address=0
> Hence:
> Device size: 120034123776 bytes, 114473.5 MiB, 120.03 GB
>
> --
> Sitsofe | http://sucs.org/~sits/
> -----Original Message-----
> From: Christoph Hellwig [mailto:[email protected]]
> Sent: Wednesday, July 23, 2014 7:10 AM
> To: Sitsofe Wheeler
> Cc: KY Srinivasan; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
>
> On Wed, Jul 23, 2014 at 01:54:43PM +0100, Sitsofe Wheeler wrote:
> > That's good to know (I was worried the device would not be detected as
> > supporting discard because it doesn't report lbpme and doesn't declare
> > a conformance version (see below)).
>
> Ok, that makes things worse - you might be able to force it through sysfs,
> though.
>
> > Is there a tree with all these updates in or do they need to be pieced
> > together from this email thread?
>
> I'm picking this up for my scsi-queue tree. Still need a review for the patch
> we're replying to before that one can go in, though.
Hannes, could you please review this patch. Christoph, other than the review of this patch,
Is there something you want done before these can be committed.
Regards,
K. Y
>
> > Additionally is it worth quirking sd_try_rc16_first on when
> > BLIST_TRY_VPD_PAGES is present?
>
> Sounds reasonable to me.
On Wed, Jul 23, 2014 at 07:15:58AM -0700, Christoph Hellwig wrote:
> On Wed, Jul 23, 2014 at 03:10:28PM +0100, Sitsofe Wheeler wrote:
> > I'm not sure this alone will work - won't sdev_bflags/bflags have
> > already been built at this point?
>
> They've been built up, but we can still or new values into it. It looks
> fine to me from review, but if you can test it on an actualy hypverv
> setup that would be valueable feedback.
The previous patches didn't work for me with a Windows 2012 R2 host with a
3.16.0-rc6.x86_64-00076-g2f7d2ec-dirty guest. After applying
https://patchwork.kernel.org/patch/4541201 (which needed a small fixup) and
https://patchwork.kernel.org/patch/4598601 and the attached debugging output
patch here's the result I got:
hv_vmbus: registering driver hv_storvsc
scsi0 : storvsc_host_t
scsi 0:0:0:0: scsi_get_device_flags_keyed: key: 3
scsi 0:0:0:0: scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL
scsi 0:0:0:0: scsi_get_device_flags_keyed: No sdev_bflags
scsi 0:0:0:0: sdev->scsi_level: 5
scsi 0:0:0:0: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 4
scsi 0:0:0:0: scsi_add_lun: Have BLIST_TRY_VPD_PAGES? No
scsi 0:0:0:0: storvsc_device_configure: Added BLIST_TRY_VPD_PAGES
scsi1 : storvsc_host_t
sd 0:0:0:0: Attached scsi generic sg0 type 0
scsi 1:0:0:0: scsi_get_device_flags_keyed: key: 3
scsi 1:0:0:0: scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL
scsi 1:0:0:0: scsi_get_device_flags_keyed: No sdev_bflags
scsi 1:0:0:0: sdev->scsi_level: 5
scsi 1:0:0:0: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 4
scsi 1:0:0:0: scsi_add_lun: Have BLIST_TRY_VPD_PAGES? No
scsi 1:0:0:0: storvsc_device_configure: Added BLIST_TRY_VPD_PAGES
scsi 1:0:0:1: scsi_get_device_flags_keyed: key: 5
scsi 1:0:0:1: scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL
scsi 1:0:0:1: scsi_get_device_flags_keyed: No sdev_bflags
scsi 1:0:0:1: scsi_get_device_flags_keyed: key: 5
scsi 1:0:0:1: scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL
scsi 1:0:0:1: scsi_get_device_flags_keyed: No sdev_bflags
scsi 1:0:0:1: sdev->scsi_level: 0
scsi 1:0:0:1: Direct-Access ADATA SSD S510 120GB 5.2. PQ: 0 ANSI: 0
scsi 1:0:0:1: scsi_add_lun: Have BLIST_TRY_VPD_PAGES? No
scsi 1:0:0:1: storvsc_device_configure: Added BLIST_TRY_VPD_PAGES
scsi 1:0:0:3: scsi_get_device_flags_keyed: key: 0
scsi 1:0:0:3: scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL
scsi 1:0:0:3: scsi_get_device_flags_keyed: No sdev_bflags
scsi 1:0:0:3: sdev->scsi_level: 5
scsi 1:0:0:3: Direct-Access Msft Virtual Disk 1.0 PQ: 0 ANSI: 4
scsi 1:0:0:3: scsi_add_lun: Have BLIST_TRY_VPD_PAGES? No
scsi 1:0:0:3: storvsc_device_configure: Added BLIST_TRY_VPD_PAGES
sd 1:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:0: [sdb] 2097152 512-byte logical blocks: (1.07 GB/1.00 GiB)
sd 1:0:0:0: [sdb] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:0: [sdb] sd_revalidate_disk: Skipped extended inquiries
sd 1:0:0:0: [sdb] Write Protect is off
sd 1:0:0:0: [sdb] Mode Sense: 0f 00 00 00
sd 1:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 0:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
ata_piix 0000:00:07.1: version 2.13
sd 1:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:0: [sdb] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:0: [sdb] sd_revalidate_disk: Skipped extended inquiries
sdb: unknown partition table
ata_piix 0000:00:07.1: Hyper-V Virtual Machine detected, ATA device ignore set
sd 1:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:0: [sdb] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:0: [sdb] sd_revalidate_disk: Skipped extended inquiries
sd 1:0:0:0: [sdb] Attached SCSI disk
sd 1:0:0:0: Attached scsi generic sg1 type 0
sd 0:0:0:0: [sda] 8388608 512-byte logical blocks: (4.29 GB/4.00 GiB)
scsi2 : ata_piix
scsi3 : ata_piix
ata1: PATA max UDMA/33 cmd 0x1f0 ctl 0x3f6 bmdma 0xffa0 irq 14
ata2: PATA max UDMA/33 cmd 0x170 ctl 0x376 bmdma 0xffa8 irq 15
sd 1:0:0:1: Attached scsi generic sg2 type 0
libphy: Fixed MDIO Bus: probed
hv_vmbus: registering driver hv_netvsc
sd 1:0:0:3: Attached scsi generic sg3 type 0
hv_netvsc: hv_netvsc channel opened successfully
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Skipped extended inquiries
sd 1:0:0:1: sd_try_rc16_first: sdp->scsi_level: 0
sd 1:0:0:3: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:1: [sdc] 234441648 512-byte logical blocks: (120 GB/111 GiB)
sd 1:0:0:1: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:1: [sdc] sd_revalidate_disk: Skipped extended inquiries
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 0f 00 00 00
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 0:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Skipped extended inquiries
sda: sda1
sd 0:0:0:0: sd_try_rc16_first: sdp->scsi_level: 5
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Skipped extended inquiries
sd 0:0:0:0: [sda] Attached SCSI disk
sd 1:0:0:1: [sdc] Write Protect is off
sd 1:0:0:3: [sdd] 199229440 512-byte logical blocks: (102 GB/95.0 GiB)
sd 1:0:0:3: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:3: [sdd] sd_revalidate_disk: Skipped extended inquiries
sd 1:0:0:3: [sdd] Write Protect is off
sd 1:0:0:3: [sdd] Mode Sense: 0f 00 00 00
sd 1:0:0:3: [sdd] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:0:1: [sdc] Mode Sense: 0f 00 00 00
sd 1:0:0:3: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:1: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:0:3: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:3: [sdd] sd_revalidate_disk: Skipped extended inquiries
sdd: unknown partition table
sd 1:0:0:3: sd_try_rc16_first: sdp->scsi_level: 5
sd 1:0:0:3: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:3: [sdd] sd_revalidate_disk: Skipped extended inquiries
sd 1:0:0:3: [sdd] Attached SCSI disk
sd 1:0:0:1: sd_try_rc16_first: sdp->scsi_level: 0
sd 1:0:0:1: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:1: [sdc] sd_revalidate_disk: Skipped extended inquiries
sdc: sdc1
sd 1:0:0:1: sd_try_rc16_first: sdp->scsi_level: 0
sd 1:0:0:1: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:1: [sdc] sd_revalidate_disk: Skipped extended inquiries
ata1.00: host indicates ignore ATA devices, ignored
sd 1:0:0:1: [sdc] Attached SCSI disk
tsc: Refined TSC clocksource calibration: 3200.069 MHz
sda is a 4 GByte VHDX attached to Hyper-V's IDE interface.
sdb is a 1 GByte VHDX attached to Hyper-V's SCSI interface.
sdc is a 111.8 GByte SSD being passed through Hyper-V's SCSI interface.
sdd is a 95 GByte VHDX being passed through Hyper-V's SCSI interface.
--
Sitsofe | http://sucs.org/~sits/
On 07/22/2014 01:06 AM, K. Y. Srinivasan wrote:
> Add blist flags to permit the reading of the VPD pages even when
> the target may claim SPC-2 compliance. MSFT targets currently
> claim SPC-2 compliance while they implement post SPC-2 features.
> With this patch we can correctly handle WRITE_SAME_16 issues.
>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> ---
> drivers/scsi/storvsc_drv.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 29d0329..15ba695 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -327,6 +327,8 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
> */
> static int storvsc_timeout = 180;
>
> +static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
> +
> #define STORVSC_MAX_IO_REQUESTS 200
>
> static void storvsc_on_channel_callback(void *context);
> @@ -1449,6 +1451,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
>
> sdevice->no_write_same = 1;
>
> + /*
> + * Add blist flags to permit the reading of the VPD pages even when
> + * the target may claim SPC-2 compliance. MSFT targets currently
> + * claim SPC-2 compliance while they implement post SPC-2 features.
> + * With this patch we can correctly handle WRITE_SAME_16 issues.
> + */
> + sdevice->sdev_bflags |= msft_blist_flags;
> +
> return 0;
> }
>
>
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +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)
On Wed, Jul 23, 2014 at 09:13:41PM +0100, Sitsofe Wheeler wrote:
> On Wed, Jul 23, 2014 at 07:15:58AM -0700, Christoph Hellwig wrote:
> > On Wed, Jul 23, 2014 at 03:10:28PM +0100, Sitsofe Wheeler wrote:
> > > I'm not sure this alone will work - won't sdev_bflags/bflags have
> > > already been built at this point?
> >
> > They've been built up, but we can still or new values into it. It looks
> > fine to me from review, but if you can test it on an actualy hypverv
> > setup that would be valueable feedback.
>
> The previous patches didn't work for me with a Windows 2012 R2 host with a
> 3.16.0-rc6.x86_64-00076-g2f7d2ec-dirty guest. After applying
>
> > term project, this late in the 3.17 cycle I'd just like to merge
> > something that gets discards on hyperv to work.
OK how about the following patches:
Sitsofe Wheeler (3):
[SCSI] Add quirk for forcing logical block provisioning tests
[SCSI] storvsc: Add Hyper-V logical block provisioning quirk
[SCSI] Make LBP quirk skip lbpme checks
drivers/scsi/scsi_scan.c | 4 +++-
drivers/scsi/sd.c | 12 ++++++++++--
drivers/scsi/storvsc_drv.c | 6 ++++++
include/scsi/scsi_device.h | 1 +
include/scsi/scsi_devinfo.h | 2 ++
5 files changed, 22 insertions(+), 3 deletions(-)
--
1.9.3
--
Sitsofe | http://sucs.org/~sits/
Despite supporting modern SCSI features (such an UNMAP) some storage
devices continue to claim conformance to an older version of the SPC
spec for compatibility with legacy operating systems.
Linux by default will not attempt to read VPD pages on devices that
claim SPC-2 or older.
Introduce a blacklist flag that allows the forcing of the paths leading
to logical block provisioning tests.
See https://lkml.org/lkml/2014/7/13/59 for the previous version.
Reported-by: K. Y. Srinivasan <[email protected]>
Original-patch-by: Martin K. Petersen <[email protected]>
Signed-off-by: Sitsofe Wheeler <[email protected]>
---
drivers/scsi/scsi_scan.c | 4 +++-
drivers/scsi/sd.c | 8 ++++++++
drivers/scsi/storvsc_drv.c | 10 ++++++++++
include/scsi/scsi_device.h | 1 +
include/scsi/scsi_devinfo.h | 2 ++
5 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e02b3aa..02ca1c2 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -950,7 +950,9 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT;
- if (*bflags & BLIST_SKIP_VPD_PAGES)
+ if (*bflags & BLIST_TRY_LBP)
+ sdev->try_lbp = 1;
+ else if (*bflags & BLIST_SKIP_VPD_PAGES)
sdev->skip_vpd_pages = 1;
transport_configure_device(&sdev->sdev_gendev);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6825eda..8249e51 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2109,6 +2109,8 @@ static int sd_try_rc16_first(struct scsi_device *sdp)
return 1;
if (scsi_device_protection(sdp))
return 1;
+ if (sdp->try_lbp)
+ return 1;
return 0;
}
@@ -2682,6 +2684,12 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
static int sd_try_extended_inquiry(struct scsi_device *sdp)
{
/*
+ * Attempt VPD inquiry if the device blacklist explicitly calls
+ * for it.
+ */
+ if (sdp->try_lbp)
+ return 1;
+ /*
* Although VPD inquiries can go to SCSI-2 type devices,
* some USB ones crash on receiving them, and the pages
* we currently ask for are for SPC-3 and beyond
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 9969fa1..5ad2810 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -326,6 +326,8 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
*/
static int storvsc_timeout = 180;
+static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
+
#define STORVSC_MAX_IO_REQUESTS 200
static void storvsc_on_channel_callback(void *context);
@@ -1441,6 +1443,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
sdevice->no_write_same = 1;
+ /*
+ * Add blist flags to permit the reading of the VPD pages even when
+ * the target may claim SPC-2 compliance. MSFT targets currently
+ * claim SPC-2 compliance while they implement post SPC-2 features.
+ * With this patch we can correctly handle WRITE_SAME_16 issues.
+ */
+ sdevice->sdev_bflags |= msft_blist_flags;
+
return 0;
}
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 27ab310..0a5c6fa 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -155,6 +155,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 try_lbp:1; /* Try LBP tests */
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 */
diff --git a/include/scsi/scsi_devinfo.h b/include/scsi/scsi_devinfo.h
index 447d2d7..9d15d78 100644
--- a/include/scsi/scsi_devinfo.h
+++ b/include/scsi/scsi_devinfo.h
@@ -32,4 +32,6 @@
#define BLIST_ATTACH_PQ3 0x1000000 /* Scan: Attach to PQ3 devices */
#define BLIST_NO_DIF 0x2000000 /* Disable T10 PI (DIF) */
#define BLIST_SKIP_VPD_PAGES 0x4000000 /* Ignore SBC-3 VPD pages */
+#define BLIST_TRY_LBP 0x10000000 /* Check Logical Block Provisioning
+ even on < SPC-3 devices */
#endif
--
1.9.3
Microsoft Hyper-V targets currently only claim SPC-2 compliance / no
compliance indicated even though they implement post SPC-2 features
which means those features are not tested for. Add a blacklist flag to
Hyper-V devices that forces said testing.
See https://lkml.org/lkml/2014/7/21/627 for the previous version of this
patch and https://lkml.org/lkml/2014/7/23/615 for example devices.
Original-patch-by: K. Y. Srinivasan <[email protected]>
Signed-off-by: Sitsofe Wheeler <[email protected]>
---
drivers/scsi/storvsc_drv.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 5ad2810..88b7173 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -326,8 +326,6 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
*/
static int storvsc_timeout = 180;
-static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
-
#define STORVSC_MAX_IO_REQUESTS 200
static void storvsc_on_channel_callback(void *context);
@@ -1444,12 +1442,10 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
sdevice->no_write_same = 1;
/*
- * Add blist flags to permit the reading of the VPD pages even when
- * the target may claim SPC-2 compliance. MSFT targets currently
- * claim SPC-2 compliance while they implement post SPC-2 features.
- * With this patch we can correctly handle WRITE_SAME_16 issues.
+ * Forcefully enable logical block provisioning testing.
*/
- sdevice->sdev_bflags |= msft_blist_flags;
+ sdevice->sdev_bflags |= BLIST_TRY_LBP;
+ sdevice->try_lbp = 1;
return 0;
}
--
1.9.3
Some block devices (such as Hyper-V passthrough SSDs) support logical
block provisioning (e.g. via UNMAP) but don't set lbpme thus disabling
discard. If the try_lbp quirk is in use skip lbpme checks that lead up
to the logical block provisioning tests.
Signed-off-by: Sitsofe Wheeler <[email protected]>
---
drivers/scsi/sd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8249e51..8bf34bc 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2559,7 +2559,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]);
- if (!sdkp->lbpme)
+ if (!sdkp->lbpme && !sdkp->device->try_lbp)
goto out;
lba_count = get_unaligned_be32(&buffer[20]);
@@ -2633,7 +2633,7 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp)
unsigned char *buffer;
const int vpd_len = 8;
- if (sdkp->lbpme == 0)
+ if (sdkp->lbpme == 0 && sdkp->device->test_lbp == 0)
return;
buffer = kmalloc(vpd_len, GFP_KERNEL);
--
1.9.3
v1 -> v2: Fix incorrectly named variable.
Some block devices (such as Hyper-V passthrough SSDs) support logical
block provisioning (e.g. via UNMAP) but don't set lbpme thus disabling
discard. If the try_lbp quirk is in use skip lbpme checks that lead up
to the logical block provisioning tests.
Signed-off-by: Sitsofe Wheeler <[email protected]>
---
drivers/scsi/sd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8249e51..8bf34bc 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2559,7 +2559,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]);
- if (!sdkp->lbpme)
+ if (!sdkp->lbpme && !sdkp->device->try_lbp)
goto out;
lba_count = get_unaligned_be32(&buffer[20]);
@@ -2633,7 +2633,7 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp)
unsigned char *buffer;
const int vpd_len = 8;
- if (sdkp->lbpme == 0)
+ if (sdkp->lbpme == 0 && sdkp->device->try_lbp == 0)
return;
buffer = kmalloc(vpd_len, GFP_KERNEL);
--
1.9.3
On Thu, Jul 24, 2014 at 08:47:39AM +0100, Sitsofe Wheeler wrote:
> On Wed, Jul 23, 2014 at 09:13:41PM +0100, Sitsofe Wheeler wrote:
> > On Wed, Jul 23, 2014 at 07:15:58AM -0700, Christoph Hellwig wrote:
> > > On Wed, Jul 23, 2014 at 03:10:28PM +0100, Sitsofe Wheeler wrote:
> > > > I'm not sure this alone will work - won't sdev_bflags/bflags have
> > > > already been built at this point?
> > >
> > > They've been built up, but we can still or new values into it. It looks
> > > fine to me from review, but if you can test it on an actualy hypverv
> > > setup that would be valueable feedback.
> >
> > The previous patches didn't work for me with a Windows 2012 R2 host with a
> > 3.16.0-rc6.x86_64-00076-g2f7d2ec-dirty guest. After applying
> >
> > > term project, this late in the 3.17 cycle I'd just like to merge
> > > something that gets discards on hyperv to work.
>
> OK how about the following patches:
>
> Sitsofe Wheeler (3):
> [SCSI] Add quirk for forcing logical block provisioning tests
> [SCSI] storvsc: Add Hyper-V logical block provisioning quirk
> [SCSI] Make LBP quirk skip lbpme checks
With the updated "Make LBP quirk skip lbpme checks" the above patches
enable discard on all Hyper-V disks (both VHDXs and passthrough SSDs) on
Windows 2012 R2 running a Linux 3.16.0-rc6.x86_64-00076-g2f7d2ec-dirty
guest.
Adding back the debugging and looking at just the discard messages shows
this:
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sd 1:0:0:3: [sdd] Discard mode: 2
sd 1:0:0:1: [sdc] Entering discard switch with NO LBP VPD
sd 1:0:0:1: [sdc] Discard mode: 1
sd 1:0:0:3: [sdd] Entering discard switch via LBP VPD
sd 1:0:0:3: [sdd] Discard mode: 1
sd 1:0:0:0: [sdb] Discard mode: 2
sd 1:0:0:3: [sdd] Discard mode: 2
sd 1:0:0:1: [sdc] Entering discard switch with NO LBP VPD
sd 1:0:0:1: [sdc] Discard mode: 1
sd 1:0:0:3: [sdd] Entering discard switch via LBP VPD
sd 1:0:0:3: [sdd] Discard mode: 1
sd 1:0:0:1: [sdc] Entering discard switch with NO LBP VPD
sd 1:0:0:1: [sdc] Discard mode: 1
sd 1:0:0:0: [sdb] Entering discard switch via LBP VPD
sd 1:0:0:0: [sdb] Discard mode: 1
sd 1:0:0:3: [sdd] Discard mode: 2
sd 1:0:0:0: [sdb] Discard mode: 2
sd 1:0:0:0: [sdb] Entering discard switch via LBP VPD
sd 1:0:0:0: [sdb] Discard mode: 1
sd 1:0:0:0: [sdb] Discard mode: 2
sd 1:0:0:0: [sdb] Entering discard switch via LBP VPD
sd 1:0:0:0: [sdb] Discard mode: 1
sd 1:0:0:3: [sdd] Entering discard switch via LBP VPD
sd 1:0:0:3: [sdd] Discard mode: 1
All devices eventually end up using discard mode 1 (SD_LBP_UNMAP) but
it's a bit strange the VHDX devices (which set lbpme) flip back and
forth between discard mode 1 and discard mode 2 (SD_LBP_WS16)...
--
Sitsofe | http://sucs.org/~sits/
>>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Sitsofe> Fix incorrectly named variable. Some block devices (such as
Sitsofe> Hyper-V passthrough SSDs) support logical block provisioning
Sitsofe> (e.g. via UNMAP) but don't set lbpme thus disabling discard.
The fix for an SSD that is known to support LBP but which does not claim
support for it is to use:
echo unmap > /sys/class/scsi_disk/foo/provisioning_mode
I'm very much against short-circuiting the LBP logic in a passthrough
driver because then we might end up in the exact situation we were
trying to avoid with this patch series. Namely sending down commands
unsupported by the target device.
This kind of thing really needs to be a sysadmin decision and can be
handled with a udev rule.
--
Martin K. Petersen Oracle Linux Engineering
On Thu, 2014-07-24 at 08:56 +0100, Sitsofe Wheeler wrote:
> Microsoft Hyper-V targets currently only claim SPC-2 compliance / no
> compliance indicated even though they implement post SPC-2 features
> which means those features are not tested for. Add a blacklist flag to
> Hyper-V devices that forces said testing.
This description is misleading: you don't force the test now, you force
the driver to send unmap commands down to the device.
> See https://lkml.org/lkml/2014/7/21/627 for the previous version of this
> patch and https://lkml.org/lkml/2014/7/23/615 for example devices.
>
> Original-patch-by: K. Y. Srinivasan <[email protected]>
> Signed-off-by: Sitsofe Wheeler <[email protected]>
> ---
> drivers/scsi/storvsc_drv.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index 5ad2810..88b7173 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -326,8 +326,6 @@ MODULE_PARM_DESC(storvsc_ringbuffer_size, "Ring buffer size (bytes)");
> */
> static int storvsc_timeout = 180;
>
> -static int msft_blist_flags = BLIST_TRY_VPD_PAGES;
> -
> #define STORVSC_MAX_IO_REQUESTS 200
>
> static void storvsc_on_channel_callback(void *context);
> @@ -1444,12 +1442,10 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
> sdevice->no_write_same = 1;
>
> /*
> - * Add blist flags to permit the reading of the VPD pages even when
> - * the target may claim SPC-2 compliance. MSFT targets currently
> - * claim SPC-2 compliance while they implement post SPC-2 features.
> - * With this patch we can correctly handle WRITE_SAME_16 issues.
> + * Forcefully enable logical block provisioning testing.
> */
> - sdevice->sdev_bflags |= msft_blist_flags;
> + sdevice->sdev_bflags |= BLIST_TRY_LBP;
> + sdevice->try_lbp = 1;
Really no way to this one. You're forcing unmap support on every
hyper-v device; including spinning rust pass through ones which won't
support it. The hyper-v storage interface has proven to be somewhat
fragile, so it may explode when the device tries to send illegal command
sense back.
If you have a specific IDENTITY string for a faulty SSD that fails to
report unmap support correctly, then we could quirk for that, but not
everything attached to the hyper-v driver.
James
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Thu, Jul 24, 2014 at 09:54:24AM -0400, Martin K. Petersen wrote:
> I'm very much against short-circuiting the LBP logic in a passthrough
> driver because then we might end up in the exact situation we were
> trying to avoid with this patch series. Namely sending down commands
> unsupported by the target device.
>
> This kind of thing really needs to be a sysadmin decision and can be
> handled with a udev rule.
I agree - I'd like to pull in KY's simple fix as soon as I get a second
review for it.
On Thu, Jul 24, 2014 at 08:34:19AM -0700, Christoph Hellwig wrote:
> I agree - I'd like to pull in KY's simple fix as soon as I get a second
> review for it.
Ok, looks like I just got that from Hannes. Let's see if there's more
to be done for the pass through case, but I'd rather wait for the next
window for that.
On Thu, Jul 24, 2014 at 09:54:24AM -0400, Martin K. Petersen wrote:
> >>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
>
> Sitsofe> Fix incorrectly named variable. Some block devices (such as
> Sitsofe> Hyper-V passthrough SSDs) support logical block provisioning
> Sitsofe> (e.g. via UNMAP) but don't set lbpme thus disabling discard.
>
> The fix for an SSD that is known to support LBP but which does not claim
> support for it is to use:
>
> echo unmap > /sys/class/scsi_disk/foo/provisioning_mode
>
> I'm very much against short-circuiting the LBP logic in a passthrough
> driver because then we might end up in the exact situation we were
> trying to avoid with this patch series. Namely sending down commands
> unsupported by the target device.
>
> This kind of thing really needs to be a sysadmin decision and can be
> handled with a udev rule.
The problem is that the SSD _does_ claim to support it. Here are the
results of booting Linux directly on the host hardware and looking at
the device directly with a 3.10.35 kernel:
root@sysresccd /root % uname -a
Linux sysresccd 3.10.35-std420-amd64 #2 SMP Wed Apr 2 18:31:51 UTC 2014 x86_64 Intel(R) Core(TM) i7-3930K CPU @ 3.20GHz GenuineIntel GNU/Linux
root@sysresccd /root % sg_vpd -p lbpv /dev/sdb
root@sysresccd /root % sg_inq /dev/sdb
standard INQUIRY:
PQual=0 Device_type=0 RMB=0 version=0x05 [SPC-3]
[AERC=0] [TrmTsk=0] NormACA=0 HiSUP=0 Resp_data_format=2
SCCS=0 ACC=0 TPGS=0 3PC=0 Protect=0 [BQue=0]
EncServ=0 MultiP=0 [MChngr=0] [ACKREQQ=0] Addr16=0
[RelAdr=0] WBus16=0 Sync=0 Linked=0 [TranDis=0] CmdQue=0
[SPI: Clocking=0x0 QAS=0 IUS=0]
length=96 (0x60) Peripheral device type: disk
Vendor identification: ATA
Product identification: ADATA SSD S510 1
Product revision level: 5.2.
Unit serial number: 03205115500300002076
root@sysresccd /root % sg_readcap -l /dev/sdb
Read Capacity results:
Protection: prot_en=0, p_type=0, p_i_exponent=0
Logical block provisioning: lbpme=1, lbprz=0
Last logical block address=234441647 (0xdf94baf), Number of logical blocks=234441648
Logical block length=512 bytes
Logical blocks per physical block exponent=0
Lowest aligned logical block address=0
Hence:
Device size: 120034123776 bytes, 114473.5 MiB, 120.03 GB
Logical block provisioning VPD page (SBC):
Unmap command supported (LBPU): 0
Write same (16) with unmap bit supported (LBWS): 1
Write same (10) with unmap bit supported (LBWS10): 0
Logical block provisioning read zeros (LBPRZ): 0
Anchored LBAs supported (ANC_SUP): 0
Threshold exponent: 0
Descriptor present (DP): 0
Provisioning type: 0
root@sysresccd /root % sg_vpd -p bl /dev/sdb
Block limits VPD page (SBC):
Write same no zero (WSNZ): 0
Maximum compare and write length: 0 blocks
Optimal transfer length granularity: 1 blocks
Maximum transfer length: 0 blocks
Optimal transfer length: 0 blocks
Maximum prefetch length: 0 blocks
Maximum unmap LBA count: 0
Maximum unmap block descriptor count: 0
Optimal unmap granularity: 1
Unmap granularity alignment valid: 0
Unmap granularity alignment: 0
Maximum write same length: 0x3fffc0 blocks
root@sysresccd /root % grep . /sys/block/sdb/device/scsi_disk/1:0:0:0/*
/sys/block/sdb/device/scsi_disk/1:0:0:0/allow_restart:1
/sys/block/sdb/device/scsi_disk/1:0:0:0/app_tag_own:0
/sys/block/sdb/device/scsi_disk/1:0:0:0/cache_type:write back
grep: /sys/block/sdb/device/scsi_disk/1:0:0:0/device: Is a directory
/sys/block/sdb/device/scsi_disk/1:0:0:0/FUA:0
/sys/block/sdb/device/scsi_disk/1:0:0:0/manage_start_stop:1
/sys/block/sdb/device/scsi_disk/1:0:0:0/max_medium_access_timeouts:2
/sys/block/sdb/device/scsi_disk/1:0:0:0/max_write_same_blocks:0
grep: /sys/block/sdb/device/scsi_disk/1:0:0:0/power: Is a directory
/sys/block/sdb/device/scsi_disk/1:0:0:0/protection_mode:none
/sys/block/sdb/device/scsi_disk/1:0:0:0/protection_type:0
/sys/block/sdb/device/scsi_disk/1:0:0:0/provisioning_mode:writesame_16
grep: /sys/block/sdb/device/scsi_disk/1:0:0:0/subsystem: Is a directory
/sys/block/sdb/device/scsi_disk/1:0:0:0/thin_provisioning:1
So we can see it is really a SATA device that announces discard
correctly and supports discard through WRITE_SAME(16). It is the act of
passing it through Hyper-V that turned it into a SCSI device that supports
UNMAP (but not WRITE_SAME(16)), doesn't announce its SCSI conformance number
and doesn't correctly announce which features it supports. Surely in
this case it's reasonable to quirk our way around the problem?
--
Sitsofe | http://sucs.org/~sits/
>>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
Sitsofe> So we can see it is really a SATA device that announces discard
Sitsofe> correctly and supports discard through WRITE_SAME(16).
No, that's the SATA device that announces support for DSM TRIM, and as a
result the Linux SATL reports support for WRITE SAME(16) w. the UNMAP
bit set and LBPME.
Sitsofe> It is the act of passing it through Hyper-V that turned it into
Sitsofe> a SCSI device that supports UNMAP (but not WRITE_SAME(16)),
Sitsofe> doesn't announce its SCSI conformance number and doesn't
Sitsofe> correctly announce which features it supports. Surely in this
Sitsofe> case it's reasonable to quirk our way around the problem?
No. That's an issue in Hyper-V that'll you'll have to take up with
Microsoft. I don't know what their passthrough limitations are for
SCSI-ATA translation. Maybe K. Y. has some insight into this?
There must be a reason why the VPD page was added and yet the device not
flagged as LBPME=1.
Many vendors do not support UNMAP/WRITE SAME to DSM TRIM translation.
Additionally, many vendors explicitly only whitelist drives that are
known to be working correctly. Your drive is an ADATA and therefore very
likely to be blacklisted by default by a vendor SATL.
--
Martin K. Petersen Oracle Linux Engineering
On Thu, Jul 24, 2014 at 08:35:13AM -0700, Christoph Hellwig wrote:
> On Thu, Jul 24, 2014 at 08:34:19AM -0700, Christoph Hellwig wrote:
> > I agree - I'd like to pull in KY's simple fix as soon as I get a second
> > review for it.
>
> Ok, looks like I just got that from Hannes. Let's see if there's more
> to be done for the pass through case, but I'd rather wait for the next
> window for that.
There's nothing Linux can do for today's Hyper-V passthrough wrt to
automatically turning on discard on my SATA SSD.
Martin explained that vendors often disable UNMAP/WRITE SAME to DSM TRIM
translation on purpose and/or have explicit device whitelists/blacklists
for those devices they want to support. It could be that Microsoft are
doing something similar (although it might be interesting to know how a
Windows 2012 guest on Hyper-V handles things).
--
Sitsofe | http://sucs.org/~sits/
On Thu, Jul 24, 2014 at 02:09:11PM +0000, James Bottomley wrote:
> On Thu, 2014-07-24 at 08:56 +0100, Sitsofe Wheeler wrote:
> > Microsoft Hyper-V targets currently only claim SPC-2 compliance / no
> > compliance indicated even though they implement post SPC-2 features
> > which means those features are not tested for. Add a blacklist flag to
> > Hyper-V devices that forces said testing.
>
> This description is misleading: you don't force the test now, you force
> the driver to send unmap commands down to the device.
I disagree - UNMAP will only be used if the vpd pages say that UNMAP is
supported by the device. I've added two hard disks (one SATA, one on USB) to
the VM and here's the debug output with all three patches applied and some
extra logging:
sd 1:0:0:4: [sdc] Entered read_capacity_16
sd 1:0:0:4: [sdc] Past illegal req
sd 1:0:0:4: [sdc] Protection check
sd 1:0:0:4: [sdc] Got past protection check
sd 1:0:0:4: [sdc] Checking LBPME
sd 1:0:0:4: [sdc] 1953525168 512-byte logical blocks: (1.00 TB/931 GiB)
sd 1:0:0:4: [sdc] 4096-byte physical blocks
sd 1:0:0:4: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:4: [sdc] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:4: [sdc] Entered block limits
sd 1:0:0:4: [sdc] Started block limits
sd 1:0:0:4: [sdc] 0x3c...
sd 1:0:0:4: [sdc] Write Protect is off
sd 1:0:0:4: [sdc] Mode Sense: 0f 00 00 00
sd 1:0:0:2: [sdd] Entered read_capacity_16
sd 1:0:0:4: [sdc] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:0:2: [sdd] 976773168 512-byte logical blocks: (500 GB/465 GiB)
sd 1:0:0:4: [sdc] Entered read_capacity_16
sd 1:0:0:2: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:2: [sdd] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:4: [sdc] Past illegal req
sd 1:0:0:4: [sdc] Protection check
sd 1:0:0:4: [sdc] Got past protection check
sd 1:0:0:4: [sdc] Checking LBPME
sd 1:0:0:2: [sdd] Entered block limits
sd 1:0:0:4: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:4: [sdc] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:4: [sdc] Entered block limits
sd 1:0:0:4: [sdc] Started block limits
sd 1:0:0:4: [sdc] 0x3c...
sd 1:0:0:2: [sdd] Write Protect is off
sd 1:0:0:2: [sdd] Mode Sense: 0f 00 00 00
sd 1:0:0:2: [sdd] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
sd 1:0:0:2: [sdd] Entered read_capacity_16
sd 1:0:0:2: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:2: [sdd] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:2: [sdd] Entered block limits
sdd: unknown partition table
sd 1:0:0:2: [sdd] Entered read_capacity_16
sd 1:0:0:2: [sdd] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:2: [sdd] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:2: [sdd] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:2: [sdd] Entered block limits
sd 1:0:0:2: [sdd] Attached SCSI disk
sdc: sdc1 sdc2
sd 1:0:0:4: [sdc] Entered read_capacity_16
sd 1:0:0:4: [sdc] Past illegal req
sd 1:0:0:4: [sdc] Protection check
sd 1:0:0:4: [sdc] Got past protection check
sd 1:0:0:4: [sdc] Checking LBPME
sd 1:0:0:4: [sdc] sd_revalidate_disk: Extended inquiry check...
sd 1:0:0:4: [sdc] sd_revalidate_disk: Performing extended inquiries
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Entered, lbmpe: 0
sd 1:0:0:4: [sdc] sd_read_block_provisioning: Passed lbmpe test
sd 1:0:0:4: [sdc] Entered block limits
sd 1:0:0:4: [sdc] Started block limits
sd 1:0:0:4: [sdc] 0x3c...
sd 1:0:0:4: [sdc] Attached SCSI disk
Both
/sys/block/sdc/device/scsi_disk/1\:0\:0\:4/thin_provisioning
and
cat /sys/block/sdd/device/scsi_disk/1\:0\:0\:2/thin_provisioning
return 0.
A Hyper-V VHDX disk had output like this:
sd 0:0:0:0: [sda] Entered read_capacity_16
sd 0:0:0:0: [sda] Past illegal req
sd 0:0:0:0: [sda] Protection check
sd 0:0:0:0: [sda] Got past protection check
sd 0:0:0:0: [sda] Checking LBPME
sd 0:0:0:0: [sda] LBPME OK!
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] 8388608 512-byte logical blocks: (4.29 GB/4.00 GiB)
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Performing extended inquiries
sd 0:0:0:0: [sda] sd_read_block_provisioning: Entered, lbmpe: 1
sd 0:0:0:0: [sda] sd_read_block_provisioning: Passed lbmpe test
sd 0:0:0:0: [sda] sd_read_block_provisioning: Setting block provisioning
sd 0:0:0:0: [sda] Entered block limits
sd 0:0:0:0: [sda] Started block limits
sd 0:0:0:0: [sda] 0x3c...
sd 0:0:0:0: [sda] Testing lbpme...
sd 0:0:0:0: [sda] ...lbpme test done
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sd 0:0:0:0: [sda] Write Protect is off
sd 0:0:0:0: [sda] Mode Sense: 0f 00 00 00
sd 0:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA
sd 0:0:0:0: [sda] Entered read_capacity_16
sd 0:0:0:0: [sda] Past illegal req
sd 0:0:0:0: [sda] Protection check
sd 0:0:0:0: [sda] Got past protection check
sd 0:0:0:0: [sda] Checking LBPME
sd 0:0:0:0: [sda] LBPME OK!
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Performing extended inquiries
sd 0:0:0:0: [sda] sd_read_block_provisioning: Entered, lbmpe: 1
sd 0:0:0:0: [sda] sd_read_block_provisioning: Passed lbmpe test
sd 0:0:0:0: [sda] sd_read_block_provisioning: Setting block provisioning
sd 0:0:0:0: [sda] Entered block limits
sd 0:0:0:0: [sda] Started block limits
sd 0:0:0:0: [sda] 0x3c...
sd 0:0:0:0: [sda] Testing lbpme...
sd 0:0:0:0: [sda] ...lbpme test done
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sda: sda1
sd 0:0:0:0: [sda] Entered read_capacity_16
sd 0:0:0:0: [sda] Past illegal req
sd 0:0:0:0: [sda] Protection check
sd 0:0:0:0: [sda] Got past protection check
sd 0:0:0:0: [sda] Checking LBPME
sd 0:0:0:0: [sda] LBPME OK!
sd 0:0:0:0: [sda] Discard mode: 2
sd 0:0:0:0: [sda] sd_revalidate_disk: Extended inquiry check...
sd 0:0:0:0: [sda] sd_revalidate_disk: Performing extended inquiries
sd 0:0:0:0: [sda] sd_read_block_provisioning: Entered, lbmpe: 1
sd 0:0:0:0: [sda] sd_read_block_provisioning: Passed lbmpe test
sd 0:0:0:0: [sda] sd_read_block_provisioning: Setting block provisioning
sd 0:0:0:0: [sda] Entered block limits
sd 0:0:0:0: [sda] Started block limits
sd 0:0:0:0: [sda] 0x3c...
sd 0:0:0:0: [sda] Testing lbpme...
sd 0:0:0:0: [sda] ...lbpme test done
sd 0:0:0:0: [sda] Entering discard switch via LBP VPD
sd 0:0:0:0: [sda] Discard mode: 1
sd 0:0:0:0: [sda] Attached SCSI disk
The hard disk never set the Discard mode so why would UNMAP be forced.
> > /*
> > - * Add blist flags to permit the reading of the VPD pages even when
> > - * the target may claim SPC-2 compliance. MSFT targets currently
> > - * claim SPC-2 compliance while they implement post SPC-2 features.
> > - * With this patch we can correctly handle WRITE_SAME_16 issues.
> > + * Forcefully enable logical block provisioning testing.
> > */
> > - sdevice->sdev_bflags |= msft_blist_flags;
> > + sdevice->sdev_bflags |= BLIST_TRY_LBP;
> > + sdevice->try_lbp = 1;
>
> Really no way to this one. You're forcing unmap support on every
> hyper-v device; including spinning rust pass through ones which won't
> support it. The hyper-v storage interface has proven to be somewhat
> fragile, so it may explode when the device tries to send illegal command
> sense back.
OK this one I can't deny but I couldn't see how else to cope with passthrough
disks (see below)...
> If you have a specific IDENTITY string for a faulty SSD that fails to
> report unmap support correctly, then we could quirk for that, but not
> everything attached to the hyper-v driver.
Let me be clear that there are two cases:
1. Hyper-V VHDX files.
2. Passthrough devices (such as SSDs) that support discard.
We can definitely identify 1. (and I have an unposted patch that
modified the struct in scsi_devinfo.c to only quirk on Hyper-V VHDX
devices) but with 2. the SATA SSD I have enables discard fine with Linux
on the host directly - it's the act of passing it through Hyper-V that
causes the issue (no SCSI conformance level is claimed and lbmpe is not
set) and that could theoretically apply to all TRIM supporting SATA SSDs
that are passed through Hyper-V.
Additionally K. Y. Srinivasan had a similar (but slightly different)
patch to my unposted one and Christoph said:
> This looks way to complicated - should be a single line added to your
> slave_configure function, maybe plus a comment stating what you do
> in your commit message:
>
>
> sdev->sdev_bflags |= BLIST_TRY_VPD_PAGES;
So it was changed to be similar to impact everything attached to
Hyper-V. If you don't like this you may want to chime in on
https://lkml.org/lkml/2014/7/24/33 (Re: [PATCH 1/1] Drivers: scsi:
storvsc: Add blist flags).
--
Sitsofe | http://sucs.org/~sits/
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Thursday, July 24, 2014 8:54 AM
> To: Sitsofe Wheeler
> Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
>
> Sitsofe> So we can see it is really a SATA device that announces discard
> Sitsofe> correctly and supports discard through WRITE_SAME(16).
>
> No, that's the SATA device that announces support for DSM TRIM, and as a
> result the Linux SATL reports support for WRITE SAME(16) w. the UNMAP bit
> set and LBPME.
>
> Sitsofe> It is the act of passing it through Hyper-V that turned it into
> Sitsofe> a SCSI device that supports UNMAP (but not WRITE_SAME(16)),
> Sitsofe> doesn't announce its SCSI conformance number and doesn't
> Sitsofe> correctly announce which features it supports. Surely in this
> Sitsofe> case it's reasonable to quirk our way around the problem?
>
> No. That's an issue in Hyper-V that'll you'll have to take up with Microsoft. I
> don't know what their passthrough limitations are for SCSI-ATA translation.
> Maybe K. Y. has some insight into this?
For the pass through case, the host validates the request and passes the request to the device.
However, not all scsi commands are passed through even though the device it is being passed through
may support the command. WRITE_SAME is one such command. Consequently, in the EVPD page,
we will set state indicating that WRITE_SAME is not supported (even if the device supports it).
Hope this helps.
K. Y
>
> There must be a reason why the VPD page was added and yet the device not
> flagged as LBPME=1.
>
> Many vendors do not support UNMAP/WRITE SAME to DSM TRIM
> translation.
> Additionally, many vendors explicitly only whitelist drives that are known to
> be working correctly. Your drive is an ADATA and therefore very likely to be
> blacklisted by default by a vendor SATL.
>
> --
> Martin K. Petersen Oracle Linux Engineering
>>>>> "KY" == KY Srinivasan <[email protected]> writes:
KY> For the pass through case, the host validates the request and passes
KY> the request to the device. However, not all scsi commands are
KY> passed through even though the device it is being passed through may
KY> support the command. WRITE_SAME is one such command. Consequently,
KY> in the EVPD page, we will set state indicating that WRITE_SAME is
KY> not supported (even if the device supports it).
The LBP VPD page flags UNMAP as being supported. Do you actually support
UNMAP to DSM TRIM SCSI-ATA translation?
One challenge in that department is that a single UNMAP command may turn
into many, many, many DSM TRIM commands on the underlying SATA device.
That's why we went with WRITE SAME for the internal Linux SATL, capping
the maximum number of blocks to what we can fit in a single DSM TRIM
command.
--
Martin K. Petersen Oracle Linux Engineering
On Fri, 2014-07-25 at 16:47 +0000, KY Srinivasan wrote:
>
> > -----Original Message-----
> > From: Martin K. Petersen [mailto:[email protected]]
> > Sent: Thursday, July 24, 2014 8:54 AM
> > To: Sitsofe Wheeler
> > Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan;
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]
> > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
> >
> > >>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
> >
> > Sitsofe> So we can see it is really a SATA device that announces discard
> > Sitsofe> correctly and supports discard through WRITE_SAME(16).
> >
> > No, that's the SATA device that announces support for DSM TRIM, and as a
> > result the Linux SATL reports support for WRITE SAME(16) w. the UNMAP bit
> > set and LBPME.
> >
> > Sitsofe> It is the act of passing it through Hyper-V that turned it into
> > Sitsofe> a SCSI device that supports UNMAP (but not WRITE_SAME(16)),
> > Sitsofe> doesn't announce its SCSI conformance number and doesn't
> > Sitsofe> correctly announce which features it supports. Surely in this
> > Sitsofe> case it's reasonable to quirk our way around the problem?
> >
> > No. That's an issue in Hyper-V that'll you'll have to take up with Microsoft. I
> > don't know what their passthrough limitations are for SCSI-ATA translation.
> > Maybe K. Y. has some insight into this?
>
> For the pass through case, the host validates the request and passes
> the request to the device.
> However, not all scsi commands are passed through even though the
> device it is being passed through
> may support the command. WRITE_SAME is one such command. Consequently,
> in the EVPD page,
> we will set state indicating that WRITE_SAME is not supported (even if
> the device supports it).
I think you haven't appreciated the problem: He's passing a SATA SSD via
the SCSI hyper-v interface. That means that the windows host is doing
SCSI<->ATA translation. The problem is that the Windows translation
layer (SATL) looks to be incomplete and it's not correctly translating
the IDENTIFY bit that corresponds to TRIM to the correct VPD pages so
consequently, Linux won't send UNMAP commands to the device (to be
translated back to TRIM).
We already know this is a bug in the Windows SATL which needs fixing (if
you could report it and get a fix, that would be great) and that we're
not going to be able to work around this automatically in Linux because
the proposed patch would have us unconditionally try UNMAP for all
Hyper-V devices. The current proposed fix is to enable UNMAP manually
via sysfs in the guest boot scripts, but obviously that means that
Hyper-V guests with direct pass through of SSDs need operator
intervention to turn on TRIM.
James
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
> Sent: Friday, July 25, 2014 10:10 AM
> To: KY Srinivasan
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> On Fri, 2014-07-25 at 16:47 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: Martin K. Petersen [mailto:[email protected]]
> > > Sent: Thursday, July 24, 2014 8:54 AM
> > > To: Sitsofe Wheeler
> > > Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan;
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks
> > > tests
> > >
> > > >>>>> "Sitsofe" == Sitsofe Wheeler <[email protected]> writes:
> > >
> > > Sitsofe> So we can see it is really a SATA device that announces
> > > Sitsofe> discard correctly and supports discard through WRITE_SAME(16).
> > >
> > > No, that's the SATA device that announces support for DSM TRIM, and
> > > as a result the Linux SATL reports support for WRITE SAME(16) w. the
> > > UNMAP bit set and LBPME.
> > >
> > > Sitsofe> It is the act of passing it through Hyper-V that turned it
> > > Sitsofe> into a SCSI device that supports UNMAP (but not
> > > Sitsofe> WRITE_SAME(16)), doesn't announce its SCSI conformance
> > > Sitsofe> number and doesn't correctly announce which features it
> > > Sitsofe> supports. Surely in this case it's reasonable to quirk our way
> around the problem?
> > >
> > > No. That's an issue in Hyper-V that'll you'll have to take up with
> > > Microsoft. I don't know what their passthrough limitations are for SCSI-
> ATA translation.
> > > Maybe K. Y. has some insight into this?
> >
> > For the pass through case, the host validates the request and passes
> > the request to the device.
> > However, not all scsi commands are passed through even though the
> > device it is being passed through may support the command. WRITE_SAME
> > is one such command. Consequently, in the EVPD page, we will set state
> > indicating that WRITE_SAME is not supported (even if the device
> > supports it).
>
> I think you haven't appreciated the problem: He's passing a SATA SSD via the
> SCSI hyper-v interface. That means that the windows host is doing SCSI<-
> >ATA translation. The problem is that the Windows translation layer (SATL)
> looks to be incomplete and it's not correctly translating the IDENTIFY bit that
> corresponds to TRIM to the correct VPD pages so consequently, Linux won't
> send UNMAP commands to the device (to be translated back to TRIM).
>
> We already know this is a bug in the Windows SATL which needs fixing (if you
> could report it and get a fix, that would be great) and that we're not going to
> be able to work around this automatically in Linux because the proposed
> patch would have us unconditionally try UNMAP for all Hyper-V devices. The
> current proposed fix is to enable UNMAP manually via sysfs in the guest boot
> scripts, but obviously that means that Hyper-V guests with direct pass
> through of SSDs need operator intervention to turn on TRIM.
James,
Thanks for the clarification. I am talking to the folks in MSFT that develop the native scsi
stack on Windows. Hyper-V's back-end driver is not involved in SATL. I will keep you guys
posted.
Regards,
K. Y
>
> James
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Friday, July 25, 2014 9:57 AM
> To: KY Srinivasan
> Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "KY" == KY Srinivasan <[email protected]> writes:
>
> KY> For the pass through case, the host validates the request and passes
> KY> the request to the device. However, not all scsi commands are
> KY> passed through even though the device it is being passed through may
> KY> support the command. WRITE_SAME is one such command.
> Consequently,
> KY> in the EVPD page, we will set state indicating that WRITE_SAME is
> KY> not supported (even if the device supports it).
>
> The LBP VPD page flags UNMAP as being supported. Do you actually support
> UNMAP to DSM TRIM SCSI-ATA translation?
Martin,
I have been told by the Windows folks that this is done. I am trying to get
additional details.
K. Y
>
> One challenge in that department is that a single UNMAP command may turn
> into many, many, many DSM TRIM commands on the underlying SATA
> device.
> That's why we went with WRITE SAME for the internal Linux SATL, capping
> the maximum number of blocks to what we can fit in a single DSM TRIM
> command.
>
> --
> Martin K. Petersen Oracle Linux Engineering
>>>>> "KY" == KY Srinivasan <[email protected]> writes:
>> The LBP VPD page flags UNMAP as being supported. Do you actually
>> support UNMAP to DSM TRIM SCSI-ATA translation?
KY> I have been told by the Windows folks that this is done. I am trying
KY> to get additional details.
Great! I'd just like to have a reasonable level of confidence in what's
happening down the stack before I entertain turning something on that's
not being properly advertised.
--
Martin K. Petersen Oracle Linux Engineering
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Saturday, July 26, 2014 9:55 AM
> To: KY Srinivasan
> Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "KY" == KY Srinivasan <[email protected]> writes:
>
> >> The LBP VPD page flags UNMAP as being supported. Do you actually
> >> support UNMAP to DSM TRIM SCSI-ATA translation?
>
> KY> I have been told by the Windows folks that this is done. I am trying
> KY> to get additional details.
>
> Great! I'd just like to have a reasonable level of confidence in what's
> happening down the stack before I entertain turning something on that's not
> being properly advertised.
As I look at the output of inquiry between Linux on Hyper-V and native Linux, is not specifying
conformance level the main issue?
K. Y
>
> --
> Martin K. Petersen Oracle Linux Engineering
>>>>> "KY" == KY Srinivasan <[email protected]> writes:
>> Great! I'd just like to have a reasonable level of confidence in
>> what's happening down the stack before I entertain turning something
>> on that's not being properly advertised.
KY> As I look at the output of inquiry between Linux on Hyper-V and
KY> native Linux, is not specifying conformance level the main issue?
The main problem for this particular use case (aside from the issue
we've already addressed) is that the passthrough device (SATA SSD) has
LBPME=0 in the READ CAPACITY(16) response. The LBP VPD is correctly
provided with LBPU flag set but because LBPME is reported as disabled we
will not attempt to issue UNMAP commands to the device.
--
Martin K. Petersen Oracle Linux Engineering
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Saturday, July 26, 2014 12:25 PM
> To: KY Srinivasan
> Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "KY" == KY Srinivasan <[email protected]> writes:
>
> >> Great! I'd just like to have a reasonable level of confidence in
> >> what's happening down the stack before I entertain turning something
> >> on that's not being properly advertised.
>
> KY> As I look at the output of inquiry between Linux on Hyper-V and
> KY> native Linux, is not specifying conformance level the main issue?
>
> The main problem for this particular use case (aside from the issue we've
> already addressed) is that the passthrough device (SATA SSD) has
> LBPME=0 in the READ CAPACITY(16) response. The LBP VPD is correctly
> provided with LBPU flag set but because LBPME is reported as disabled we
> will not attempt to issue UNMAP commands to the device.
Oh; ok. I missed the read_capacity response.
Thanks,
K. Y
>
> --
> Martin K. Petersen Oracle Linux Engineering
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Saturday, July 26, 2014 12:25 PM
> To: KY Srinivasan
> Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "KY" == KY Srinivasan <[email protected]> writes:
>
> >> Great! I'd just like to have a reasonable level of confidence in
> >> what's happening down the stack before I entertain turning something
> >> on that's not being properly advertised.
>
> KY> As I look at the output of inquiry between Linux on Hyper-V and
> KY> native Linux, is not specifying conformance level the main issue?
>
> The main problem for this particular use case (aside from the issue we've
> already addressed) is that the passthrough device (SATA SSD) has
> LBPME=0 in the READ CAPACITY(16) response. The LBP VPD is correctly
> provided with LBPU flag set but because LBPME is reported as disabled we
> will not attempt to issue UNMAP commands to the device.
Had a chance to chat with the Windows sscsi folks. Here is their response:
"At the time thin-provisioning was defined, the discovery information was first proposed in READ CAPACITY 16 command. And then moved into the new dedicated VPD page - B2h. You can see the information reported in this VPD page is richer than READ CAPACITY 16 command. As this transition happened during we added the feature, Windows uses the newer method that based on VPD page B2h. It looks Linux tries to use both new and old method which is weird to me."
Would it make sense for us to work around this issue for Linux on Hyper-V (just use the VPD page instead of looking at the output of READ CAPACITY)?
Regards,
K. Y
>
> --
> Martin K. Petersen Oracle Linux Engineering
>>>>> "KY" == KY Srinivasan <[email protected]> writes:
KY,
KY> "At the time thin-provisioning was defined, the discovery
KY> information was first proposed in READ CAPACITY 16 command. And then
KY> moved into the new dedicated VPD page - B2h. You can see the
KY> information reported in this VPD page is richer than READ CAPACITY
KY> 16 command. As this transition happened during we added the feature,
KY> Windows uses the newer method that based on VPD page B2h. It looks
KY> Linux tries to use both new and old method which is weird to me."
The READ CAPACITY(16) response is not optional.
SBC3r36 section 4.7.3.3 Thin provisioned logical unit:
The device server in a thin provisioned logical unit shall set:
a) the LBPME bit to one in the READ CAPACITY (16) parameter data (see
5.16.2); and
b) the PROVISIONING TYPE field to 010b (i.e., thin provisioned) in the
Logical Block Provisioning VPD page (see 6.6.4).
That's a "shall". The LBP VPD elaborates on the provisioning type,
commands preference, etc. But it's all gated by LBPME=1 in the READ
CAPACITY(16) response.
--
Martin K. Petersen Oracle Linux Engineering
> -----Original Message-----
> From: Martin K. Petersen [mailto:[email protected]]
> Sent: Monday, July 28, 2014 12:03 PM
> To: KY Srinivasan
> Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> >>>>> "KY" == KY Srinivasan <[email protected]> writes:
>
> KY,
>
> KY> "At the time thin-provisioning was defined, the discovery
> KY> information was first proposed in READ CAPACITY 16 command. And
> then
> KY> moved into the new dedicated VPD page - B2h. You can see the
> KY> information reported in this VPD page is richer than READ CAPACITY
> KY> 16 command. As this transition happened during we added the feature,
> KY> Windows uses the newer method that based on VPD page B2h. It looks
> KY> Linux tries to use both new and old method which is weird to me."
>
> The READ CAPACITY(16) response is not optional.
Ok; that settles the issue then. I will attempt to get it fixed on Windows.
K. Y
>
> --
> Martin K. Petersen Oracle Linux Engineering
On Mon, 2014-07-28 at 19:05 +0000, KY Srinivasan wrote:
>
> > -----Original Message-----
> > From: Martin K. Petersen [mailto:[email protected]]
> > Sent: Monday, July 28, 2014 12:03 PM
> > To: KY Srinivasan
> > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> > [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; linux-
> > [email protected]
> > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
> >
> > >>>>> "KY" == KY Srinivasan <[email protected]> writes:
> >
> > KY,
> >
> > KY> "At the time thin-provisioning was defined, the discovery
> > KY> information was first proposed in READ CAPACITY 16 command. And
> > then
> > KY> moved into the new dedicated VPD page - B2h. You can see the
> > KY> information reported in this VPD page is richer than READ CAPACITY
> > KY> 16 command. As this transition happened during we added the feature,
> > KY> Windows uses the newer method that based on VPD page B2h. It looks
> > KY> Linux tries to use both new and old method which is weird to me."
> >
> > The READ CAPACITY(16) response is not optional.
>
> Ok; that settles the issue then. I will attempt to get it fixed on Windows.
Like Martin says, this isn't optional either/or; it's mandatory to
support the RC 16 bits. If you don't want to get into playing the
messenger between us and the windows guys on SCSI standards, we'd be
happy to communicate directly, either by email or a phone meeting.
James
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
> Sent: Monday, July 28, 2014 1:03 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> On Mon, 2014-07-28 at 19:05 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: Martin K. Petersen [mailto:[email protected]]
> > > Sent: Monday, July 28, 2014 12:03 PM
> > > To: KY Srinivasan
> > > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks
> > > tests
> > >
> > > >>>>> "KY" == KY Srinivasan <[email protected]> writes:
> > >
> > > KY,
> > >
> > > KY> "At the time thin-provisioning was defined, the discovery
> > > KY> information was first proposed in READ CAPACITY 16 command. And
> > > then
> > > KY> moved into the new dedicated VPD page - B2h. You can see the
> > > KY> information reported in this VPD page is richer than READ
> > > KY> CAPACITY
> > > KY> 16 command. As this transition happened during we added the
> > > KY> feature, Windows uses the newer method that based on VPD page
> > > KY> B2h. It looks Linux tries to use both new and old method which is
> weird to me."
> > >
> > > The READ CAPACITY(16) response is not optional.
> >
> > Ok; that settles the issue then. I will attempt to get it fixed on Windows.
>
> Like Martin says, this isn't optional either/or; it's mandatory to support the RC
> 16 bits. If you don't want to get into playing the messenger between us and
> the windows guys on SCSI standards, we'd be happy to communicate
> directly, either by email or a phone meeting.
Thanks James. I will take up your offer if needed.
Regards,
K. Y
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> -----Original Message-----
> From: James Bottomley [mailto:[email protected]]
> Sent: Monday, July 28, 2014 1:03 PM
> To: KY Srinivasan
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests
>
> On Mon, 2014-07-28 at 19:05 +0000, KY Srinivasan wrote:
> >
> > > -----Original Message-----
> > > From: Martin K. Petersen [mailto:[email protected]]
> > > Sent: Monday, July 28, 2014 12:03 PM
> > > To: KY Srinivasan
> > > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig;
> > > [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; linux-
> > > [email protected]
> > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks
> > > tests
> > >
> > > >>>>> "KY" == KY Srinivasan <[email protected]> writes:
> > >
> > > KY,
> > >
> > > KY> "At the time thin-provisioning was defined, the discovery
> > > KY> information was first proposed in READ CAPACITY 16 command. And
> > > then
> > > KY> moved into the new dedicated VPD page - B2h. You can see the
> > > KY> information reported in this VPD page is richer than READ
> > > KY> CAPACITY
> > > KY> 16 command. As this transition happened during we added the
> > > KY> feature, Windows uses the newer method that based on VPD page
> > > KY> B2h. It looks Linux tries to use both new and old method which is
> weird to me."
> > >
> > > The READ CAPACITY(16) response is not optional.
> >
> > Ok; that settles the issue then. I will attempt to get it fixed on Windows.
>
> Like Martin says, this isn't optional either/or; it's mandatory to support the RC
> 16 bits. If you don't want to get into playing the messenger between us and
> the windows guys on SCSI standards, we'd be happy to communicate
> directly, either by email or a phone meeting.
We will fix this bug in the next release of Windows; we are also looking at backporting the fix to prior versions of Windows.
Thanks,
K. Y
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
>>>>> "KY" == KY Srinivasan <[email protected]> writes:
KY> We will fix this bug in the next release of Windows; we are also
KY> looking at backporting the fix to prior versions of Windows.
Excellent. Thanks for looking into this!
--
Martin K. Petersen Oracle Linux Engineering