Re: [PATCH 1/2] mm: clarify __GFP_MEMALLOC usage

[Michal Hocko <mhocko@kernel.org>]

On Sat 04-04-20 08:23:45, Neil Brown wrote:
> On Fri, Apr 03 2020, David Rientjes wrote:
[...]
> > Hmm, any guidance that we can offer to users of this flag that aren't 
> > aware of __GFP_MEMALLOC internals?  If I were to read this and not be 
> > aware of the implementation, I would ask "how do I know when I'm at risk 
> > of depleting this reserve" especially since the amount of reserve is 
> > controlled by sysctl.  How do I know when I'm risking a depletion of this 
> > shared reserve?
> 
> "how do I know when I'm at risk of depleting this reserve" is definitely
> the wrong question to be asking.  The questions to ask are:
> - how little memory to I need to ensure forward progress?
> - how quick will that forward progress be?

Absolutely agreed. The total amount of reserves will always depend on
all other users. Unless they are perfectly coordinated, which is not the
case.

> In the ideal case a small allocation will be all that is needed in order
> for that allocation plus another page to be freed "quickly", in time
> governed only by throughput to some device.  In that case you probably
> don't need to worry about rate limiting.

Right but I wouldn't expect this to be a general usage pattern of this
flag. "Allocate to free memory" suggests this would be a part of the
memory reclaim process and that really needs some form of rate
limiting. Be it the reclaim itself directly or some other mechanism if
this happens from a different context.

> The reason I brought up ratelimiting is that RCU is slow.  You can get
> quite a lot of memory caught up in the kfree-rcu lists.  That's not much
> of a problem for normal memory, but it might be for the more limited
> reserves.

Right.

> The other difficulty with the the kfree_rcu case is that we have no idea
> how many users there will be, so we cannot realistically model how long
> the queue might get.  Compare with NFS swap-out there the only user it
> the VM swapping memory which (I think?) already tries to pace writeout
> with the speed of the device (or is that just writeback...).  I'm
> clearly not sure of the details but it is a more constrained environment
> so it is more predicatable.

Mel explained this http://lkml.kernel.org/r/20200401131426.GN3772@suse.de

> In many cases, preallocating a private reserve is better than using
> GFP_MEMALLOC.  That is what mempools provide and they are very effective
> (though often way over-allocated*).
> GFP_MEMALLOC was added because swap-over-NFS requires lots of different
> allocations (transmit headers, receive buffers, possible routing changes
> etc), many of them in the network layer which is very sensitive
> to latency (and mempools require a spinlock to get the reserves).

Yes.

> Maybe the documentation should say.
>  Don't use this - use a mempool.  Here be dragons.

OK, this looks like a good idea.
 
> I'm not sure you can really say anything more useful without writing a
> long essay.

Yes and I am not sure it would be really more helpful than confusing.
What do you think about this updated patch?

From 6c90b0a19a07c87d24ad576e69b33c6e19c2f9a2 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.com>
Date: Wed, 1 Apr 2020 14:00:56 +0200
Subject: [PATCH] mm: clarify __GFP_MEMALLOC usage

It seems that the existing documentation is not explicit about the
expected usage and potential risks enough. While it is calls out
that users have to free memory when using this flag it is not really
apparent that users have to careful to not deplete memory reserves
and that they should implement some sort of throttling wrt. freeing
process.

This is partly based on Neil's explanation [1].

Let's also call out that a pre allocated pool allocator should be
considered.

[1] http://lkml.kernel.org/r/877dz0yxoa.fsf@notabene.neil.brown.name
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/gfp.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index e5b817cb86e7..9cacef1a3ee0 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -110,6 +110,11 @@ struct vm_area_struct;
  * the caller guarantees the allocation will allow more memory to be freed
  * very shortly e.g. process exiting or swapping. Users either should
  * be the MM or co-ordinating closely with the VM (e.g. swap over NFS).
+ * Users of this flag have to be extremely careful to not deplete the reserve
+ * completely and implement a throttling mechanism which controls the consumption
+ * of the reserve based on the amount of freed memory.
+ * Usage of a pre-allocated pool (e.g. mempool) should be always considered before
+ * using this flag.
  *
  * %__GFP_NOMEMALLOC is used to explicitly forbid access to emergency reserves.
  * This takes precedence over the %__GFP_MEMALLOC flag if both are set.
-- 
2.25.1

-- 
Michal Hocko
SUSE Labs