linux-kernel - Re: [PATCH 1/2] mm: clarify __GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200407072439.GG18914@dhcp22.suse.cz>
Date:   Tue, 7 Apr 2020 09:24:39 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     NeilBrown <neilb@...e.de>
Cc:     John Hubbard <jhubbard@...dia.com>,
        David Rientjes <rientjes@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joel Fernandes <joel@...lfernandes.org>,
        "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] mm: clarify __GFP_MEMALLOC usage

On Tue 07-04-20 11:00:29, Neil Brown wrote:
> On Mon, Apr 06 2020, John Hubbard wrote:
> 
> > On 4/6/20 12:01 AM, Michal Hocko 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.

Thanks! I will stick with it then.

[...]

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

Exactly! And the more specific we are the more likely people are going
to follow literally. And we do not want that. We want people to be aware
of the limitation but want them to think hard before using the flag.

> 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

This really makes sense to mention in the allocation manual
(Documentation/core-api/memory-allocation.rst) as suggested by John.
Care to make it into a patch?
-- 
Michal Hocko
SUSE Labs