Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Matthew Wilcox" <willy@infradead.org>
Cc:     "Andrew Morton" <akpm@linux-foundation.org>,
        "Michal Hocko" <mhocko@suse.com>,
        "Thierry Reding" <thierry.reding@gmail.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] MM: discard __GFP_ATOMIC
In-reply-to: <YZewStgII83phKtA@casper.infradead.org>
References: <163712397076.13692.4727608274002939094@noble.neil.brown.name>,
 <YZUBIbALcSHn4Rub@casper.infradead.org>,
 <163727727803.13692.15470049610672496362@noble.neil.brown.name>,
 <YZewStgII83phKtA@casper.infradead.org>
Date:   Sat, 20 Nov 2021 21:51:20 +1100
Message-id: <163740548025.13692.6428652897557849182@noble.neil.brown.name>
Precedence: bulk

On Sat, 20 Nov 2021, Matthew Wilcox wrote:
> On Fri, Nov 19, 2021 at 10:14:38AM +1100, NeilBrown wrote:
> > On Thu, 18 Nov 2021, Matthew Wilcox wrote:
> > > Surely this should be gfpflags_allow_blocking() instead of poking about
> > > in the innards of gfp flags?
> > 
> > Possibly.  Didn't know about gfpflags_allow_blocking().  From a quick
> > grep in the kernel, a whole lot of other people don't know about it
> > either, though clearly some do.
> > 
> > Maybe we should reaname "__GFP_DIRECT_RECLAIM" to
> > "__GFP_ALLOW_BLOCKING", because that is what most users seems to care
> > about.
> 
> I tend towards the school of thought that the __GFP flags should make
> sense to the implementation and users should use either GFP_ or functions.
> When we see users adding or subtracting __GFP flags, that's a problem.

Except __GFP_NOWARN of course, or __GFP_ZERO, or __GFP_NOFAIL.
What about __GFP_HIGHMEM?  __GFP_DMA?  __GFP_HIGH?

They all seem to be quite meaningful to the caller - explicitly
specifying properties of the memory or properties of the service.
(But maybe you would prefer __GFP_HIGH be spelled "__GFP_LOW_WATERMARK"
so it would make more sense to the implementation).

__GFP_DIRECTRECLAIM seems to me to be more the exception than the rule -
specifying internal implementation details.

Actually ... I take it back about __GFP_NOWARN.  That probably shouldn't
exist at all.  Warnings should be based on how stressed the mm system is,
not on whether the caller wants thinks failure is manageable.

NeilBrown