Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCH 1/2] mm: clarify __GFP_MEMALLOC usage | From | John Hubbard <> | Date | Mon, 6 Apr 2020 18:21:39 -0700 |
| |
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
| ![\](/images/icornerr.gif) |