On Thu, Apr 3, 2025 at 9:22 AM Dave Chinner <david@xxxxxxxxxxxxx> 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...
> > >
> > > > 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".
This behavior was introduced in commit 7de8728f55ff ("mm: vmalloc:
refactor vm_area_alloc_pages()") and only applies when
HAVE_ARCH_HUGE_VMALLOC is enabled (added in commit 121e6f3258fe,
"mm/vmalloc: hugepage vmalloc mappings").
Instead of disabling __GFP_NOFAIL for hugevmalloc allocations, perhaps
we could simply enforce "vmap_allow_huge= false" when __GFP_NOFAIL is
specified. Or we could ...
>
> 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.
1. Introduce a new vmalloc() flag to explicitly disable hugepage
mappings when needed (e.g., for __GFP_NOFAIL cases).
2. Extend kvmalloc() with finer control by allowing separate GFP flags
for kmalloc and vmalloc, plus an option to disable hugevmalloc:
kvmalloc(size_t size, gfp_t kmalloc_flags, gfp_t vmalloc_flags, bool
allow_hugevmalloc);
Then we can replace the xlog_cil_kvmalloc() with:
kvmalloc(size, GFP_NOWAIT, __GFP_NOFAIL, false);
This is just a preliminary idea...
--
Regards
Yafang
