Re: [6.2 regression][bisected]discard storm on idle since v6.1-rc8-59-g63a7cb130718 discard=async

[Boris Burkov <boris@bur.io>]

On Tue, Apr 04, 2023 at 09:22:05PM +0200, David Sterba wrote:
> On Tue, Apr 04, 2023 at 11:51:51AM -0700, Boris Burkov wrote:
> > Our reasonable options, as I see them:
> > - back to nodiscard, rely on periodic trims from the OS.
> 
> We had that before and it's a fallback in case we can't fix it but still
> the problem would persist for anyone enabling async discard so it's only
> limiting the impact.
> 
> > - leave low iops_limit, drives stay busy unexpectedly long, conclude that
> >   that's OK, and communicate the tuning/measurement options better.
> 
> This does not sound user friendly, tuning should be possible but not
> required by default. We already have enough other things that users need
> to decide and in this case I don't know if there's enough information to
> even make a good decision upfront.
> 
> > - set a high iops_limit (e.g. 1000) drives will get back to idle faster.
> > - change an unset iops_limit to mean truly unlimited async discard, set
> >   that as the default, and anyone who cares to meter it can set an
> >   iops_limit.
> > 
> > The regression here is in drive idle time due to modest discard getting
> > metered out over minutes rather than dealt with relatively quickly. So
> > I would favor the unlimited async discard mode and will send a patch to
> > that effect which we can discuss.
> 
> Can we do options 3 and 4, i.e. set a high iops so that the batch gets
> processed faster and (4) that there's the manual override to drop the
> limit completely?

Yup, I'm on it.

Chris also had a good idea for a "time since commit" check to further
limit how long async discard would crank for.

This would look something like "go as fast as I'm allowed for 5s after
every transaction". It could lead to a big buildup that requires a
periodic trim to clear.

I'm playing with adding that as an additional tunable which would apply
if set.

Boris