Re: [PATCH v2 2/4] mm: introduce cma_alloc_bulk API

[Michal Hocko]

On Wed 02-12-20 21:22:36, David Hildenbrand wrote:
> On 02.12.20 20:26, Minchan Kim wrote:
> > On Wed, Dec 02, 2020 at 07:51:07PM +0100, Michal Hocko wrote:
[...]
> >> I am still not sure a specific flag is a good interface. Really can this
> >> be gfp_mask instead?
> > 
> > I am not strong(even, I did it with GFP_NORETRY) but David wanted to
> > have special mode and I agreed when he mentioned ALLOC_CONTIG_HARD as
> > one of options in future(it would be hard to indicate that mode with
> > gfp flags).
> 
> I can't tell regarding the CMA interface, but for the alloc_contig()
> interface I think modes make sense. Yes, it's different to other
> allocaters, but the contig range allocater is different already. E.g.,
> the CMA allocater mostly hides "which exact PFNs you try to allocate".

Yes, alloc_contig_range is a low level API but it already has a gfp_mask
parameter. Adding yet another allocation mode sounds like API
convolution to me.

> In the contig range allocater, gfp flags are currently used to express
> how to allocate pages used as migration targets. I don't think mangling
> in other gfp flags (or even overloading them) makes things a lot
> clearer. E.g., GFP_NORETRY: don't retry to allocate migration targets?
> don't retry to migrate pages? both?
>
> As I said, other aspects might be harder to model (e.g., don't drain
> LRU) and hiding them behind generic gfp flags (e.g., GFP_NORETRY) feels
> wrong.
> 
> With the mode, we're expressing details for the necessary page
> migration. Suggestions on how to model that are welcome.

The question is whether the caller should really have such an intimate
knowledge and control of the alloc_contig_range implementation. This all
are implementation details. Should really anybody think about how many
times migration retries or control LRU draining? Those can change in the
future and I do not think we really want to go over all users grown over
that time and try to deduce what was the intention behind.

I think we should aim at easy and very highlevel behavior:
- GFP_NOWAIT - unsupported currently IIRC but something that something
  that should be possible to implement. Isolation is non blocking,
  migration could be skipped
- GFP_KERNEL - default behavior whatever that means
- GFP_NORETRY - opportunistic allocation as lightweight as we can get.
  Failures to be expected also for transient reasons.
- GFP_RETRY_MAYFAIL - try hard but not as hard as to trigger disruption
  (e.g. via oom killer).

- __GFP_THIS_NODE - stick to a node without fallback
- we can support zone modifiers although there is no existing user.
- __GFP_NOWARN - obvious

And that is it. Or maybe I am seeing that oversimplified.
-- 
Michal Hocko
SUSE Labs