Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759709AbaGXSDr (ORCPT ); Thu, 24 Jul 2014 14:03:47 -0400 Received: from mail-wi0-f182.google.com ([209.85.212.182]:56673 "EHLO mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757637AbaGXSDp (ORCPT ); Thu, 24 Jul 2014 14:03:45 -0400 Date: Thu, 24 Jul 2014 19:03:35 +0100 From: Sitsofe Wheeler To: James Bottomley Cc: "linux-kernel@vger.kernel.org" , "hch@infradead.org" , "devel@linuxdriverproject.org" , "apw@canonical.com" , "kys@microsoft.com" , "linux-scsi@vger.kernel.org" , "ohering@suse.com" , "gregkh@linuxfoundation.org" , "jasowang@redhat.com" Subject: Re: [PATCH 2/3] [SCSI] storvsc: Add Hyper-V logical block provisioning tests Message-ID: <20140724180335.GA13999@sucs.org> References: <1405983961-18782-1-git-send-email-kys@microsoft.com> <20140723141028.GA22724@sucs.org> <20140723141558.GA9705@infradead.org> <20140723201341.GA10292@sucs.org> <20140724074739.GA15127@sucs.org> <20140724075653.GC15127@sucs.org> <1406210951.2040.5.camel@jarvis.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1406210951.2040.5.camel@jarvis.lan> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/