linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
@ 2014-07-21  3:33 K. Y. Srinivasan
  2014-07-21 11:27 ` Christoph Hellwig
  0 siblings, 1 reply; 16+ messages in thread
From: K. Y. Srinivasan @ 2014-07-21  3:33 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 |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 29d0329..2a58dc4 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -42,6 +42,7 @@
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_devinfo.h>
 #include <scsi/scsi_dbg.h>
+#include "scsi_priv.h"
 
 /*
  * All wire protocol details (storage protocol between the guest and the host)
@@ -327,6 +328,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);
@@ -1438,6 +1441,8 @@ static void storvsc_device_destroy(struct scsi_device *sdevice)
 
 static int storvsc_device_configure(struct scsi_device *sdevice)
 {
+	int blist_flags;
+
 	scsi_adjust_queue_depth(sdevice, MSG_SIMPLE_TAG,
 				STORVSC_MAX_IO_REQUESTS);
 
@@ -1449,6 +1454,14 @@ static int storvsc_device_configure(struct scsi_device *sdevice)
 
 	sdevice->no_write_same = 1;
 
+	blist_flags = scsi_get_device_flags_keyed(sdevice, "Msft",
+						  "Virtual Disk",
+						  SCSI_DEVINFO_GLOBAL);
+
+	if (blist_flags != msft_blist_flags)
+		scsi_dev_info_list_add_keyed(1, "Msft", "Virtual Disk", NULL,
+					     msft_blist_flags,
+					     SCSI_DEVINFO_GLOBAL);
 	return 0;
 }
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-21  3:33 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan
@ 2014-07-21 11:27 ` Christoph Hellwig
  2014-07-21 13:19   ` KY Srinivasan
  0 siblings, 1 reply; 16+ messages in thread
From: Christoph Hellwig @ 2014-07-21 11:27 UTC (permalink / raw)
  To: K. Y. Srinivasan
  Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley,
	hch, linux-scsi

On Sun, Jul 20, 2014 at 08:33:42PM -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.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

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;


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-21 11:27 ` Christoph Hellwig
@ 2014-07-21 13:19   ` KY Srinivasan
  0 siblings, 0 replies; 16+ messages in thread
From: KY Srinivasan @ 2014-07-21 13:19 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: gregkh, linux-kernel, devel, ohering, apw, jasowang, jbottomley,
	linux-scsi



> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Monday, July 21, 2014 4:27 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 Sun, Jul 20, 2014 at 08:33:42PM -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.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> 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;

Thanks Christoph. We can go with this. I will re-send the patch.


K. Y

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-24  5:40 ` Hannes Reinecke
@ 2014-08-01 19:48   ` Sitsofe Wheeler
  0 siblings, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-21 23:06 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; 16+ 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] 16+ 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
  0 siblings, 0 replies; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ messages in thread

* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-21 23:06 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 ` Hannes Reinecke
  2 siblings, 1 reply; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ 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; 16+ 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] 16+ messages in thread

* Re: [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags
  2014-07-21 23:06 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 ` Hannes Reinecke
  2 siblings, 2 replies; 16+ 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] 16+ messages in thread

* [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; 16+ 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] 16+ messages in thread

end of thread, other threads:[~2014-08-01 19:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21  3:33 [PATCH 1/1] Drivers: scsi: storvsc: Add blist flags K. Y. Srinivasan
2014-07-21 11:27 ` Christoph Hellwig
2014-07-21 13:19   ` KY Srinivasan
2014-07-21 23:06 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  5:40 ` 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).