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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/6/20 6:00 PM, NeilBrown wrote:
...
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.

I think this version is pretty good.

    *
    * %__GFP_NOMEMALLOC is used to explicitly forbid access to emergency reserves.
    * This takes precedence over the %__GFP_MEMALLOC flag if both are set.


Hi Michal and all,

How about using approximately this wording instead? I found Neil's wording to be
especially helpful so I mixed it in. (Also fixed a couple of slight 80-col overruns.)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index be2754841369..c247a911d8c7 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -111,6 +111,15 @@ struct vm_area_struct;
    * 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).
    *
+ * To be extra clear: users of __GFP_MEMALLOC must be working to free other
+ * memory, and that other memory needs to be freed "soon"; specifically, before
+ * the reserve is exhausted. This generally implies a throttling mechanism that
+ * balances the amount of __GFP_MEMALLOC memory used against the amount that the
+ * caller is about to free.

I don't like this change. "balances the amount ... is about to free"
does say anything about time, so it doesn't seem to be about throttling.

I think it is hard to write rules because the rules are a bit spongey.

With mempools, we have a nice clear rule.  When you allocate from a
mempool you must have a clear path to freeing that allocation which will
not block on memory allocation except from a subordinate mempool.  This
implies a partial ordering between mempools.  When you have layered
block devices the path through the layers from filesystem down to
hardware defines the order.  It isn't enforced, but it is quite easy to
reason about.

GFP_MEMALLOC effectively provides multiple mempools.  So it could
theoretically deadlock if multiple long dependency chains
happened. i.e. if 1000 threads each make a GFP_MEMALLOC allocation and
then need to make another one before the first can be freed - then you
hit problems.  There is no formal way to guarantee that this doesn't
happen.  We just say "be gentle" and minimize the users of this flag,
and keep more memory in reserve than we really need.
Note that 'threads' here might not be Linux tasks.  If you have an IO
request that proceed asynchronously, moving from queue to queue and
being handled by different task, then each one is a "thread" for the
purpose of understanding mem-alloc dependency.

So maybe what I really should focus on is not how quickly things happen,
but how many happen concurrently.  The idea of throttling is to allow
previous requests to complete before we start too many more.

With Swap-over-NFS, some of the things that might need to be allocated
are routing table entries.  These scale with the number of NFS servers
rather than the number of IO requests, so they are not going to cause
concurrency problems.
We also need memory to store replies, but these never exceed the number
of pending requests, so there is limited concurrency there.
NFS can send a lot of requests in parallel, but the main limit is the
RPC "slot table" and while that grows dynamically, it does so with
GFP_NOFS, so it can block or fail (I wonder if that should explicitly
disable the use of the reserves).

So there a limit on concurrency imposed by non-GFP_MEMALLOC allocations

So ... maybe the documentation should say that boundless concurrency of
allocations (i.e. one module allocating a boundless number of times
before previous allocations are freed) must be avoided.


Well, that's a good discussion that you just wrote, above, and I think it
demonstrates that it's hard to describe the situation in just a couple of
sentences. With that in mind, perhaps it's best to take the above notes
as a starting point, adjust them slightly and drop them into
Documentation/core-api/memory-allocation.rst ?

Then the comments here could refer to it.


thanks,
--
John Hubbard
NVIDIA




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux