Re: [PATCH 2/2] xfs: add 'discard_sync' mount flag

[Jens Axboe <axboe@kernel.dk>]

On 4/30/18 1:57 PM, Eric Sandeen wrote:
> 
> 
> On 4/30/18 2:21 PM, Jens Axboe wrote:
>> On 4/30/18 1:19 PM, Eric Sandeen wrote:
>>>
>>>
>>> On 4/30/18 1:25 PM, Luis R. Rodriguez wrote:
>>>> On Mon, Apr 30, 2018 at 12:07:31PM -0600, Jens Axboe wrote:
>>>>> On 4/30/18 11:19 AM, Brian Foster wrote:
>>>>>> On Mon, Apr 30, 2018 at 09:32:52AM -0600, Jens Axboe wrote:
>>>>>>> XFS recently added support for async discards. While this can be
>>>>>>> a win for some workloads and devices, there are also cases where
>>>>>>> async bursty discard will severly harm the latencies of reads
>>>>>>> and writes.
>>>>>>>
>>>>>>> Add a 'discard_sync' mount flag to revert to using sync discard,
>>>>>>> issuing them one at the time and waiting for each one. This fixes
>>>>>>> a big performance regression we had moving to kernels that include
>>>>>>> the XFS async discard support.
>>>>>>>
>>>>>>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>>>>>>> ---
>>>>>>
>>>>>> Hm, I figured the async discard stuff would have been a pretty clear win
>>>>>> all around, but then again I'm not terribly familiar with what happens
>>>>>> with discards beneath the fs. I do know that the previous behavior would
>>>>>> cause fs level latencies due to holding up log I/O completion while
>>>>>> discards completed one at a time. My understanding is that this lead to
>>>>>> online discard being pretty much universally "not recommended" in favor
>>>>>> of fstrim.
>>>>>
>>>>> It's not a secret that most devices suck at discard.
>>>>
>>>> How can we know if a device sucks at discard?
>>>
>>> I was going to ask the same thing.  ;)  "Meh, punt to the admin!"
>>>
>>> I'm having deja vu but can't remember why.  Seems like this has come up
>>> before and we thought it should be a block device tunable, not pushed down
>>> from the filesystem.  Is that possible?
>>
>> The problem is that it'll depend on the workload as well. The device in
>> may laptop is fine with discard for my workload, which is very light.
>> But if you are running RocksDB on it, and doing heavy compactions and
>> deletes, it probably would not be.
> 
> Ok, but I'm not sure how that precludes a block device tunable?  You'd tune
> it for your workload, right?

What kind of tunable are you thinking of? Right now we have one tunable,
which is the max discard size. Patch #1 actually helps make this do what
it should for sync discards, instead of just building a bio chain and
submitting that all at once.

> Or is the concern that it could only be for the entire block device, and
> perhaps different partitions have different workloads?
> 
> Sorry, caveman filesystem guy doesn't completely understand block devices.

I just don't know what you are trying to tune :-)

-- 
Jens Axboe