Re: [PATCH 3.16 106/217] sd: disable discard_zeroes_data for UNMAP

[Rafael David Tinoco]

Martin,

On Fri, Apr 29, 2016 at 9:16 AM, Martin K. Petersen
<martin.petersen@xxxxxxxxxx> wrote:
>>>>>> "Rafael" == Rafael David Tinoco <rafael.tinoco@xxxxxxxxxxxxx> 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