Re: [PATCH 0/4] fixes for btrfs async discards

[Pavel Begunkov <asml.silence@gmail.com>]

On 05/11/2020 22:23, David Sterba wrote:
> On Wed, Nov 04, 2020 at 09:45:50AM +0000, Pavel Begunkov wrote:
>> Several fixes for async discards. The first patch might increase discard
>> rate, drastically in some cases. That may be a suprise for those
>> assuming that hitting iops_limit is rare and rarther outliers. Though,
>> it still stays in allowed range, so should be fine.
> 
> I think this highly depends on the workload, if you really need to issue
> the discards fast because of the rate of the change in the regular data.
> That was the point of the async discard and the knobs, the defaults
> should be ok for most users and allow adjusting for specific loads.

Chris mentioned that _there are_ problems with faster drives though.
The problem is that this iops_limit knot just clamps the chosen delay.
Ultimately, I want to find later a better delay function than

delay = CONSTANT_INTERVAL_MS / nr_extents.

But that will take some thinking.

For instance, one of the cases I've seen is recycling large extents
like deletion of subvolumes. There we have a small number of extents
but each takes a lot of space, so there are, say, 100+GB queued to be
discarded. But because there are few extents, delay is calculated
to >10s that's then clamped to a constant max limit.
That was taking a long to recycle. Not sure though how many bytes/extents
are discarded on each iteration of btrfs_discard_workfn().

> 
> My testing of the original discard patchset was tailored towards the
> default usecase and likely intense than yours. I did not observe the
> corner cases where the work queue scheduling was off, or changing the
> sysfs values did not poke the right kthreads.
> 
> Patches look ok to me and I've added them to topic branch for testing,
> going to misc-next later. Thanks.
> 

-- 
Pavel Begunkov