Re: [Lsf-pc] [LSF/MM/BPF TOPIC] Reclamation interactions with RCU

NeilBrown
2024-03-04T01:17:05Z

On Mon, 04 Mar 2024, Dave Chinner wrote:
> On Mon, Mar 04, 2024 at 09:45:48AM +1100, NeilBrown wrote:
> > I have in mind a more explicit statement of how much waiting is
> > acceptable.
> >
> > GFP_NOFAIL - wait indefinitely
>
> Make this the default, and we don't need a flag for it at all.

These aren't meant to be flags - those start with __.
They are .. "flag combinations" to use the term description from
gfp_types.h

There could only be a "default" if we used macro magic to allow the GFP_
argument to be omitted.

>
> > GFP_KILLABLE - wait indefinitely unless fatal signal is pending.
> > GFP_RETRY - may retry but deadlock, though unlikely, is possible. So
> > don't wait indefinitely. May abort more quickly if fatal
> > signal is pending.
>
> KILLABLE and RETRY are the same thing from the caller POV.
> Effectively "GFP_MAY_FAIL", where it will try really hard, but if it
> there is a risk of deadlock or a fatal signal pending, it will fail.
>
> > GFP_NO_RETRY - only try things once. This may sleep, but will give up
> > fairly quickly. Either deadlock is a significant
> > possibility, or alternate strategy is fairly cheap.
> > GFP_ATOMIC - don't sleep - same as current.
>
> We're talking about wait semantics, so GFP_ATOMIC should be named
> GFP_NO_WAIT and described as "same as NO_RETRY but will not sleep".
>
> That gives us three modifying flags to the default behaviour of
> sleeping until success: GFP_MAY_FAIL, GFP_NO_RETRY and GFP_NO_WAIT.

We currently have both __GFP_NORETRY and __GFP_RETRY_MAYFAIL which
differ in how many retries (1 or a few) and are both used (the former
about twice as much as the latter).
Do we need both?

Commit dcda9b04713c ("mm, tree wide: replace __GFP_REPEAT by __GFP_RETRY_MAYFAIL with more useful semantic")

might be useful in understanding the RETRY_MAYFAIL semantic.

I think the intent is that RETRY_MAYFAIL doesn't trigger the oom killer.
That seems like it could be a useful distinction.

GFP_NOFAIL - retry indefinitely
GFP_NOOOM - retry until fatal signal or OOM condition
GFP_NORETRY - sleep if needed, but don't retry
GFP_NOSLEEP - AKA GFP_ATOMIC

We might need a better name than GFP_NOOOM :-)

Thanks,
NeilBrown

>
> I will note there is one more case callers might really want to
> avoid: direct reclaim. That sort of behaviour might be worth folding
> into GFP_NO_WAIT, as there are cases where we want the allocation
> attempt to fail without trying to reclaim memory because it's *much*
> faster to simply use the fallback mechanism than it is to attempt
> memory reclaim (e.g. xlog_kvmalloc()).
>
> > I don't see how "GFP_KERNEL" fits into that spectrum.
>
> Agreed.
>
> > The definition of
> > "this will try really hard, but might fail and we can't really tell you
> > what circumstances it might fail in" isn't fun to work with.
>
> Yup, XFS was designed for NO_FAIL and MAY_FAIL behaviour, and in more
> recent times we also use NO_RECLAIM to provide our own kvmalloc
> semantics because the current kvmalloc API really only supports
> "GFP_KERNEL" allocation.
>
> > > Deprecating GFP_NOFS and GFP_NOIO would be wonderful - those should
> > > really just be PF_MEMALLOC_NOFS and PF_MEMALLOC_NOIO, now that we're
> > > pushing for memalloc_flags_(save|restore) more.
>
> This is largely now subsystem maintenance work - the infrastructure
> is there, and some subsystems have been converted over entirely to
> use it. The remaining work either needs to be mandated or have
> someone explicitly tasked with completing that work.
>
> IOWs, the process in which we change APIs and then leave the long
> tail of conversions to subsystem maintainers is just a mechanism for
> creating technical debt that takes forever to clean up...
>
> > > Getting rid of those would be a really nice cleanup beacuse then gfp
> > > flags would mostly just be:
> > > - the type of memory to allocate (highmem, zeroed, etc.)
> > > - how hard to try (don't block at all, block some, block forever)
>
> Yup, would be a very good improvement.
>
> -Dave.
> --
> Dave Chinner
> david@fromorbit.com
>