On Wed, Jul 02, 2025 at 09:30:30AM +0200, Vlastimil Babka wrote:
> On 7/2/25 3:41 AM, Tetsuo Handa wrote:
> > By the way, why is xfs_init_fs_context() using __GFP_NOFAIL ?
> >
> > mp = kzalloc(sizeof(struct xfs_mount), GFP_KERNEL | __GFP_NOFAIL);
> > if (!mp)
> > return -ENOMEM;
> >
> > This looks an allocation attempt which can fail safely.
It's irrelevant - it shouldn't fail regardless of __GFP_NOFAIL being
specified.
> Indeed. Dave Chinner's commit f078d4ea82760 ("xfs: convert kmem_alloc()
> to kmalloc()") dropped the xfs wrapper. This allocation didn't use
> KM_MAYFAIL so it got __GFP_NOFAIL. The commit mentions this high-order
> nofail issue for another allocation site that had to use xlog_kvmalloc().
I don't see how high-order allocation behaviour is relevant here.
Pahole says the struct xfs_mount is 4224 bytes in length. It is an
order-1 allocation and if we've fragmented memory so badly that slab
can't allocate an order-1 page then *lots* of other stuff is going
to be stalling. (e.g. slab pages for inodes are typically order-3,
same as the kmalloc-8kk slab).
Note that the size of the structure is largely because of the
embedded cpumask for inodegc:
struct cpumask m_inodegc_cpumask; /* 3104 1024 */
This should probably be pulled out into a dynamically allocated
inodegc specific structure. Then the struct xfs_mount is only a
order-0 allocation and should never fail, regardless of
__GFP_NOFAIL being specified or not.
> I think either this allocation really can fail as the code (return
> -ENOMEM) suggests and thus can drop __GFP_NOFAIL, or it can use
> kvmalloc() - I think the wrapper for that can be removed now too after
> the discussion in [1] resulted in commit 46459154f997 ("mm: kvmalloc:
> make kmalloc fast path real fast path").
I know about that - I have patches that I'm testing that replace
xlog_kvmalloc() with kvmalloc calls.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
