Message-ID: <Z-42znN1q7dVNM-h@tiehlicka>
Date: Thu, 3 Apr 2025 09:20:46 +0200
From: Michal Hocko <mhocko@...e.com>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Dave Chinner <david@...morbit.com>, Yafang Shao <laoar.shao@...il.com>,
	Harry Yoo <harry.yoo@...cle.com>, Kees Cook <kees@...nel.org>,
	joel.granados@...nel.org, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org, Josef Bacik <josef@...icpanda.com>,
	linux-mm@...ck.org, Vlastimil Babka <vbabka@...e.cz>
Subject: Re: [PATCH] proc: Avoid costly high-order page allocations when
 reading proc files

On Wed 02-04-25 22:05:57, Shakeel Butt wrote:
> On Thu, Apr 03, 2025 at 12:22:31PM +1100, Dave Chinner wrote:
> > On Wed, Apr 02, 2025 at 04:10:06PM -0700, Shakeel Butt wrote:
> > > On Thu, Apr 03, 2025 at 08:16:56AM +1100, Dave Chinner wrote:
> > > > On Wed, Apr 02, 2025 at 02:24:45PM +0200, Michal Hocko wrote:
> > > > > On Wed 02-04-25 22:32:14, Dave Chinner wrote:
> > > > > > Have a look at xlog_kvmalloc() in XFS. It implements a basic
> > > > > > fast-fail, no retry high order kmalloc before it falls back to
> > > > > > vmalloc by turning off direct reclaim for the kmalloc() call.
> > > > > > Hence if the there isn't a high-order page on the free lists ready
> > > > > > to allocate, it falls back to vmalloc() immediately.
> > > > > > 
> > > > > > For XFS, using xlog_kvmalloc() reduced the high-order per-allocation
> > > > > > overhead by around 80% when compared to a standard kvmalloc()
> > > > > > call. Numbers and profiles were documented in the commit message
> > > > > > (reproduced in whole below)...
> > > > > 
> > > > > Btw. it would be really great to have such concerns to be posted to the
> > > > > linux-mm ML so that we are aware of that.
> > > > 
> > > > I have brought it up in the past, along with all the other kvmalloc
> > > > API problems that are mentioned in that commit message.
> > > > Unfortunately, discussion focus always ended up on calling context
> > > > and API flags (e.g. whether stuff like GFP_NOFS should be supported
> > > > or not) no the fast-fail-then-no-fail behaviour we need.
> > > > 
> > > > Yes, these discussions have resulted in API changes that support
> > > > some new subset of gfp flags, but the performance issues have never
> > > > been addressed...

I, at least, was not aware of the performance aspect. We are trying to
make kvmalloc as usable as possible to prevent its open coded variants
to grow in subystems.

> > > > > kvmalloc currently doesn't support GFP_NOWAIT semantic but it does allow
> > > > > to express - I prefer SLAB allocator over vmalloc.
> > > > 
> > > > The conditional use of __GFP_NORETRY for the kmalloc call is broken
> > > > if we try to use __GFP_NOFAIL with kvmalloc() - this causes the gfp
> > > > mask to hold __GFP_NOFAIL | __GFP_NORETRY....

Correct.

> > > > We have a hard requirement for xlog_kvmalloc() to provide
> > > > __GFP_NOFAIL semantics.
> > > > 
> > > > IOWs, we need kvmalloc() to support kmalloc(GFP_NOWAIT) for
> > > > performance with fallback to vmalloc(__GFP_NOFAIL) for
> > > > correctness...

Understood.

> > > Are you asking the above kvmalloc() semantics just for xfs or for all
> > > the users of kvmalloc() api? 
> > 
> > I'm suggesting that fast-fail should be the default behaviour for
> > everyone.
> > 
> > If you look at __vmalloc() internals, you'll see that it turns off
> > __GFP_NOFAIL for high order allocations because "reclaim is too
> > costly and it's far cheaper to fall back to order-0 pages".
> > 
> > That's pretty much exactly what we are doing with xlog_kvmalloc(),
> > and what I'm suggesting that kvmalloc should be doing by default.
> > 
> > i.e. If it's necessary for mm internal implementations to avoid
> > high-order reclaim when there is a faster order-0 allocation
> > fallback path available for performance reasons, then we should be
> > using that same behaviour anywhere optimisitic high-order allocation
> > is used as an optimisation for those same performance reasons.
> > 
> 
> I am convinced and I think Michal is onboard as well for the above. At
> least we should try and see how it goes.

If we find out that this doesn't really work because a fragmentation
of page blocks is a real problem then we might need to reconsider this.

> > The overall __GFP_NOFAIL requirement is something XFS needs, but it
> > is most definitely not something that should be enabled by default.
> > However, it needs to work with kvmalloc(), and it is not possible to
> > do so right now.
> 
> After the kmalloc(GFP_NOWAIT) being default in kvmalloc(), what remains
> to support kvmalloc(__GFP_NOFAIL)? (Yafang mentioned vmap_huge)

We already do support kvmalloc(__GFP_NOFAIL) since 9376130c390a7 IIRC.

-- 
Michal Hocko
SUSE Labs

Powered by blists - more mailing lists