* [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags @ 2014-07-21 23:06 K. Y. Srinivasan 2014-07-23 10:04 ` Sitsofe Wheeler ` (2 more replies) 0 siblings, 3 replies; 43+ messages in thread From: K. Y. Srinivasan @ 2014-07-21 23:06 UTC (permalink / raw) To: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi Cc: K. Y. Srinivasan 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 <kys@microsoft.com> --- 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-21 23:06 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan @ 2014-07-23 10:04 ` Sitsofe Wheeler 2014-07-23 11:51 ` Christoph Hellwig 2014-07-23 15:39 ` KY Srinivasan 2014-07-23 14:10 ` Sitsofe Wheeler 2014-07-24 5:40 ` [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags Hannes Reinecke 2 siblings, 2 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-23 10:04 UTC (permalink / raw) To: K. Y. Srinivasan Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 10:04 ` Sitsofe Wheeler @ 2014-07-23 11:51 ` Christoph Hellwig 2014-07-23 12:54 ` Sitsofe Wheeler 2014-07-23 15:39 ` KY Srinivasan 1 sibling, 1 reply; 43+ messages in thread From: Christoph Hellwig @ 2014-07-23 11:51 UTC (permalink / raw) To: Sitsofe Wheeler Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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.. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 11:51 ` Christoph Hellwig @ 2014-07-23 12:54 ` Sitsofe Wheeler 2014-07-23 14:10 ` Christoph Hellwig 0 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-23 12:54 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 12:54 ` Sitsofe Wheeler @ 2014-07-23 14:10 ` Christoph Hellwig 2014-07-23 15:31 ` Sitsofe Wheeler 2014-07-23 15:40 ` KY Srinivasan 0 siblings, 2 replies; 43+ messages in thread From: Christoph Hellwig @ 2014-07-23 14:10 UTC (permalink / raw) To: Sitsofe Wheeler Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 14:10 ` Christoph Hellwig @ 2014-07-23 15:31 ` Sitsofe Wheeler 2014-07-23 15:40 ` KY Srinivasan 1 sibling, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-23 15:31 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 14:10 ` Christoph Hellwig 2014-07-23 15:31 ` Sitsofe Wheeler @ 2014-07-23 15:40 ` KY Srinivasan 1 sibling, 0 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-23 15:40 UTC (permalink / raw) To: Christoph Hellwig, Sitsofe Wheeler Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi, Hannes Reinecke (hare@suse.de) > -----Original Message----- > From: Christoph Hellwig [mailto:hch@infradead.org] > Sent: Wednesday, July 23, 2014 7:10 AM > To: Sitsofe Wheeler > Cc: KY Srinivasan; gregkh@linuxfoundation.org; linux- > kernel@vger.kernel.org; devel@linuxdriverproject.org; ohering@suse.com; > apw@canonical.com; jasowang@redhat.com; jbottomley@parallels.com; > linux-scsi@vger.kernel.org > 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. ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 10:04 ` Sitsofe Wheeler 2014-07-23 11:51 ` Christoph Hellwig @ 2014-07-23 15:39 ` KY Srinivasan 1 sibling, 0 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-23 15:39 UTC (permalink / raw) To: Sitsofe Wheeler Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi > -----Original Message----- > From: Sitsofe Wheeler [mailto:sitsofe@gmail.com] > Sent: Wednesday, July 23, 2014 3:05 AM > To: KY Srinivasan > Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; hch@infradead.org; > linux-scsi@vger.kernel.org > 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-21 23:06 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan 2014-07-23 10:04 ` Sitsofe Wheeler @ 2014-07-23 14:10 ` Sitsofe Wheeler 2014-07-23 14:15 ` Christoph Hellwig 2014-07-24 5:40 ` [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags Hannes Reinecke 2 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-23 14:10 UTC (permalink / raw) To: K. Y. Srinivasan Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 14:10 ` Sitsofe Wheeler @ 2014-07-23 14:15 ` Christoph Hellwig 2014-07-23 20:13 ` Sitsofe Wheeler 0 siblings, 1 reply; 43+ messages in thread From: Christoph Hellwig @ 2014-07-23 14:15 UTC (permalink / raw) To: Sitsofe Wheeler Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-23 14:15 ` Christoph Hellwig @ 2014-07-23 20:13 ` Sitsofe Wheeler 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 0 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-23 20:13 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi [-- Attachment #1: Type: text/plain, Size: 6973 bytes --] 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/ [-- Attachment #2: 0001-Add-debugging-output-to-SCSI-disk-initalisation-to-t.patch --] [-- Type: text/x-diff, Size: 9362 bytes --] >From abc3c29ce756f8be6aa8a945a8c81b29d396dbb6 Mon Sep 17 00:00:00 2001 From: Sitsofe Wheeler <sitsofe@yahoo.com> Date: Wed, 23 Jul 2014 15:41:31 +0000 Subject: [PATCH] Add debugging output to SCSI disk initalisation to trace LBP testing. --- drivers/scsi/scsi.c | 5 +++++ drivers/scsi/scsi_devinfo.c | 7 +++++++ drivers/scsi/scsi_scan.c | 10 ++++++++-- drivers/scsi/sd.c | 28 ++++++++++++++++++++++++++++ drivers/scsi/storvsc_drv.c | 1 + 5 files changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 88d46fe..6a7a854 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -992,11 +992,13 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf, if (sdev->skip_vpd_pages) goto fail; + printk(KERN_NOTICE "Don't skip\n"); /* Ask for all the pages supported by this device */ result = scsi_vpd_inquiry(sdev, buf, 0, buf_len); if (result < 4) goto fail; + printk(KERN_NOTICE "Got all pages\n"); /* If the user actually wanted this page, we can skip the rest */ if (page == 0) return 0; @@ -1008,13 +1010,16 @@ int scsi_get_vpd_page(struct scsi_device *sdev, u8 page, unsigned char *buf, if (i < result && i >= buf_len) /* ran off the end of the buffer, give us benefit of doubt */ goto found; + printk(KERN_NOTICE "Couldn't find page\n"); /* The device claims it doesn't support the requested page */ goto fail; found: + printk(KERN_NOTICE "Read vpd page\n"); result = scsi_vpd_inquiry(sdev, buf, page, buf_len); if (result < 0) goto fail; + printk(KERN_NOTICE "Read vpd page success\n"); return 0; diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index f969aca..cbde61f 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -619,14 +619,21 @@ int scsi_get_device_flags_keyed(struct scsi_device *sdev, return devinfo->flags; } } + sdev_printk(KERN_NOTICE, sdev, + "scsi_get_device_flags_keyed: key: %d\n", + sdev->scsi_level); /* nothing found, return nothing */ if (key != SCSI_DEVINFO_GLOBAL) return 0; + sdev_printk(KERN_NOTICE, sdev, + "scsi_get_device_flags_keyed: Post SCSI_DEVINFO_GLOBAL\n"); /* except for the global list, where we have an exception */ if (sdev->sdev_bflags) return sdev->sdev_bflags; + sdev_printk(KERN_NOTICE, sdev, + "scsi_get_device_flags_keyed: No sdev_bflags\n"); return scsi_default_dev_flags; } EXPORT_SYMBOL(scsi_get_device_flags_keyed); diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 492cd70..7de78b8 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -734,6 +734,7 @@ static int scsi_probe_lun(struct scsi_device *sdev, unsigned char *inq_result, (sdev->scsi_level == 1 && (inq_result[3] & 0x0f) == 1)) sdev->scsi_level++; sdev->sdev_target->scsi_level = sdev->scsi_level; + sdev_printk(KERN_NOTICE, sdev, "sdev->scsi_level: %d\n", sdev->scsi_level); return 0; } @@ -950,9 +951,14 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result, sdev->eh_timeout = SCSI_DEFAULT_EH_TIMEOUT; - if (*bflags & BLIST_TRY_VPD_PAGES) + sdev_printk(KERN_NOTICE, sdev, + "scsi_add_lun: Have BLIST_TRY_VPD_PAGES? %s", + ((*bflags & BLIST_TRY_VPD_PAGES) ? "Yes" : "No")); + if (*bflags & BLIST_TRY_VPD_PAGES) { sdev->try_vpd_pages = 1; - else if (*bflags & BLIST_SKIP_VPD_PAGES) + sdev_printk(KERN_NOTICE, sdev, + "scsi_add_lun: Set try_vpd_pages"); + } 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 ed2e99e..b905e1e 100644 --- a/drivers/scsi/sd.c +++ b/drivers/scsi/sd.c @@ -636,6 +636,8 @@ static void sd_config_discard(struct scsi_disk *sdkp, unsigned int mode) sdkp->provisioning_mode = mode; + //dump_stack(); + sd_printk(KERN_NOTICE, sdkp, "Discard mode: %u\n", mode); switch (mode) { case SD_LBP_DISABLE: @@ -1947,6 +1949,7 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, unsigned long long lba; unsigned sector_size; + sd_printk(KERN_NOTICE, sdkp, "Entered read_capacity_16\n"); if (sdp->no_read_capacity_16) return -EINVAL; @@ -1985,6 +1988,7 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, retries--; } while (the_result && retries); + sd_printk(KERN_ERR, sdkp, "Past illegal req\n"); if (the_result) { sd_printk(KERN_NOTICE, sdkp, "READ CAPACITY(16) failed\n"); @@ -1995,10 +1999,13 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, sector_size = get_unaligned_be32(&buffer[8]); lba = get_unaligned_be64(&buffer[0]); + sd_printk(KERN_ERR, sdkp, "Protection check\n"); if (sd_read_protection_type(sdkp, buffer) < 0) { + sd_printk(KERN_ERR, sdkp, "Protection %d\n", sd_read_protection_type(sdkp, buffer)); sdkp->capacity = 0; return -ENODEV; } + sd_printk(KERN_ERR, sdkp, "Got past protection check\n"); if ((sizeof(sdkp->capacity) == 4) && (lba >= 0xffffffffULL)) { sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a " @@ -2018,8 +2025,10 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp, sd_printk(KERN_NOTICE, sdkp, "physical block alignment offset: %u\n", alignment); + sd_printk(KERN_NOTICE, sdkp, "Checking LBPME\n"); if (buffer[14] & 0x80) { /* LBPME */ sdkp->lbpme = 1; + sd_printk(KERN_NOTICE, sdkp, "LBPME OK!\n"); if (buffer[14] & 0x40) /* LBPRZ */ sdkp->lbprz = 1; @@ -2109,6 +2118,9 @@ static int sd_try_rc16_first(struct scsi_device *sdp) return 1; if (scsi_device_protection(sdp)) return 1; + sdev_printk(KERN_NOTICE, sdp, + "sd_try_rc16_first: sdp->scsi_level: %d\n", + sdp->scsi_level); return 0; } @@ -2542,24 +2554,29 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) const int vpd_len = 64; unsigned char *buffer = kmalloc(vpd_len, GFP_KERNEL); + sd_printk(KERN_NOTICE, sdkp, "Entered block limits\n"); if (!buffer || /* Block Limits VPD */ scsi_get_vpd_page(sdkp->device, 0xb0, buffer, vpd_len)) goto out; + sd_printk(KERN_NOTICE, sdkp, "Started block limits\n"); blk_queue_io_min(sdkp->disk->queue, get_unaligned_be16(&buffer[6]) * sector_sz); blk_queue_io_opt(sdkp->disk->queue, get_unaligned_be32(&buffer[12]) * sector_sz); + sd_printk(KERN_NOTICE, sdkp, "0x3c...\n"); if (buffer[3] == 0x3c) { unsigned int lba_count, desc_count; sdkp->max_ws_blocks = (u32)get_unaligned_be64(&buffer[36]); + sd_printk(KERN_NOTICE, sdkp, "Testing lbpme...\n"); if (!sdkp->lbpme) goto out; + sd_printk(KERN_NOTICE, sdkp, "...lbpme test done\n"); lba_count = get_unaligned_be32(&buffer[20]); desc_count = get_unaligned_be32(&buffer[24]); @@ -2574,6 +2591,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) if (!sdkp->lbpvpd) { /* LBP VPD page not provided */ + sd_printk(KERN_NOTICE, sdkp, "Entering discard switch with NO LBP VPD\n"); if (sdkp->max_unmap_blocks) sd_config_discard(sdkp, SD_LBP_UNMAP); else @@ -2581,6 +2599,7 @@ static void sd_read_block_limits(struct scsi_disk *sdkp) } else { /* LBP VPD page tells us what to use */ + sd_printk(KERN_NOTICE, sdkp, "Entering discard switch via LBP VPD\n"); if (sdkp->lbpu && sdkp->max_unmap_blocks) sd_config_discard(sdkp, SD_LBP_UNMAP); else if (sdkp->lbpws) @@ -2631,14 +2650,19 @@ static void sd_read_block_provisioning(struct scsi_disk *sdkp) unsigned char *buffer; const int vpd_len = 8; + sd_printk(KERN_NOTICE, sdkp, + "sd_read_block_provisioning: Entered, lbmpe: %u\n", + sdkp->lbpme); if (sdkp->lbpme == 0) return; + sd_printk(KERN_NOTICE, sdkp, "sd_read_block_provisioning: Passed lbmpe test\n"); buffer = kmalloc(vpd_len, GFP_KERNEL); if (!buffer || scsi_get_vpd_page(sdkp->device, 0xb2, buffer, vpd_len)) goto out; + sd_printk(KERN_NOTICE, sdkp, "sd_read_block_provisioning: Setting block provisioning\n"); sdkp->lbpvpd = 1; sdkp->lbpu = (buffer[5] >> 7) & 1; /* UNMAP */ sdkp->lbpws = (buffer[5] >> 6) & 1; /* WRITE SAME(16) with UNMAP */ @@ -2734,10 +2758,14 @@ static int sd_revalidate_disk(struct gendisk *disk) if (sdkp->media_present) { sd_read_capacity(sdkp, buffer); + sd_printk(KERN_NOTICE, sdkp, "sd_revalidate_disk: Extended inquiry check...\n"); if (sd_try_extended_inquiry(sdp)) { + sd_printk(KERN_NOTICE, sdkp, "sd_revalidate_disk: Performing extended inquiries\n"); sd_read_block_provisioning(sdkp); sd_read_block_limits(sdkp); sd_read_block_characteristics(sdkp); + } else { + sd_printk(KERN_NOTICE, sdkp, "sd_revalidate_disk: Skipped extended inquiries\n"); } sd_read_write_protect_flag(sdkp, buffer); diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 5ad2810..5f733c7 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1450,6 +1450,7 @@ static int storvsc_device_configure(struct scsi_device *sdevice) * With this patch we can correctly handle WRITE_SAME_16 issues. */ sdevice->sdev_bflags |= msft_blist_flags; + sdev_printk(KERN_NOTICE, sdevice, "storvsc_device_configure: Added BLIST_TRY_VPD_PAGES\n"); return 0; } -- 1.9.3 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 0/3] Enable discard on Hyper-V 2014-07-23 20:13 ` Sitsofe Wheeler @ 2014-07-24 7:47 ` Sitsofe Wheeler 2014-07-24 7:52 ` [PATCH 1/3] [SCSI] Add quirk for forcing logical block provisioning tests Sitsofe Wheeler ` (3 more replies) 0 siblings, 4 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 7:47 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 1/3] [SCSI] Add quirk for forcing logical block provisioning tests 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler @ 2014-07-24 7:52 ` Sitsofe Wheeler 2014-07-24 7:56 ` [PATCH 2/3] [SCSI] storvsc: Add Hyper-V " Sitsofe Wheeler ` (2 subsequent siblings) 3 siblings, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 7:52 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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 <kys@microsoft.com> Original-patch-by: Martin K. Petersen <martin.petersen@oracle.com> Signed-off-by: Sitsofe Wheeler <sitsofe@yahoo.com> --- 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 2/3] [SCSI] storvsc: Add Hyper-V logical block provisioning tests 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 2014-07-24 7:52 ` [PATCH 1/3] [SCSI] Add quirk for forcing logical block provisioning tests Sitsofe Wheeler @ 2014-07-24 7:56 ` Sitsofe Wheeler 2014-07-24 14:09 ` James Bottomley 2014-07-24 7:58 ` [PATCH 3/3] [SCSI] Make LBP quirk skip lbpme checks tests Sitsofe Wheeler 2014-07-24 12:37 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 3 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 7:56 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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 <kys@microsoft.com> Signed-off-by: Sitsofe Wheeler <sitsofe@yahoo.com> --- 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] [SCSI] storvsc: Add Hyper-V logical block provisioning tests 2014-07-24 7:56 ` [PATCH 2/3] [SCSI] storvsc: Add Hyper-V " Sitsofe Wheeler @ 2014-07-24 14:09 ` James Bottomley 2014-07-24 18:03 ` Sitsofe Wheeler 0 siblings, 1 reply; 43+ messages in thread From: James Bottomley @ 2014-07-24 14:09 UTC (permalink / raw) To: sitsofe Cc: linux-kernel, hch, devel, apw, kys, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2528 bytes --] 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 <kys@microsoft.com> > Signed-off-by: Sitsofe Wheeler <sitsofe@yahoo.com> > --- > 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 2/3] [SCSI] storvsc: Add Hyper-V logical block provisioning tests 2014-07-24 14:09 ` James Bottomley @ 2014-07-24 18:03 ` Sitsofe Wheeler 0 siblings, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 18:03 UTC (permalink / raw) To: James Bottomley Cc: linux-kernel, hch, devel, apw, kys, linux-scsi, ohering, gregkh, jasowang 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 2014-07-24 7:52 ` [PATCH 1/3] [SCSI] Add quirk for forcing logical block provisioning tests Sitsofe Wheeler 2014-07-24 7:56 ` [PATCH 2/3] [SCSI] storvsc: Add Hyper-V " Sitsofe Wheeler @ 2014-07-24 7:58 ` Sitsofe Wheeler 2014-07-24 12:22 ` [PATCH v2 " Sitsofe Wheeler 2014-07-24 12:37 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 3 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 7:58 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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 <sitsofe@yahoo.com> --- 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 7:58 ` [PATCH 3/3] [SCSI] Make LBP quirk skip lbpme checks tests Sitsofe Wheeler @ 2014-07-24 12:22 ` Sitsofe Wheeler 2014-07-24 13:54 ` Martin K. Petersen 0 siblings, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 12:22 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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 <sitsofe@yahoo.com> --- 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 ^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 12:22 ` [PATCH v2 " Sitsofe Wheeler @ 2014-07-24 13:54 ` Martin K. Petersen 2014-07-24 15:34 ` Christoph Hellwig 2014-07-24 15:36 ` Sitsofe Wheeler 0 siblings, 2 replies; 43+ messages in thread From: Martin K. Petersen @ 2014-07-24 13:54 UTC (permalink / raw) To: Sitsofe Wheeler Cc: Christoph Hellwig, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 13:54 ` Martin K. Petersen @ 2014-07-24 15:34 ` Christoph Hellwig 2014-07-24 15:35 ` Christoph Hellwig 2014-07-24 15:36 ` Sitsofe Wheeler 1 sibling, 1 reply; 43+ messages in thread From: Christoph Hellwig @ 2014-07-24 15:34 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 15:34 ` Christoph Hellwig @ 2014-07-24 15:35 ` Christoph Hellwig 2014-07-24 16:24 ` Sitsofe Wheeler 0 siblings, 1 reply; 43+ messages in thread From: Christoph Hellwig @ 2014-07-24 15:35 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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. ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 15:35 ` Christoph Hellwig @ 2014-07-24 16:24 ` Sitsofe Wheeler 0 siblings, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 16:24 UTC (permalink / raw) To: Christoph Hellwig Cc: Martin K. Petersen, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 13:54 ` Martin K. Petersen 2014-07-24 15:34 ` Christoph Hellwig @ 2014-07-24 15:36 ` Sitsofe Wheeler 2014-07-24 15:54 ` Martin K. Petersen 1 sibling, 1 reply; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 15:36 UTC (permalink / raw) To: Martin K. Petersen Cc: Christoph Hellwig, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi On Thu, Jul 24, 2014 at 09:54:24AM -0400, Martin K. Petersen wrote: > >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 15:36 ` Sitsofe Wheeler @ 2014-07-24 15:54 ` Martin K. Petersen 2014-07-25 16:47 ` KY Srinivasan 0 siblings, 1 reply; 43+ messages in thread From: Martin K. Petersen @ 2014-07-24 15:54 UTC (permalink / raw) To: Sitsofe Wheeler Cc: Martin K. Petersen, Christoph Hellwig, K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-24 15:54 ` Martin K. Petersen @ 2014-07-25 16:47 ` KY Srinivasan 2014-07-25 16:57 ` Martin K. Petersen 2014-07-25 17:10 ` James Bottomley 0 siblings, 2 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-25 16:47 UTC (permalink / raw) To: Martin K. Petersen, Sitsofe Wheeler Cc: Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Thursday, July 24, 2014 8:54 AM > To: Sitsofe Wheeler > Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-25 16:47 ` KY Srinivasan @ 2014-07-25 16:57 ` Martin K. Petersen 2014-07-26 13:44 ` KY Srinivasan 2014-07-25 17:10 ` James Bottomley 1 sibling, 1 reply; 43+ messages in thread From: Martin K. Petersen @ 2014-07-25 16:57 UTC (permalink / raw) To: KY Srinivasan Cc: Martin K. Petersen, Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-25 16:57 ` Martin K. Petersen @ 2014-07-26 13:44 ` KY Srinivasan 2014-07-26 16:54 ` Martin K. Petersen 0 siblings, 1 reply; 43+ messages in thread From: KY Srinivasan @ 2014-07-26 13:44 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Friday, July 25, 2014 9:57 AM > To: KY Srinivasan > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-26 13:44 ` KY Srinivasan @ 2014-07-26 16:54 ` Martin K. Petersen 2014-07-26 17:17 ` KY Srinivasan 0 siblings, 1 reply; 43+ messages in thread From: Martin K. Petersen @ 2014-07-26 16:54 UTC (permalink / raw) To: KY Srinivasan Cc: Martin K. Petersen, Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-26 16:54 ` Martin K. Petersen @ 2014-07-26 17:17 ` KY Srinivasan 2014-07-26 19:25 ` Martin K. Petersen 0 siblings, 1 reply; 43+ messages in thread From: KY Srinivasan @ 2014-07-26 17:17 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Saturday, July 26, 2014 9:55 AM > To: KY Srinivasan > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-26 17:17 ` KY Srinivasan @ 2014-07-26 19:25 ` Martin K. Petersen 2014-07-27 2:09 ` KY Srinivasan 2014-07-28 18:50 ` KY Srinivasan 0 siblings, 2 replies; 43+ messages in thread From: Martin K. Petersen @ 2014-07-26 19:25 UTC (permalink / raw) To: KY Srinivasan Cc: Martin K. Petersen, Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-26 19:25 ` Martin K. Petersen @ 2014-07-27 2:09 ` KY Srinivasan 2014-07-28 18:50 ` KY Srinivasan 1 sibling, 0 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-27 2:09 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Saturday, July 26, 2014 12:25 PM > To: KY Srinivasan > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-26 19:25 ` Martin K. Petersen 2014-07-27 2:09 ` KY Srinivasan @ 2014-07-28 18:50 ` KY Srinivasan 2014-07-28 19:02 ` Martin K. Petersen 1 sibling, 1 reply; 43+ messages in thread From: KY Srinivasan @ 2014-07-28 18:50 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Saturday, July 26, 2014 12:25 PM > To: KY Srinivasan > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-28 18:50 ` KY Srinivasan @ 2014-07-28 19:02 ` Martin K. Petersen 2014-07-28 19:05 ` KY Srinivasan 0 siblings, 1 reply; 43+ messages in thread From: Martin K. Petersen @ 2014-07-28 19:02 UTC (permalink / raw) To: KY Srinivasan Cc: Martin K. Petersen, Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-28 19:02 ` Martin K. Petersen @ 2014-07-28 19:05 ` KY Srinivasan 2014-07-28 20:02 ` James Bottomley 0 siblings, 1 reply; 43+ messages in thread From: KY Srinivasan @ 2014-07-28 19:05 UTC (permalink / raw) To: Martin K. Petersen Cc: Sitsofe Wheeler, Christoph Hellwig, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi > -----Original Message----- > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > Sent: Monday, July 28, 2014 12:03 PM > To: KY Srinivasan > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > jasowang@redhat.com; jbottomley@parallels.com; linux- > scsi@vger.kernel.org > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-28 19:05 ` KY Srinivasan @ 2014-07-28 20:02 ` James Bottomley 2014-07-28 20:05 ` KY Srinivasan 2014-07-29 17:41 ` KY Srinivasan 0 siblings, 2 replies; 43+ messages in thread From: James Bottomley @ 2014-07-28 20:02 UTC (permalink / raw) To: kys Cc: linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1753 bytes --] On Mon, 2014-07-28 at 19:05 +0000, KY Srinivasan wrote: > > > -----Original Message----- > > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > > Sent: Monday, July 28, 2014 12:03 PM > > To: KY Srinivasan > > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > > jasowang@redhat.com; jbottomley@parallels.com; linux- > > scsi@vger.kernel.org > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-28 20:02 ` James Bottomley @ 2014-07-28 20:05 ` KY Srinivasan 2014-07-29 17:41 ` KY Srinivasan 1 sibling, 0 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-28 20:05 UTC (permalink / raw) To: James Bottomley Cc: linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2370 bytes --] > -----Original Message----- > From: James Bottomley [mailto:jbottomley@parallels.com] > Sent: Monday, July 28, 2014 1:03 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; hch@infradead.org; sitsofe@gmail.com; > devel@linuxdriverproject.org; apw@canonical.com; > martin.petersen@oracle.com; linux-scsi@vger.kernel.org; > ohering@suse.com; gregkh@linuxfoundation.org; jasowang@redhat.com > 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:martin.petersen@oracle.com] > > > Sent: Monday, July 28, 2014 12:03 PM > > > To: KY Srinivasan > > > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > > > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > > > jasowang@redhat.com; jbottomley@parallels.com; linux- > > > scsi@vger.kernel.org > > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks > > > tests > > > > > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-28 20:02 ` James Bottomley 2014-07-28 20:05 ` KY Srinivasan @ 2014-07-29 17:41 ` KY Srinivasan 2014-07-29 19:30 ` Martin K. Petersen 1 sibling, 1 reply; 43+ messages in thread From: KY Srinivasan @ 2014-07-29 17:41 UTC (permalink / raw) To: James Bottomley Cc: linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2445 bytes --] > -----Original Message----- > From: James Bottomley [mailto:jbottomley@parallels.com] > Sent: Monday, July 28, 2014 1:03 PM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; hch@infradead.org; sitsofe@gmail.com; > devel@linuxdriverproject.org; apw@canonical.com; > martin.petersen@oracle.com; linux-scsi@vger.kernel.org; > ohering@suse.com; gregkh@linuxfoundation.org; jasowang@redhat.com > 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:martin.petersen@oracle.com] > > > Sent: Monday, July 28, 2014 12:03 PM > > > To: KY Srinivasan > > > Cc: Martin K. Petersen; Sitsofe Wheeler; Christoph Hellwig; > > > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > > > jasowang@redhat.com; jbottomley@parallels.com; linux- > > > scsi@vger.kernel.org > > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks > > > tests > > > > > > >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-29 17:41 ` KY Srinivasan @ 2014-07-29 19:30 ` Martin K. Petersen 0 siblings, 0 replies; 43+ messages in thread From: Martin K. Petersen @ 2014-07-29 19:30 UTC (permalink / raw) To: KY Srinivasan Cc: James Bottomley, linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang >>>>> "KY" == KY Srinivasan <kys@microsoft.com> 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 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-25 16:47 ` KY Srinivasan 2014-07-25 16:57 ` Martin K. Petersen @ 2014-07-25 17:10 ` James Bottomley 2014-07-26 13:42 ` KY Srinivasan 1 sibling, 1 reply; 43+ messages in thread From: James Bottomley @ 2014-07-25 17:10 UTC (permalink / raw) To: kys Cc: linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3093 bytes --] On Fri, 2014-07-25 at 16:47 +0000, KY Srinivasan wrote: > > > -----Original Message----- > > From: Martin K. Petersen [mailto:martin.petersen@oracle.com] > > Sent: Thursday, July 24, 2014 8:54 AM > > To: Sitsofe Wheeler > > Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan; > > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > > jasowang@redhat.com; jbottomley@parallels.com; linux- > > scsi@vger.kernel.org > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests > > > > >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* RE: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks tests 2014-07-25 17:10 ` James Bottomley @ 2014-07-26 13:42 ` KY Srinivasan 0 siblings, 0 replies; 43+ messages in thread From: KY Srinivasan @ 2014-07-26 13:42 UTC (permalink / raw) To: James Bottomley Cc: linux-kernel, hch, sitsofe, devel, apw, martin.petersen, linux-scsi, ohering, gregkh, jasowang [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3902 bytes --] > -----Original Message----- > From: James Bottomley [mailto:jbottomley@parallels.com] > Sent: Friday, July 25, 2014 10:10 AM > To: KY Srinivasan > Cc: linux-kernel@vger.kernel.org; hch@infradead.org; sitsofe@gmail.com; > devel@linuxdriverproject.org; apw@canonical.com; > martin.petersen@oracle.com; linux-scsi@vger.kernel.org; > ohering@suse.com; gregkh@linuxfoundation.org; jasowang@redhat.com > 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:martin.petersen@oracle.com] > > > Sent: Thursday, July 24, 2014 8:54 AM > > > To: Sitsofe Wheeler > > > Cc: Martin K. Petersen; Christoph Hellwig; KY Srinivasan; > > > gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; > > > devel@linuxdriverproject.org; ohering@suse.com; apw@canonical.com; > > > jasowang@redhat.com; jbottomley@parallels.com; linux- > > > scsi@vger.kernel.org > > > Subject: Re: [PATCH v2 3/3] [SCSI] Make LBP quirk skip lbpme checks > > > tests > > > > > > >>>>> "Sitsofe" == Sitsofe Wheeler <sitsofe@gmail.com> 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Ç+·®+%Ëÿ±éݶ\x17¥wÿº{.nÇ+·¥{±þG«éÿ{ayº\x1dÊÚë,j\a¢f£¢·hïêÿêçz_è®\x03(éÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?¨èÚ&£ø§~á¶iOæ¬z·vØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?I¥ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 0/3] Enable discard on Hyper-V 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler ` (2 preceding siblings ...) 2014-07-24 7:58 ` [PATCH 3/3] [SCSI] Make LBP quirk skip lbpme checks tests Sitsofe Wheeler @ 2014-07-24 12:37 ` Sitsofe Wheeler 3 siblings, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-07-24 12:37 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi 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/ ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-21 23:06 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan 2014-07-23 10:04 ` Sitsofe Wheeler 2014-07-23 14:10 ` Sitsofe Wheeler @ 2014-07-24 5:40 ` Hannes Reinecke 2014-08-01 19:48 ` Sitsofe Wheeler 2 siblings, 1 reply; 43+ messages in thread From: Hannes Reinecke @ 2014-07-24 5:40 UTC (permalink / raw) To: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, hch, linux-scsi 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 <kys@microsoft.com> > --- > 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 <hare@suse.de> Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@suse.de +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags 2014-07-24 5:40 ` [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags Hannes Reinecke @ 2014-08-01 19:48 ` Sitsofe Wheeler 0 siblings, 0 replies; 43+ messages in thread From: Sitsofe Wheeler @ 2014-08-01 19:48 UTC (permalink / raw) To: Christoph Hellwig Cc: K. Y. Srinivasan, gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley, linux-scsi, Hannes Reinecke On Thu, Jul 24, 2014 at 07:40:36AM +0200, Hannes Reinecke wrote: > 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 <kys@microsoft.com> > > > Reviewed-by: Hannes Reinecke <hare@suse.de> 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 > https://patchwork.kernel.org/patch/4541201 (which needed a small fixup) and > https://patchwork.kernel.org/patch/4598601 and the attached debugging output I've tested f3cfabce7a2e92564d380de3aad4b43901fb7ae6 (Drivers: add blist flags) from the drivers-for-3.17 branch of scsi-queue (http://git.infradead.org/users/hch/scsi-queue.git/commit/f3cfabce7a2e92564d380de3aad4b43901fb7ae6 ) and this patch still doesn't appear to work (thin provisioning on Hyper-V's Virtual Disks is not enabled): # sg_inq /dev/sda standard INQUIRY: PQual=0 Device_type=0 RMB=0 version=0x04 [SPC-2] [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 length=36 (0x24) Peripheral device type: disk Vendor identification: Msft Product identification: Virtual Disk Product revision level: 1.0 # sg_vpd -p lbpv /dev/sda 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): 0 Threshold exponent: 1 Descriptor present (DP): 0 Provisioning type: 2 grep . /sys/block/sd*/device/scsi_disk/*/{thin,prov,rotat,device/model}* | sort /sys/block/sda/device/scsi_disk/0:0:0:0/device/model:Virtual Disk /sys/block/sda/device/scsi_disk/0:0:0:0/provisioning_mode:full /sys/block/sda/device/scsi_disk/0:0:0:0/thin_provisioning:0 /sys/block/sdb/device/scsi_disk/1:0:0:0/device/model:Virtual Disk /sys/block/sdb/device/scsi_disk/1:0:0:0/provisioning_mode:full /sys/block/sdb/device/scsi_disk/1:0:0:0/thin_provisioning:0 /sys/block/sdc/device/scsi_disk/1:0:0:2/device/model:00AAKS-00TMA0 /sys/block/sdc/device/scsi_disk/1:0:0:2/provisioning_mode:full /sys/block/sdc/device/scsi_disk/1:0:0:2/thin_provisioning:0 /sys/block/sdd/device/scsi_disk/1:0:0:1/device/model:SSD S510 120GB /sys/block/sdd/device/scsi_disk/1:0:0:1/provisioning_mode:full /sys/block/sdd/device/scsi_disk/1:0:0:1/thin_provisioning:0 /sys/block/sde/device/scsi_disk/1:0:0:3/device/model:Virtual Disk /sys/block/sde/device/scsi_disk/1:0:0:3/provisioning_mode:full /sys/block/sde/device/scsi_disk/1:0:0:3/thin_provisioning:0 /sys/block/sdf/device/scsi_disk/1:0:0:4/device/model:ST1000DM003-9YN1 /sys/block/sdf/device/scsi_disk/1:0:0:4/provisioning_mode:full /sys/block/sdf/device/scsi_disk/1:0:0:4/thin_provisioning:0 At least the virtual disks should have thin_provisioning... I was also expecting the passthrough SSD to be reported as non-rotational with this patch: # sg_inq /dev/sdd 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 # sg_vpd -p bdc /dev/sdd Block device characteristics VPD page (SBC): Non-rotating medium (e.g. solid state) Product type: Not specified WABEREQ=0 WACEREQ=0 Nominal form factor not reported FUAB=0 VBULS=0 # grep -H . /sys/block/sdd/queue/rot* /sys/block/sdd/queue/rotational:1 -- Sitsofe | http://sucs.org/~sits/ ^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2014-08-01 19:49 UTC | newest] Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-07-21 23:06 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan 2014-07-23 10:04 ` Sitsofe Wheeler 2014-07-23 11:51 ` Christoph Hellwig 2014-07-23 12:54 ` Sitsofe Wheeler 2014-07-23 14:10 ` Christoph Hellwig 2014-07-23 15:31 ` Sitsofe Wheeler 2014-07-23 15:40 ` KY Srinivasan 2014-07-23 15:39 ` KY Srinivasan 2014-07-23 14:10 ` Sitsofe Wheeler 2014-07-23 14:15 ` Christoph Hellwig 2014-07-23 20:13 ` Sitsofe Wheeler 2014-07-24 7:47 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 2014-07-24 7:52 ` [PATCH 1/3] [SCSI] Add quirk for forcing logical block provisioning tests Sitsofe Wheeler 2014-07-24 7:56 ` [PATCH 2/3] [SCSI] storvsc: Add Hyper-V " Sitsofe Wheeler 2014-07-24 14:09 ` James Bottomley 2014-07-24 18:03 ` Sitsofe Wheeler 2014-07-24 7:58 ` [PATCH 3/3] [SCSI] Make LBP quirk skip lbpme checks tests Sitsofe Wheeler 2014-07-24 12:22 ` [PATCH v2 " Sitsofe Wheeler 2014-07-24 13:54 ` Martin K. Petersen 2014-07-24 15:34 ` Christoph Hellwig 2014-07-24 15:35 ` Christoph Hellwig 2014-07-24 16:24 ` Sitsofe Wheeler 2014-07-24 15:36 ` Sitsofe Wheeler 2014-07-24 15:54 ` Martin K. Petersen 2014-07-25 16:47 ` KY Srinivasan 2014-07-25 16:57 ` Martin K. Petersen 2014-07-26 13:44 ` KY Srinivasan 2014-07-26 16:54 ` Martin K. Petersen 2014-07-26 17:17 ` KY Srinivasan 2014-07-26 19:25 ` Martin K. Petersen 2014-07-27 2:09 ` KY Srinivasan 2014-07-28 18:50 ` KY Srinivasan 2014-07-28 19:02 ` Martin K. Petersen 2014-07-28 19:05 ` KY Srinivasan 2014-07-28 20:02 ` James Bottomley 2014-07-28 20:05 ` KY Srinivasan 2014-07-29 17:41 ` KY Srinivasan 2014-07-29 19:30 ` Martin K. Petersen 2014-07-25 17:10 ` James Bottomley 2014-07-26 13:42 ` KY Srinivasan 2014-07-24 12:37 ` [PATCH 0/3] Enable discard on Hyper-V Sitsofe Wheeler 2014-07-24 5:40 ` [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags Hannes Reinecke 2014-08-01 19:48 ` Sitsofe Wheeler
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).