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: <Z-3i1wATGh6vI8x8@dread.disaster.area>
Date: Thu, 3 Apr 2025 12:22:31 +1100
From: Dave Chinner <david@...morbit.com>
To: Shakeel Butt <shakeel.butt@...ux.dev>
Cc: Michal Hocko <mhocko@...e.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, 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...
> > 
> > > 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....
> > 
> > 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...
> 
> 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.

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.

-Dave.
-- 
Dave Chinner
david@...morbit.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ