> -----Original Message-----
> From: h...@infradead.org [mailto:h...@infradead.org]
> Sent: Thursday, July 10, 2014 11:32 PM
> To: James Bottomley
> Cc: KY Srinivasan; linux-kernel@vger.kernel.org; m...@mkp.net;
> h...@infradead.org; de...@linuxdriverproject.org; a...@canonical.com;
> sta...@vger.kernel.org; linux-s...@vger.kernel.org; oher...@suse.com;
> jasow...@redhat.com
> Subject: Re: [PATCH 4/8] Drivers: scsi: storvsc: Filter WRITE_SAME_16
> 
> On Wed, Jul 09, 2014 at 10:27:24PM +0000, James Bottomley wrote:
> > If we fix it at source, why would there be any need to filter?  That's
> > the reason the no_write_same flag was introduced.  If we can find and
> > fix the bug, it can go back into the stable trees as a bug fix, hence
> > nothing should ever emit write_same(10 or 16) and additional driver
> > code is redundant (and counter productive, since if this ever breaks
> > again you're our best canary).
> >
> > This looks like it might be the problem but Martin should confirm (I
> > think the problem comes to us from the RC16 code which unconditionally
> > sets WS16).
> 
> I think the problem is a differnet one.  If we have the logical provisioning
> EVPD it configures what method to use, but if we don't have one we simply
> check for a max unmap blocks field, and if that's not present use WRITE
> SAME.
> 
> The patch checks the no_write_same flag before doing that, for which we
> also have to do the write_same setup before the discard setup in
> sd_revalidate_disk.
> 
> Ky: does hyperv support UNMAP?  If so any idea why it doesn't set the
> maximum unmap block count field in the EVPD?

Windows hosts do support UNMAP and set the field in the EVPD. However, since 
the host
advertises SPC-2 compliance, Linux does not even query the VPD page.
 
> 
> If we want to enable UNMAP in this case I'd prefer a blacklist entry than
> trying UNMAP despite the device not advertising it.
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c index ba756b1..fbccfd2 
> 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -2614,9 +2614,10 @@ static void sd_read_block_limits(struct scsi_disk
> *sdkp)
> 
>                       if (sdkp->max_unmap_blocks)
>                               sd_config_discard(sdkp, SD_LBP_UNMAP);
> -                     else
> +                     else if (!sdkp->device->no_write_same)
>                               sd_config_discard(sdkp, SD_LBP_WS16);
> -
> +                     else
> +                             sd_config_discard(sdkp, SD_LBP_DISABLE);
>               } else {        /* LBP VPD page tells us what to use */
> 
>                       if (sdkp->lbpu && sdkp->max_unmap_blocks) @@ -
> 2766,6 +2767,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>        */
>       if (sdkp->media_present) {
>               sd_read_capacity(sdkp, buffer);
> +             sd_read_write_same(sdkp, buffer);
> 
>               if (sd_try_extended_inquiry(sdp)) {
>                       sd_read_block_provisioning(sdkp);
> @@ -2776,7 +2778,6 @@ static int sd_revalidate_disk(struct gendisk *disk)
>               sd_read_write_protect_flag(sdkp, buffer);
>               sd_read_cache_type(sdkp, buffer);
>               sd_read_app_tag_own(sdkp, buffer);
> -             sd_read_write_same(sdkp, buffer);
>       }
> 
>       sdkp->first_scan = 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to