Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753889AbcD2PPS (ORCPT ); Fri, 29 Apr 2016 11:15:18 -0400 Received: from mail-ob0-f180.google.com ([209.85.214.180]:33294 "EHLO mail-ob0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752964AbcD2PPQ (ORCPT ); Fri, 29 Apr 2016 11:15:16 -0400 MIME-Version: 1.0 In-Reply-To: References: <1461859893.1169.49.camel@decadent.org.uk> From: Rafael David Tinoco Date: Fri, 29 Apr 2016 12:14:46 -0300 Message-ID: Subject: Re: [PATCH 3.16 106/217] sd: disable discard_zeroes_data for UNMAP To: "Martin K. Petersen" Cc: Ben Hutchings , linux-kernel@vger.kernel.org, stable@vger.kernel.org, akpm@linux-foundation.org, Paolo Bonzini , Christoph Hellwig Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2275 Lines: 59 Martin, On Fri, Apr 29, 2016 at 9:16 AM, Martin K. Petersen wrote: >>>>>> "Rafael" == Rafael David Tinoco writes: > > Rafael, > > Rafael> And this happened because the storage in question didn't set > Rafael> properly "max_ws_blocks" (it was 0) from VPD 0xb0 (max_ws_blocks > Rafael> is calculated using it). > > We appear to be talking about a device that has an internal limit but > does not advertise it in the dedicated field. Exactly, Just quoted it as example of things that can brake. For this case, fixing VPD's max_ws_blocks fixed the problem because of: * IF max_ws_blocks > 0xffff max_ws_blocks = min (max_ws_blocks, 0x7fffff) No need for a patch. > NDOB is a recent performance optimization for high performance SSD > devices that allows us to skip sending the zeroed data buffer. However, > there is no way to tell whether a device supports NDOB without sending a > WRITE SAME command which has the nasty side effect of being destructive. > I am also not aware of any devices that actually support it yet. Yep, no way to inquiry it, unfortunately. I'll let the vendor that approached me to speak for themselves about their NDOB support, if they're willing to. They faced performance issues with WRITESAME(), but, there is no free lunch, i'm afraid, to guarantee subsequent READs are good. > As I mentioned earlier, I have some changes in the pipeline that will > separate the ioctl implementation from the library functions. That will > allow us to use WRITE SAME w/ UNMAP for zeroouts and UNMAP for discards > on the same device. However, this is still not going to solve the > problem with your device that fails WRITE SAME w/ UNMAP. Because we are > not going to discontinue using that combination. Quite the contrary, we > are increasingly depending on it. Definitely. > Consequently, if you have a device that has problems in that area we > need to quirk it rather than try to work around it in the core code > heuristics. Sure, my objection was because I could see 2 "storage servers" implementation problems that appeared with this change, but I do see your point on data guarantees. Thank you very much for clarifying this. -- Rafael Tinoco Canonical Sustaining Engineering