Re: [PATCH 1/4] xfs: Remove kmem_zone_alloc() usage

[Dave Chinner <david@fromorbit.com>]

On Wed, Jul 08, 2020 at 02:56:05PM +0200, Carlos Maiolino wrote:
> Use kmem_cache_alloc() directly.
> 
> All kmem_zone_alloc() users pass 0 as flags, which are translated into:
> GFP_KERNEL | __GFP_NOWARN, and kmem_zone_alloc() loops forever until the
> allocation succeeds.
> 
> So, call kmem_cache_alloc() with __GFP_NOFAIL directly. which will have
> the same result.
> 
> Once allocation will never fail, don't bother to add __GFP_NOWARN.

Last two paragraphs are a little odd. Maybe:

We can use __GFP_NOFAIL to tell the allocator to loop forever rather
than doing it ourself, and because the allocation will never fail we
do not need to use __GFP_NOWARN anymore. Hence all callers can be
converted to use GFP_KERNEL | __GFP_NOFAIL.


> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_alloc.c |  3 ++-
>  fs/xfs/libxfs/xfs_bmap.c  |  3 ++-
>  fs/xfs/xfs_icache.c       | 11 +++--------
>  3 files changed, 7 insertions(+), 10 deletions(-)
> 
> @@ -36,14 +36,9 @@ xfs_inode_alloc(
>  {
>  	struct xfs_inode	*ip;
>  
> -	/*
> -	 * if this didn't occur in transactions, we could use
> -	 * KM_MAYFAIL and return NULL here on ENOMEM. Set the
> -	 * code up to do this anyway.
> -	 */
> -	ip = kmem_zone_alloc(xfs_inode_zone, 0);
> -	if (!ip)
> -		return NULL;
> +	ip = kmem_cache_alloc(xfs_inode_zone,
> +			      GFP_KERNEL | __GFP_NOFAIL);
> +

Hmmmm. We really should check PF_FSTRANS here for the flags we
should be setting. Something like:

	gfp_t		gfp_mask = GFP_KERNEL;

	if (current->flags & PF_FSTRANS)
		gfp_mask |= __GFP_NOFAIL;

	ip = kmem_cache_alloc(xfs_inode_zone, gfp_mask);
	if (!ip)
		return NULL;

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com