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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxx
