| Message ID | 20200708125608.155645-2-cmaiolino@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Continue xfs kmem cleanup | expand |
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.
diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c index 203e74fa64aa6..583242253c027 100644 --- a/fs/xfs/libxfs/xfs_alloc.c +++ b/fs/xfs/libxfs/xfs_alloc.c @@ -2467,7 +2467,8 @@ xfs_defer_agfl_block( ASSERT(xfs_bmap_free_item_zone != NULL); ASSERT(oinfo != NULL); - new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0); + new = kmem_cache_alloc(xfs_bmap_free_item_zone, + GFP_KERNEL | __GFP_NOFAIL); new->xefi_startblock = XFS_AGB_TO_FSB(mp, agno, agbno); new->xefi_blockcount = 1; new->xefi_oinfo = *oinfo; diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index 667cdd0dfdf4a..fd5c0d669d0d7 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -553,7 +553,8 @@ __xfs_bmap_add_free( #endif ASSERT(xfs_bmap_free_item_zone != NULL); - new = kmem_zone_alloc(xfs_bmap_free_item_zone, 0); + new = kmem_cache_alloc(xfs_bmap_free_item_zone, + GFP_KERNEL | __GFP_NOFAIL); new->xefi_startblock = bno; new->xefi_blockcount = (xfs_extlen_t)len; if (oinfo) diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c index 5daef654956cb..69f887abe4334 100644 --- a/fs/xfs/xfs_icache.c +++ b/fs/xfs/xfs_icache.c @@ -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); + if (inode_init_always(mp->m_super, VFS_I(ip))) { kmem_cache_free(xfs_inode_zone, ip); return NULL;
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. 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(-)