Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
From:   "NeilBrown" <neilb@suse.de>
To:     "Michal Hocko" <mhocko@suse.com>
Cc:     linux-mm@kvack.org, "Dave Chinner" <david@fromorbit.com>,
        "Andrew Morton" <akpm@linux-foundation.org>,
        "Christoph Hellwig" <hch@infradead.org>,
        "Uladzislau Rezki" <urezki@gmail.com>,
        linux-fsdevel@vger.kernel.org,
        "LKML" <linux-kernel@vger.kernel.org>,
        "Ilya Dryomov" <idryomov@gmail.com>,
        "Jeff Layton" <jlayton@kernel.org>
Subject: Re: [PATCH 3/4] mm/vmalloc: be more explicit about supported gfp flags.
In-reply-to: <YXep1ctN1wPP+1a8@dhcp22.suse.cz>
References: <20211025150223.13621-1-mhocko@kernel.org>,
 <20211025150223.13621-4-mhocko@kernel.org>,
 <163520436674.16092.18372437960890952300@noble.neil.brown.name>,
 <YXep1ctN1wPP+1a8@dhcp22.suse.cz>
Date:   Tue, 26 Oct 2021 21:43:17 +1100
Message-id: <163524499768.8576.4634415079916744478@noble.neil.brown.name>
Precedence: bulk

On Tue, 26 Oct 2021, Michal Hocko wrote:
> On Tue 26-10-21 10:26:06, Neil Brown wrote:
> > On Tue, 26 Oct 2021, Michal Hocko wrote:
> > > From: Michal Hocko <mhocko@suse.com>
> > >=20
> > > The core of the vmalloc allocator __vmalloc_area_node doesn't say
> > > anything about gfp mask argument. Not all gfp flags are supported
> > > though. Be more explicit about constrains.
> > >=20
> > > Signed-off-by: Michal Hocko <mhocko@suse.com>
> > > ---
> > >  mm/vmalloc.c | 12 ++++++++++--
> > >  1 file changed, 10 insertions(+), 2 deletions(-)
> > >=20
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index 602649919a9d..2199d821c981 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -2980,8 +2980,16 @@ static void *__vmalloc_area_node(struct vm_struc=
t *area, gfp_t gfp_mask,
> > >   * @caller:		  caller's return address
> > >   *
> > >   * Allocate enough pages to cover @size from the page level
> > > - * allocator with @gfp_mask flags.  Map them into contiguous
> > > - * kernel virtual space, using a pagetable protection of @prot.
> > > + * allocator with @gfp_mask flags. Please note that the full set of gfp
> > > + * flags are not supported. GFP_KERNEL would be a preferred allocation=
 mode
> > > + * but GFP_NOFS and GFP_NOIO are supported as well. Zone modifiers are=
 not
> >=20
> > In what sense is GFP_KERNEL "preferred"??
> > The choice of GFP_NOFS, when necessary, isn't based on preference but
> > on need.
> >=20
> > I understand that you would prefer no one ever used GFP_NOFs ever - just
> > use the scope API.  I even agree.  But this is not the place to make
> > that case.=20
>=20
> Any suggestion for a better wording?

 "GFP_KERNEL, GFP_NOFS, and GFP_NOIO are all supported".

>=20
> > > + * supported. From the reclaim modifiers__GFP_DIRECT_RECLAIM is requir=
ed (aka
> > > + * GFP_NOWAIT is not supported) and only __GFP_NOFAIL is supported (aka
> >=20
> > I don't think "aka" is the right thing to use here.  It is short for
> > "also known as" and there is nothing that is being known as something
> > else.
> > It would be appropriate to say (i.e. GFP_NOWAIT is not supported).
> > "i.e." is short for the Latin "id est" which means "that is" and
> > normally introduces an alternate description (whereas aka introduces an
> > alternate name).
>=20
> OK
> =20
> > > + * __GFP_NORETRY and __GFP_RETRY_MAYFAIL are not supported).
> >=20
> > Why do you think __GFP_NORETRY and __GFP_RETRY_MAYFAIL are not supported.
>=20
> Because they cannot be passed to the page table allocator. In both cases
> the allocation would fail when system is short on memory. GFP_KERNEL
> used for ptes implicitly doesn't behave that way.

Could you please point me to the particular allocation which uses
GFP_KERNEL rather than the flags passed to __vmalloc_node()?  I cannot
find it.

>=20
> >=20
> > > + * __GFP_NOWARN can be used to suppress error messages about failures.
> >=20
> > Surely "NOWARN" suppresses warning messages, not error messages ....
>=20
> I am not sure I follow. NOWARN means "do not warn" independently on the
> log level chosen for the message. Is an allocation failure an error
> message? Is the "vmalloc error: size %lu, failed to map pages" an error
> message?

If guess working with a C compiler has trained me to think that
"warnings" are different from "errors".

>=20
> Anyway I will go with "__GFP_NOWARN can be used to suppress failure message=
s"
>=20
> Is that better?

Yes, that's an excellent solution!  Thanks.

NeilBrown


> --=20
> Michal Hocko
> SUSE Labs
>=20
>=20