[PATCH 0/5] Continue xfs kmem cleanup - V3

[PATCH 0/5] Continue xfs kmem cleanup - V3

[Carlos Maiolino]

Hi, new version of this series, including suggestions on the previous one.

In summary, this new version mostly changes patches 1 and 5 only, where:

Patch 1: uses __GFP_NOFAIL unconditionally in xfs_inode_alloc() instead of
current->flags, so it keeps the original function's logic, which will be changed
in a different patch not belonging to this series.

Patch 5: instead of removing xfs_da_state_alloc(), it refactors the function to
also set state->{args, mp} removing some lines of code. This patch now doesn't
'really' belong to this series since it's now just a refactoring, but
essentially it's a V2 of the previous one, but if needed I can submit it alone.

Series survived a few xfstests run.

Cheers.

Carlos Maiolino (5):
  xfs: Remove kmem_zone_alloc() usage
  xfs: Remove kmem_zone_zalloc() usage
  xfs: Modify xlog_ticket_alloc() to use kernel's MM API
  xfs: remove xfs_zone_{alloc,zalloc} helpers
  xfs: Refactor xfs_da_state_alloc() helper

 fs/xfs/kmem.c                      | 21 ---------------------
 fs/xfs/kmem.h                      |  8 --------
 fs/xfs/libxfs/xfs_alloc.c          |  3 ++-
 fs/xfs/libxfs/xfs_alloc_btree.c    |  2 +-
 fs/xfs/libxfs/xfs_attr.c           | 17 +++++------------
 fs/xfs/libxfs/xfs_bmap.c           |  8 ++++++--
 fs/xfs/libxfs/xfs_bmap_btree.c     |  2 +-
 fs/xfs/libxfs/xfs_da_btree.c       |  8 ++++++--
 fs/xfs/libxfs/xfs_da_btree.h       |  2 +-
 fs/xfs/libxfs/xfs_dir2_node.c      | 17 +++++------------
 fs/xfs/libxfs/xfs_ialloc_btree.c   |  2 +-
 fs/xfs/libxfs/xfs_inode_fork.c     |  6 +++---
 fs/xfs/libxfs/xfs_refcount_btree.c |  2 +-
 fs/xfs/libxfs/xfs_rmap_btree.c     |  2 +-
 fs/xfs/scrub/dabtree.c             |  4 +---
 fs/xfs/xfs_bmap_item.c             |  4 ++--
 fs/xfs/xfs_buf.c                   |  4 +---
 fs/xfs/xfs_buf_item.c              |  2 +-
 fs/xfs/xfs_dquot.c                 |  2 +-
 fs/xfs/xfs_extfree_item.c          |  6 ++++--
 fs/xfs/xfs_icache.c                | 10 ++--------
 fs/xfs/xfs_icreate_item.c          |  2 +-
 fs/xfs/xfs_inode_item.c            |  3 ++-
 fs/xfs/xfs_log.c                   |  9 +++------
 fs/xfs/xfs_log_cil.c               |  3 +--
 fs/xfs/xfs_log_priv.h              |  4 +---
 fs/xfs/xfs_refcount_item.c         |  5 +++--
 fs/xfs/xfs_rmap_item.c             |  5 +++--
 fs/xfs/xfs_trace.h                 |  1 -
 fs/xfs/xfs_trans.c                 |  4 ++--
 fs/xfs/xfs_trans_dquot.c           |  3 ++-
 31 files changed, 63 insertions(+), 108 deletions(-)

-- 
2.26.2

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

[Carlos Maiolino]

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.

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>
---

Changelog:
	V2:
		- Wire up xfs_inode_alloc to use __GFP_NOFAIL
		  if it's called inside a transaction
		- Rewrite changelog in a more decent way.
	V3:
		- Use __GFP_NOFAIL unconditionally in xfs_inode_alloc(),
		  use of PF_FSTRANS will be added when the patch re-adding
		  it is moved to mainline.

 fs/xfs/libxfs/xfs_alloc.c |  3 ++-
 fs/xfs/libxfs/xfs_bmap.c  |  3 ++-
 fs/xfs/xfs_icache.c       | 10 ++--------
 3 files changed, 6 insertions(+), 10 deletions(-)

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 58a750ce689c0..c2d97e4f131fb 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -36,14 +36,8 @@ 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;
-- 
2.26.2

[PATCH 2/5] xfs: Remove kmem_zone_zalloc() usage

[Carlos Maiolino]

Use kmem_cache_zalloc() directly.

With the exception of xlog_ticket_alloc() which will be dealt on the
next patch for readability.


Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V2:
		- Remove comment left by mistake on previous patch
		- Keep the same logic in _xfs_buf_alloc, not allowing
		  its allocation to fail
		- Fix some line breaks keeping them inside the 80char
		  rule.

 fs/xfs/libxfs/xfs_alloc_btree.c    | 2 +-
 fs/xfs/libxfs/xfs_bmap.c           | 5 ++++-
 fs/xfs/libxfs/xfs_bmap_btree.c     | 2 +-
 fs/xfs/libxfs/xfs_da_btree.c       | 2 +-
 fs/xfs/libxfs/xfs_ialloc_btree.c   | 2 +-
 fs/xfs/libxfs/xfs_inode_fork.c     | 6 +++---
 fs/xfs/libxfs/xfs_refcount_btree.c | 2 +-
 fs/xfs/libxfs/xfs_rmap_btree.c     | 2 +-
 fs/xfs/xfs_bmap_item.c             | 4 ++--
 fs/xfs/xfs_buf.c                   | 4 +---
 fs/xfs/xfs_buf_item.c              | 2 +-
 fs/xfs/xfs_dquot.c                 | 2 +-
 fs/xfs/xfs_extfree_item.c          | 6 ++++--
 fs/xfs/xfs_icreate_item.c          | 2 +-
 fs/xfs/xfs_inode_item.c            | 3 ++-
 fs/xfs/xfs_refcount_item.c         | 5 +++--
 fs/xfs/xfs_rmap_item.c             | 5 +++--
 fs/xfs/xfs_trans.c                 | 4 ++--
 fs/xfs/xfs_trans_dquot.c           | 3 ++-
 19 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 60c453cb3ee37..57f8b16a6ea44 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -484,7 +484,7 @@ xfs_allocbt_init_common(
 
 	ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
 
-	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
 
 	cur->bc_tp = tp;
 	cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index fd5c0d669d0d7..9c40d59710357 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1099,7 +1099,10 @@ xfs_bmap_add_attrfork(
 	if (error)
 		goto trans_cancel;
 	ASSERT(ip->i_afp == NULL);
-	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, 0);
+
+	ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone,
+				      GFP_KERNEL | __GFP_NOFAIL);
+
 	ip->i_afp->if_format = XFS_DINODE_FMT_EXTENTS;
 	ip->i_afp->if_flags = XFS_IFEXTENTS;
 	logflags = 0;
diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
index d9c63f17d2dec..ecec604e6e4d7 100644
--- a/fs/xfs/libxfs/xfs_bmap_btree.c
+++ b/fs/xfs/libxfs/xfs_bmap_btree.c
@@ -552,7 +552,7 @@ xfs_bmbt_init_cursor(
 	struct xfs_btree_cur	*cur;
 	ASSERT(whichfork != XFS_COW_FORK);
 
-	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
 
 	cur->bc_tp = tp;
 	cur->bc_mp = mp;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 897749c41f36e..a4e1f01daf3d8 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -81,7 +81,7 @@ kmem_zone_t *xfs_da_state_zone;	/* anchor for state struct zone */
 xfs_da_state_t *
 xfs_da_state_alloc(void)
 {
-	return kmem_zone_zalloc(xfs_da_state_zone, KM_NOFS);
+	return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL);
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
index b2c122ad8f0e9..3c8aebc36e643 100644
--- a/fs/xfs/libxfs/xfs_ialloc_btree.c
+++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
@@ -411,7 +411,7 @@ xfs_inobt_init_common(
 {
 	struct xfs_btree_cur	*cur;
 
-	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
 	cur->bc_tp = tp;
 	cur->bc_mp = mp;
 	cur->bc_btnum = btnum;
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 28b366275ae0e..0cf853d42d622 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -291,7 +291,7 @@ xfs_iformat_attr_fork(
 	 * Initialize the extent count early, as the per-format routines may
 	 * depend on it.
 	 */
-	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_NOFS);
+	ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone, GFP_NOFS | __GFP_NOFAIL);
 	ip->i_afp->if_format = dip->di_aformat;
 	if (unlikely(ip->i_afp->if_format == 0)) /* pre IRIX 6.2 file system */
 		ip->i_afp->if_format = XFS_DINODE_FMT_EXTENTS;
@@ -673,8 +673,8 @@ xfs_ifork_init_cow(
 	if (ip->i_cowfp)
 		return;
 
-	ip->i_cowfp = kmem_zone_zalloc(xfs_ifork_zone,
-				       KM_NOFS);
+	ip->i_cowfp = kmem_cache_zalloc(xfs_ifork_zone,
+				       GFP_NOFS | __GFP_NOFAIL);
 	ip->i_cowfp->if_flags = XFS_IFEXTENTS;
 	ip->i_cowfp->if_format = XFS_DINODE_FMT_EXTENTS;
 }
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 7fd6044a4f780..fde21b5e82ed0 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -325,7 +325,7 @@ xfs_refcountbt_init_common(
 	ASSERT(agno != NULLAGNUMBER);
 	ASSERT(agno < mp->m_sb.sb_agcount);
 
-	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
 	cur->bc_tp = tp;
 	cur->bc_mp = mp;
 	cur->bc_btnum = XFS_BTNUM_REFC;
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index b7c05314d07c9..caf0d799c1f42 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -457,7 +457,7 @@ xfs_rmapbt_init_common(
 {
 	struct xfs_btree_cur	*cur;
 
-	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
+	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
 	cur->bc_tp = tp;
 	cur->bc_mp = mp;
 	/* Overlapping btree; 2 keys per pointer. */
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 6736c5ab188f2..ec3691372e7c0 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -138,7 +138,7 @@ xfs_bui_init(
 {
 	struct xfs_bui_log_item		*buip;
 
-	buip = kmem_zone_zalloc(xfs_bui_zone, 0);
+	buip = kmem_cache_zalloc(xfs_bui_zone, GFP_KERNEL | __GFP_NOFAIL);
 
 	xfs_log_item_init(mp, &buip->bui_item, XFS_LI_BUI, &xfs_bui_item_ops);
 	buip->bui_format.bui_nextents = XFS_BUI_MAX_FAST_EXTENTS;
@@ -215,7 +215,7 @@ xfs_trans_get_bud(
 {
 	struct xfs_bud_log_item		*budp;
 
-	budp = kmem_zone_zalloc(xfs_bud_zone, 0);
+	budp = kmem_cache_zalloc(xfs_bud_zone, GFP_KERNEL | __GFP_NOFAIL);
 	xfs_log_item_init(tp->t_mountp, &budp->bud_item, XFS_LI_BUD,
 			  &xfs_bud_item_ops);
 	budp->bud_buip = buip;
diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index dda0c94458797..d4cdcb6fb2fe1 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -214,9 +214,7 @@ _xfs_buf_alloc(
 	int			i;
 
 	*bpp = NULL;
-	bp = kmem_zone_zalloc(xfs_buf_zone, KM_NOFS);
-	if (unlikely(!bp))
-		return -ENOMEM;
+	bp = kmem_cache_zalloc(xfs_buf_zone, GFP_NOFS | __GFP_NOFAIL);
 
 	/*
 	 * We don't want certain flags to appear in b_flags unless they are
diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index e9428c30862a9..3eb45782cf923 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -739,7 +739,7 @@ xfs_buf_item_init(
 		return 0;
 	}
 
-	bip = kmem_zone_zalloc(xfs_buf_item_zone, 0);
+	bip = kmem_cache_zalloc(xfs_buf_item_zone, GFP_KERNEL | __GFP_NOFAIL);
 	xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF, &xfs_buf_item_ops);
 	bip->bli_buf = bp;
 
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 76353c9a723ee..0f1a0de761787 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -475,7 +475,7 @@ xfs_dquot_alloc(
 {
 	struct xfs_dquot	*dqp;
 
-	dqp = kmem_zone_zalloc(xfs_qm_dqzone, 0);
+	dqp = kmem_cache_zalloc(xfs_qm_dqzone, GFP_KERNEL | __GFP_NOFAIL);
 
 	dqp->dq_flags = type;
 	dqp->q_core.d_id = cpu_to_be32(id);
diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index b9c333bae0a12..6cb8cd11072a3 100644
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -161,7 +161,8 @@ xfs_efi_init(
 			((nextents - 1) * sizeof(xfs_extent_t)));
 		efip = kmem_zalloc(size, 0);
 	} else {
-		efip = kmem_zone_zalloc(xfs_efi_zone, 0);
+		efip = kmem_cache_zalloc(xfs_efi_zone,
+					 GFP_KERNEL | __GFP_NOFAIL);
 	}
 
 	xfs_log_item_init(mp, &efip->efi_item, XFS_LI_EFI, &xfs_efi_item_ops);
@@ -332,7 +333,8 @@ xfs_trans_get_efd(
 				(nextents - 1) * sizeof(struct xfs_extent),
 				0);
 	} else {
-		efdp = kmem_zone_zalloc(xfs_efd_zone, 0);
+		efdp = kmem_cache_zalloc(xfs_efd_zone,
+					GFP_KERNEL | __GFP_NOFAIL);
 	}
 
 	xfs_log_item_init(tp->t_mountp, &efdp->efd_item, XFS_LI_EFD,
diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
index 287a9e5c7d758..9b3994b9c716d 100644
--- a/fs/xfs/xfs_icreate_item.c
+++ b/fs/xfs/xfs_icreate_item.c
@@ -97,7 +97,7 @@ xfs_icreate_log(
 {
 	struct xfs_icreate_item	*icp;
 
-	icp = kmem_zone_zalloc(xfs_icreate_zone, 0);
+	icp = kmem_cache_zalloc(xfs_icreate_zone, GFP_KERNEL | __GFP_NOFAIL);
 
 	xfs_log_item_init(tp->t_mountp, &icp->ic_item, XFS_LI_ICREATE,
 			  &xfs_icreate_item_ops);
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index 3840117f8a5e2..895f61b2b4f01 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -615,7 +615,8 @@ xfs_inode_item_init(
 	struct xfs_inode_log_item *iip;
 
 	ASSERT(ip->i_itemp == NULL);
-	iip = ip->i_itemp = kmem_zone_zalloc(xfs_ili_zone, 0);
+	iip = ip->i_itemp = kmem_cache_zalloc(xfs_ili_zone,
+					      GFP_KERNEL | __GFP_NOFAIL);
 
 	iip->ili_inode = ip;
 	spin_lock_init(&iip->ili_lock);
diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index c81639891e298..7b2c72bc28582 100644
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -143,7 +143,8 @@ xfs_cui_init(
 		cuip = kmem_zalloc(xfs_cui_log_item_sizeof(nextents),
 				0);
 	else
-		cuip = kmem_zone_zalloc(xfs_cui_zone, 0);
+		cuip = kmem_cache_zalloc(xfs_cui_zone,
+					 GFP_KERNEL | __GFP_NOFAIL);
 
 	xfs_log_item_init(mp, &cuip->cui_item, XFS_LI_CUI, &xfs_cui_item_ops);
 	cuip->cui_format.cui_nextents = nextents;
@@ -220,7 +221,7 @@ xfs_trans_get_cud(
 {
 	struct xfs_cud_log_item		*cudp;
 
-	cudp = kmem_zone_zalloc(xfs_cud_zone, 0);
+	cudp = kmem_cache_zalloc(xfs_cud_zone, GFP_KERNEL | __GFP_NOFAIL);
 	xfs_log_item_init(tp->t_mountp, &cudp->cud_item, XFS_LI_CUD,
 			  &xfs_cud_item_ops);
 	cudp->cud_cuip = cuip;
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index a86599db20a6f..dc5b0753cd519 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -141,7 +141,8 @@ xfs_rui_init(
 	if (nextents > XFS_RUI_MAX_FAST_EXTENTS)
 		ruip = kmem_zalloc(xfs_rui_log_item_sizeof(nextents), 0);
 	else
-		ruip = kmem_zone_zalloc(xfs_rui_zone, 0);
+		ruip = kmem_cache_zalloc(xfs_rui_zone,
+					 GFP_KERNEL | __GFP_NOFAIL);
 
 	xfs_log_item_init(mp, &ruip->rui_item, XFS_LI_RUI, &xfs_rui_item_ops);
 	ruip->rui_format.rui_nextents = nextents;
@@ -243,7 +244,7 @@ xfs_trans_get_rud(
 {
 	struct xfs_rud_log_item		*rudp;
 
-	rudp = kmem_zone_zalloc(xfs_rud_zone, 0);
+	rudp = kmem_cache_zalloc(xfs_rud_zone, GFP_KERNEL | __GFP_NOFAIL);
 	xfs_log_item_init(tp->t_mountp, &rudp->rud_item, XFS_LI_RUD,
 			  &xfs_rud_item_ops);
 	rudp->rud_ruip = ruip;
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 0ad72a83edac4..ed72867b1a193 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -90,7 +90,7 @@ xfs_trans_dup(
 
 	trace_xfs_trans_dup(tp, _RET_IP_);
 
-	ntp = kmem_zone_zalloc(xfs_trans_zone, 0);
+	ntp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
 
 	/*
 	 * Initialize the new transaction structure.
@@ -263,7 +263,7 @@ xfs_trans_alloc(
 	 * GFP_NOFS allocation context so that we avoid lockdep false positives
 	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
 	 */
-	tp = kmem_zone_zalloc(xfs_trans_zone, 0);
+	tp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
 	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
 		sb_start_intwrite(mp->m_super);
 
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
index c0f73b82c0551..394d6a0aa18dc 100644
--- a/fs/xfs/xfs_trans_dquot.c
+++ b/fs/xfs/xfs_trans_dquot.c
@@ -860,7 +860,8 @@ STATIC void
 xfs_trans_alloc_dqinfo(
 	xfs_trans_t	*tp)
 {
-	tp->t_dqinfo = kmem_zone_zalloc(xfs_qm_dqtrxzone, 0);
+	tp->t_dqinfo = kmem_cache_zalloc(xfs_qm_dqtrxzone,
+					 GFP_KERNEL | __GFP_NOFAIL);
 }
 
 void
-- 
2.26.2

[PATCH 3/5] xfs: Modify xlog_ticket_alloc() to use kernel's MM API

[Carlos Maiolino]

xlog_ticket_alloc() is always called under NOFS context, except from
unmount path, which eitherway is holding many FS locks, so, there is no
need for its callers to keep passing allocation flags into it.

change xlog_ticket_alloc() to use default kmem_cache_zalloc(), remove
its alloc_flags argument, and always use GFP_NOFS | __GFP_NOFAIL flags.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V2:
		- Remove alloc_flags argument from xlog_ticket_alloc()
		  and update patch description accordingly.

 fs/xfs/xfs_log.c      | 9 +++------
 fs/xfs/xfs_log_cil.c  | 3 +--
 fs/xfs/xfs_log_priv.h | 4 +---
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index 00fda2e8e7380..ad0c69ee89475 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -433,7 +433,7 @@ xfs_log_reserve(
 	XFS_STATS_INC(mp, xs_try_logspace);
 
 	ASSERT(*ticp == NULL);
-	tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0);
+	tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent);
 	*ticp = tic;
 
 	xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt
@@ -3408,15 +3408,12 @@ xlog_ticket_alloc(
 	int			unit_bytes,
 	int			cnt,
 	char			client,
-	bool			permanent,
-	xfs_km_flags_t		alloc_flags)
+	bool			permanent)
 {
 	struct xlog_ticket	*tic;
 	int			unit_res;
 
-	tic = kmem_zone_zalloc(xfs_log_ticket_zone, alloc_flags);
-	if (!tic)
-		return NULL;
+	tic = kmem_cache_zalloc(xfs_log_ticket_zone, GFP_NOFS | __GFP_NOFAIL);
 
 	unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes);
 
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index 9ed90368ab311..56c32eecffead 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -37,8 +37,7 @@ xlog_cil_ticket_alloc(
 {
 	struct xlog_ticket *tic;
 
-	tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0,
-				KM_NOFS);
+	tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0);
 
 	/*
 	 * set the current reservation to zero so we know to steal the basic
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index 75a62870b63af..1c6fdbf3d5066 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -464,9 +464,7 @@ xlog_ticket_alloc(
 	int		unit_bytes,
 	int		count,
 	char		client,
-	bool		permanent,
-	xfs_km_flags_t	alloc_flags);
-
+	bool		permanent);
 
 static inline void
 xlog_write_adv_cnt(void **ptr, int *len, int *off, size_t bytes)
-- 
2.26.2

[PATCH 4/5] xfs: remove xfs_zone_{alloc,zalloc} helpers

[Carlos Maiolino]

All their users have been converted to use MM API directly, no need to
keep them around anymore.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/kmem.c      | 21 ---------------------
 fs/xfs/kmem.h      |  8 --------
 fs/xfs/xfs_trace.h |  1 -
 3 files changed, 30 deletions(-)

diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
index f1366475c389c..e841ed781a257 100644
--- a/fs/xfs/kmem.c
+++ b/fs/xfs/kmem.c
@@ -115,24 +115,3 @@ kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
 		congestion_wait(BLK_RW_ASYNC, HZ/50);
 	} while (1);
 }
-
-void *
-kmem_zone_alloc(kmem_zone_t *zone, xfs_km_flags_t flags)
-{
-	int	retries = 0;
-	gfp_t	lflags = kmem_flags_convert(flags);
-	void	*ptr;
-
-	trace_kmem_zone_alloc(kmem_cache_size(zone), flags, _RET_IP_);
-	do {
-		ptr = kmem_cache_alloc(zone, lflags);
-		if (ptr || (flags & KM_MAYFAIL))
-			return ptr;
-		if (!(++retries % 100))
-			xfs_err(NULL,
-		"%s(%u) possible memory allocation deadlock in %s (mode:0x%x)",
-				current->comm, current->pid,
-				__func__, lflags);
-		congestion_wait(BLK_RW_ASYNC, HZ/50);
-	} while (1);
-}
diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
index 34cbcfde92281..8e8555817e6d3 100644
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -85,14 +85,6 @@ kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
 #define kmem_zone	kmem_cache
 #define kmem_zone_t	struct kmem_cache
 
-extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t);
-
-static inline void *
-kmem_zone_zalloc(kmem_zone_t *zone, xfs_km_flags_t flags)
-{
-	return kmem_zone_alloc(zone, flags | KM_ZERO);
-}
-
 static inline struct page *
 kmem_to_page(void *addr)
 {
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 50c478374a31b..627fe4fef9ac7 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -3536,7 +3536,6 @@ DEFINE_KMEM_EVENT(kmem_alloc);
 DEFINE_KMEM_EVENT(kmem_alloc_io);
 DEFINE_KMEM_EVENT(kmem_alloc_large);
 DEFINE_KMEM_EVENT(kmem_realloc);
-DEFINE_KMEM_EVENT(kmem_zone_alloc);
 
 TRACE_EVENT(xfs_check_new_dalign,
 	TP_PROTO(struct xfs_mount *mp, int new_dalign, xfs_ino_t calc_rootino),
-- 
2.26.2

[PATCH 5/5] xfs: Refactor xfs_da_state_alloc() helper

[Carlos Maiolino]

Every call to xfs_da_state_alloc() also requires setting up state->args
and state->mp

Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and
return a xfs_da_state_t with both args and mp already set.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Changelog:
	V3:
		- Originally this patch removed xfs_da_state_alloc(),
		  per hch's suggestion, instead of removing, it has been
		  refactored, to also set state->{args,mp} which removes
		  a few lines of code.

 fs/xfs/libxfs/xfs_attr.c      | 17 +++++------------
 fs/xfs/libxfs/xfs_da_btree.c  |  8 ++++++--
 fs/xfs/libxfs/xfs_da_btree.h  |  2 +-
 fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------
 fs/xfs/scrub/dabtree.c        |  4 +---
 5 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 3b1bd6e112f89..52e01fc0c5d04 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -750,9 +750,7 @@ xfs_attr_node_addname(
 	dp = args->dp;
 	mp = dp->i_mount;
 restart:
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = mp;
+	state = xfs_da_state_alloc(args);
 
 	/*
 	 * Search to see if name already exists, and get back a pointer
@@ -899,9 +897,8 @@ xfs_attr_node_addname(
 		 * attr, not the "new" one.
 		 */
 		args->attr_filter |= XFS_ATTR_INCOMPLETE;
-		state = xfs_da_state_alloc();
-		state->args = args;
-		state->mp = mp;
+		state = xfs_da_state_alloc(args);
+
 		state->inleaf = 0;
 		error = xfs_da3_node_lookup_int(state, &retval);
 		if (error)
@@ -975,9 +972,7 @@ xfs_attr_node_removename(
 	 * Tie a string around our finger to remind us where we are.
 	 */
 	dp = args->dp;
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = dp->i_mount;
+	state = xfs_da_state_alloc(args);
 
 	/*
 	 * Search to see if name exists, and get back a pointer to it.
@@ -1207,9 +1202,7 @@ xfs_attr_node_get(xfs_da_args_t *args)
 
 	trace_xfs_attr_node_get(args);
 
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = args->dp->i_mount;
+	state = xfs_da_state_alloc(args);
 
 	/*
 	 * Search to see if name exists, and get back a pointer to it.
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index a4e1f01daf3d8..47da88154b434 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -79,9 +79,13 @@ kmem_zone_t *xfs_da_state_zone;	/* anchor for state struct zone */
  * We don't put them on the stack since they're large.
  */
 xfs_da_state_t *
-xfs_da_state_alloc(void)
+xfs_da_state_alloc(xfs_da_args_t *args)
 {
-	return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL);
+	xfs_da_state_t *state = kmem_cache_zalloc(xfs_da_state_zone,
+						  GFP_NOFS | __GFP_NOFAIL);
+	state->args = args;
+	state->mp = args->dp->i_mount;
+	return state;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 6e25de6621e4f..bb039dcb0cce4 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -219,7 +219,7 @@ enum xfs_dacmp xfs_da_compname(struct xfs_da_args *args,
 				const unsigned char *name, int len);
 
 
-xfs_da_state_t *xfs_da_state_alloc(void);
+xfs_da_state_t *xfs_da_state_alloc(xfs_da_args_t *args);
 void xfs_da_state_free(xfs_da_state_t *state);
 
 void	xfs_da3_node_hdr_from_disk(struct xfs_mount *mp,
diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
index 6ac4aad98cd76..5d51265d29d6f 100644
--- a/fs/xfs/libxfs/xfs_dir2_node.c
+++ b/fs/xfs/libxfs/xfs_dir2_node.c
@@ -2015,9 +2015,7 @@ xfs_dir2_node_addname(
 	/*
 	 * Allocate and initialize the state (btree cursor).
 	 */
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = args->dp->i_mount;
+	state = xfs_da_state_alloc(args);
 	/*
 	 * Look up the name.  We're not supposed to find it, but
 	 * this gives us the insertion point.
@@ -2086,9 +2084,8 @@ xfs_dir2_node_lookup(
 	/*
 	 * Allocate and initialize the btree cursor.
 	 */
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = args->dp->i_mount;
+	state = xfs_da_state_alloc(args);
+
 	/*
 	 * Fill in the path to the entry in the cursor.
 	 */
@@ -2139,9 +2136,7 @@ xfs_dir2_node_removename(
 	/*
 	 * Allocate and initialize the btree cursor.
 	 */
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = args->dp->i_mount;
+	state = xfs_da_state_alloc(args);
 
 	/* Look up the entry we're deleting, set up the cursor. */
 	error = xfs_da3_node_lookup_int(state, &rval);
@@ -2206,9 +2201,7 @@ xfs_dir2_node_replace(
 	/*
 	 * Allocate and initialize the btree cursor.
 	 */
-	state = xfs_da_state_alloc();
-	state->args = args;
-	state->mp = args->dp->i_mount;
+	state = xfs_da_state_alloc(args);
 
 	/*
 	 * We have to save new inode number and ftype since
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index 44b15015021f3..e56786f0a13c8 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -476,9 +476,7 @@ xchk_da_btree(
 	ds.dargs.whichfork = whichfork;
 	ds.dargs.trans = sc->tp;
 	ds.dargs.op_flags = XFS_DA_OP_OKNOENT;
-	ds.state = xfs_da_state_alloc();
-	ds.state->args = &ds.dargs;
-	ds.state->mp = mp;
+	ds.state = xfs_da_state_alloc(&ds.dargs);
 	ds.sc = sc;
 	ds.private = private;
 	if (whichfork == XFS_ATTR_FORK) {
-- 
2.26.2

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

[Christoph Hellwig]

On Wed, Jul 22, 2020 at 11:05:14AM +0200, Carlos Maiolino wrote:
> -	/*
> -	 * 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);
> +

I would have kept a version of this comment.  But if this on your
radar for the next merge window anyway this should be ok:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 5/5] xfs: Refactor xfs_da_state_alloc() helper

[Christoph Hellwig]

On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote:
> Every call to xfs_da_state_alloc() also requires setting up state->args
> and state->mp
> 
> Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and
> return a xfs_da_state_t with both args and mp already set.

I would have not use the typedef versions and switched to the raw
struct type, but that's something we should eventually take care of
with a bulk cleanup anyway, so:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Carlos Maiolino]

On Wed, Jul 22, 2020 at 03:13:46PM +0100, Christoph Hellwig wrote:
> On Wed, Jul 22, 2020 at 11:05:14AM +0200, Carlos Maiolino wrote:
> > -	/*
> > -	 * 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);
> > +
> 
> I would have kept a version of this comment.  But if this on your
> radar for the next merge window anyway this should be ok:

I thought it makes no sense to keep the comment, and yes, I'll keep an eye on it
and rework this as soon as we have PF_FSTRANS back.

Thanks for the review.

> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 

-- 
Carlos

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

[Dave Chinner]

On Wed, Jul 22, 2020 at 11:05:14AM +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.
> 
> 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>
> ---
> 
> Changelog:
> 	V2:
> 		- Wire up xfs_inode_alloc to use __GFP_NOFAIL
> 		  if it's called inside a transaction
> 		- Rewrite changelog in a more decent way.
> 	V3:
> 		- Use __GFP_NOFAIL unconditionally in xfs_inode_alloc(),
> 		  use of PF_FSTRANS will be added when the patch re-adding
> 		  it is moved to mainline.
> 
>  fs/xfs/libxfs/xfs_alloc.c |  3 ++-
>  fs/xfs/libxfs/xfs_bmap.c  |  3 ++-
>  fs/xfs/xfs_icache.c       | 10 ++--------
>  3 files changed, 6 insertions(+), 10 deletions(-)

Looks fine with or without the comment as christoph noted.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 2/5] xfs: Remove kmem_zone_zalloc() usage

[Dave Chinner]

On Wed, Jul 22, 2020 at 11:05:15AM +0200, Carlos Maiolino wrote:
> Use kmem_cache_zalloc() directly.
> 
> With the exception of xlog_ticket_alloc() which will be dealt on the
> next patch for readability.
> 
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks fine.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 3/5] xfs: Modify xlog_ticket_alloc() to use kernel's MM API

[Dave Chinner]

On Wed, Jul 22, 2020 at 11:05:16AM +0200, Carlos Maiolino wrote:
> xlog_ticket_alloc() is always called under NOFS context, except from
> unmount path, which eitherway is holding many FS locks, so, there is no
> need for its callers to keep passing allocation flags into it.
> 
> change xlog_ticket_alloc() to use default kmem_cache_zalloc(), remove
> its alloc_flags argument, and always use GFP_NOFS | __GFP_NOFAIL flags.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> Changelog:
> 	V2:
> 		- Remove alloc_flags argument from xlog_ticket_alloc()
> 		  and update patch description accordingly.
> 
>  fs/xfs/xfs_log.c      | 9 +++------
>  fs/xfs/xfs_log_cil.c  | 3 +--
>  fs/xfs/xfs_log_priv.h | 4 +---
>  3 files changed, 5 insertions(+), 11 deletions(-)

Looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 5/5] xfs: Refactor xfs_da_state_alloc() helper

[Dave Chinner]

On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote:
> Every call to xfs_da_state_alloc() also requires setting up state->args
> and state->mp
> 
> Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and
> return a xfs_da_state_t with both args and mp already set.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> Changelog:
> 	V3:
> 		- Originally this patch removed xfs_da_state_alloc(),
> 		  per hch's suggestion, instead of removing, it has been
> 		  refactored, to also set state->{args,mp} which removes
> 		  a few lines of code.
> 
>  fs/xfs/libxfs/xfs_attr.c      | 17 +++++------------
>  fs/xfs/libxfs/xfs_da_btree.c  |  8 ++++++--
>  fs/xfs/libxfs/xfs_da_btree.h  |  2 +-
>  fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------
>  fs/xfs/scrub/dabtree.c        |  4 +---
>  5 files changed, 18 insertions(+), 30 deletions(-)

With the typedefs fixed,

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

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

[Darrick J. Wong]

On Wed, Jul 22, 2020 at 11:05:14AM +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.
> 
> 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>

Looks ok to me too (I put the comment back in),
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Changelog:
> 	V2:
> 		- Wire up xfs_inode_alloc to use __GFP_NOFAIL
> 		  if it's called inside a transaction
> 		- Rewrite changelog in a more decent way.
> 	V3:
> 		- Use __GFP_NOFAIL unconditionally in xfs_inode_alloc(),
> 		  use of PF_FSTRANS will be added when the patch re-adding
> 		  it is moved to mainline.
> 
>  fs/xfs/libxfs/xfs_alloc.c |  3 ++-
>  fs/xfs/libxfs/xfs_bmap.c  |  3 ++-
>  fs/xfs/xfs_icache.c       | 10 ++--------
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> 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 58a750ce689c0..c2d97e4f131fb 100644
> --- a/fs/xfs/xfs_icache.c
> +++ b/fs/xfs/xfs_icache.c
> @@ -36,14 +36,8 @@ 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;
> -- 
> 2.26.2
> 

Re: [PATCH 2/5] xfs: Remove kmem_zone_zalloc() usage

[Darrick J. Wong]

On Wed, Jul 22, 2020 at 11:05:15AM +0200, Carlos Maiolino wrote:
> Use kmem_cache_zalloc() directly.
> 
> With the exception of xlog_ticket_alloc() which will be dealt on the
> next patch for readability.
> 
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Changelog:
> 	V2:
> 		- Remove comment left by mistake on previous patch
> 		- Keep the same logic in _xfs_buf_alloc, not allowing
> 		  its allocation to fail
> 		- Fix some line breaks keeping them inside the 80char
> 		  rule.
> 
>  fs/xfs/libxfs/xfs_alloc_btree.c    | 2 +-
>  fs/xfs/libxfs/xfs_bmap.c           | 5 ++++-
>  fs/xfs/libxfs/xfs_bmap_btree.c     | 2 +-
>  fs/xfs/libxfs/xfs_da_btree.c       | 2 +-
>  fs/xfs/libxfs/xfs_ialloc_btree.c   | 2 +-
>  fs/xfs/libxfs/xfs_inode_fork.c     | 6 +++---
>  fs/xfs/libxfs/xfs_refcount_btree.c | 2 +-
>  fs/xfs/libxfs/xfs_rmap_btree.c     | 2 +-
>  fs/xfs/xfs_bmap_item.c             | 4 ++--
>  fs/xfs/xfs_buf.c                   | 4 +---
>  fs/xfs/xfs_buf_item.c              | 2 +-
>  fs/xfs/xfs_dquot.c                 | 2 +-
>  fs/xfs/xfs_extfree_item.c          | 6 ++++--
>  fs/xfs/xfs_icreate_item.c          | 2 +-
>  fs/xfs/xfs_inode_item.c            | 3 ++-
>  fs/xfs/xfs_refcount_item.c         | 5 +++--
>  fs/xfs/xfs_rmap_item.c             | 5 +++--
>  fs/xfs/xfs_trans.c                 | 4 ++--
>  fs/xfs/xfs_trans_dquot.c           | 3 ++-
>  19 files changed, 35 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
> index 60c453cb3ee37..57f8b16a6ea44 100644
> --- a/fs/xfs/libxfs/xfs_alloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_alloc_btree.c
> @@ -484,7 +484,7 @@ xfs_allocbt_init_common(
>  
>  	ASSERT(btnum == XFS_BTNUM_BNO || btnum == XFS_BTNUM_CNT);
>  
> -	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
> +	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
>  
>  	cur->bc_tp = tp;
>  	cur->bc_mp = mp;
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index fd5c0d669d0d7..9c40d59710357 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1099,7 +1099,10 @@ xfs_bmap_add_attrfork(
>  	if (error)
>  		goto trans_cancel;
>  	ASSERT(ip->i_afp == NULL);
> -	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, 0);
> +
> +	ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone,
> +				      GFP_KERNEL | __GFP_NOFAIL);
> +
>  	ip->i_afp->if_format = XFS_DINODE_FMT_EXTENTS;
>  	ip->i_afp->if_flags = XFS_IFEXTENTS;
>  	logflags = 0;
> diff --git a/fs/xfs/libxfs/xfs_bmap_btree.c b/fs/xfs/libxfs/xfs_bmap_btree.c
> index d9c63f17d2dec..ecec604e6e4d7 100644
> --- a/fs/xfs/libxfs/xfs_bmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -552,7 +552,7 @@ xfs_bmbt_init_cursor(
>  	struct xfs_btree_cur	*cur;
>  	ASSERT(whichfork != XFS_COW_FORK);
>  
> -	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
> +	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
>  
>  	cur->bc_tp = tp;
>  	cur->bc_mp = mp;
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 897749c41f36e..a4e1f01daf3d8 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -81,7 +81,7 @@ kmem_zone_t *xfs_da_state_zone;	/* anchor for state struct zone */
>  xfs_da_state_t *
>  xfs_da_state_alloc(void)
>  {
> -	return kmem_zone_zalloc(xfs_da_state_zone, KM_NOFS);
> +	return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL);
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_ialloc_btree.c b/fs/xfs/libxfs/xfs_ialloc_btree.c
> index b2c122ad8f0e9..3c8aebc36e643 100644
> --- a/fs/xfs/libxfs/xfs_ialloc_btree.c
> +++ b/fs/xfs/libxfs/xfs_ialloc_btree.c
> @@ -411,7 +411,7 @@ xfs_inobt_init_common(
>  {
>  	struct xfs_btree_cur	*cur;
>  
> -	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
> +	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
>  	cur->bc_tp = tp;
>  	cur->bc_mp = mp;
>  	cur->bc_btnum = btnum;
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 28b366275ae0e..0cf853d42d622 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -291,7 +291,7 @@ xfs_iformat_attr_fork(
>  	 * Initialize the extent count early, as the per-format routines may
>  	 * depend on it.
>  	 */
> -	ip->i_afp = kmem_zone_zalloc(xfs_ifork_zone, KM_NOFS);
> +	ip->i_afp = kmem_cache_zalloc(xfs_ifork_zone, GFP_NOFS | __GFP_NOFAIL);
>  	ip->i_afp->if_format = dip->di_aformat;
>  	if (unlikely(ip->i_afp->if_format == 0)) /* pre IRIX 6.2 file system */
>  		ip->i_afp->if_format = XFS_DINODE_FMT_EXTENTS;
> @@ -673,8 +673,8 @@ xfs_ifork_init_cow(
>  	if (ip->i_cowfp)
>  		return;
>  
> -	ip->i_cowfp = kmem_zone_zalloc(xfs_ifork_zone,
> -				       KM_NOFS);
> +	ip->i_cowfp = kmem_cache_zalloc(xfs_ifork_zone,
> +				       GFP_NOFS | __GFP_NOFAIL);
>  	ip->i_cowfp->if_flags = XFS_IFEXTENTS;
>  	ip->i_cowfp->if_format = XFS_DINODE_FMT_EXTENTS;
>  }
> diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
> index 7fd6044a4f780..fde21b5e82ed0 100644
> --- a/fs/xfs/libxfs/xfs_refcount_btree.c
> +++ b/fs/xfs/libxfs/xfs_refcount_btree.c
> @@ -325,7 +325,7 @@ xfs_refcountbt_init_common(
>  	ASSERT(agno != NULLAGNUMBER);
>  	ASSERT(agno < mp->m_sb.sb_agcount);
>  
> -	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
> +	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
>  	cur->bc_tp = tp;
>  	cur->bc_mp = mp;
>  	cur->bc_btnum = XFS_BTNUM_REFC;
> diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
> index b7c05314d07c9..caf0d799c1f42 100644
> --- a/fs/xfs/libxfs/xfs_rmap_btree.c
> +++ b/fs/xfs/libxfs/xfs_rmap_btree.c
> @@ -457,7 +457,7 @@ xfs_rmapbt_init_common(
>  {
>  	struct xfs_btree_cur	*cur;
>  
> -	cur = kmem_zone_zalloc(xfs_btree_cur_zone, KM_NOFS);
> +	cur = kmem_cache_zalloc(xfs_btree_cur_zone, GFP_NOFS | __GFP_NOFAIL);
>  	cur->bc_tp = tp;
>  	cur->bc_mp = mp;
>  	/* Overlapping btree; 2 keys per pointer. */
> diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
> index 6736c5ab188f2..ec3691372e7c0 100644
> --- a/fs/xfs/xfs_bmap_item.c
> +++ b/fs/xfs/xfs_bmap_item.c
> @@ -138,7 +138,7 @@ xfs_bui_init(
>  {
>  	struct xfs_bui_log_item		*buip;
>  
> -	buip = kmem_zone_zalloc(xfs_bui_zone, 0);
> +	buip = kmem_cache_zalloc(xfs_bui_zone, GFP_KERNEL | __GFP_NOFAIL);
>  
>  	xfs_log_item_init(mp, &buip->bui_item, XFS_LI_BUI, &xfs_bui_item_ops);
>  	buip->bui_format.bui_nextents = XFS_BUI_MAX_FAST_EXTENTS;
> @@ -215,7 +215,7 @@ xfs_trans_get_bud(
>  {
>  	struct xfs_bud_log_item		*budp;
>  
> -	budp = kmem_zone_zalloc(xfs_bud_zone, 0);
> +	budp = kmem_cache_zalloc(xfs_bud_zone, GFP_KERNEL | __GFP_NOFAIL);
>  	xfs_log_item_init(tp->t_mountp, &budp->bud_item, XFS_LI_BUD,
>  			  &xfs_bud_item_ops);
>  	budp->bud_buip = buip;
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index dda0c94458797..d4cdcb6fb2fe1 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -214,9 +214,7 @@ _xfs_buf_alloc(
>  	int			i;
>  
>  	*bpp = NULL;
> -	bp = kmem_zone_zalloc(xfs_buf_zone, KM_NOFS);
> -	if (unlikely(!bp))
> -		return -ENOMEM;
> +	bp = kmem_cache_zalloc(xfs_buf_zone, GFP_NOFS | __GFP_NOFAIL);
>  
>  	/*
>  	 * We don't want certain flags to appear in b_flags unless they are
> diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
> index e9428c30862a9..3eb45782cf923 100644
> --- a/fs/xfs/xfs_buf_item.c
> +++ b/fs/xfs/xfs_buf_item.c
> @@ -739,7 +739,7 @@ xfs_buf_item_init(
>  		return 0;
>  	}
>  
> -	bip = kmem_zone_zalloc(xfs_buf_item_zone, 0);
> +	bip = kmem_cache_zalloc(xfs_buf_item_zone, GFP_KERNEL | __GFP_NOFAIL);
>  	xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF, &xfs_buf_item_ops);
>  	bip->bli_buf = bp;
>  
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index 76353c9a723ee..0f1a0de761787 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -475,7 +475,7 @@ xfs_dquot_alloc(
>  {
>  	struct xfs_dquot	*dqp;
>  
> -	dqp = kmem_zone_zalloc(xfs_qm_dqzone, 0);
> +	dqp = kmem_cache_zalloc(xfs_qm_dqzone, GFP_KERNEL | __GFP_NOFAIL);
>  
>  	dqp->dq_flags = type;
>  	dqp->q_core.d_id = cpu_to_be32(id);
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index b9c333bae0a12..6cb8cd11072a3 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -161,7 +161,8 @@ xfs_efi_init(
>  			((nextents - 1) * sizeof(xfs_extent_t)));
>  		efip = kmem_zalloc(size, 0);
>  	} else {
> -		efip = kmem_zone_zalloc(xfs_efi_zone, 0);
> +		efip = kmem_cache_zalloc(xfs_efi_zone,
> +					 GFP_KERNEL | __GFP_NOFAIL);
>  	}
>  
>  	xfs_log_item_init(mp, &efip->efi_item, XFS_LI_EFI, &xfs_efi_item_ops);
> @@ -332,7 +333,8 @@ xfs_trans_get_efd(
>  				(nextents - 1) * sizeof(struct xfs_extent),
>  				0);
>  	} else {
> -		efdp = kmem_zone_zalloc(xfs_efd_zone, 0);
> +		efdp = kmem_cache_zalloc(xfs_efd_zone,
> +					GFP_KERNEL | __GFP_NOFAIL);
>  	}
>  
>  	xfs_log_item_init(tp->t_mountp, &efdp->efd_item, XFS_LI_EFD,
> diff --git a/fs/xfs/xfs_icreate_item.c b/fs/xfs/xfs_icreate_item.c
> index 287a9e5c7d758..9b3994b9c716d 100644
> --- a/fs/xfs/xfs_icreate_item.c
> +++ b/fs/xfs/xfs_icreate_item.c
> @@ -97,7 +97,7 @@ xfs_icreate_log(
>  {
>  	struct xfs_icreate_item	*icp;
>  
> -	icp = kmem_zone_zalloc(xfs_icreate_zone, 0);
> +	icp = kmem_cache_zalloc(xfs_icreate_zone, GFP_KERNEL | __GFP_NOFAIL);
>  
>  	xfs_log_item_init(tp->t_mountp, &icp->ic_item, XFS_LI_ICREATE,
>  			  &xfs_icreate_item_ops);
> diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
> index 3840117f8a5e2..895f61b2b4f01 100644
> --- a/fs/xfs/xfs_inode_item.c
> +++ b/fs/xfs/xfs_inode_item.c
> @@ -615,7 +615,8 @@ xfs_inode_item_init(
>  	struct xfs_inode_log_item *iip;
>  
>  	ASSERT(ip->i_itemp == NULL);
> -	iip = ip->i_itemp = kmem_zone_zalloc(xfs_ili_zone, 0);
> +	iip = ip->i_itemp = kmem_cache_zalloc(xfs_ili_zone,
> +					      GFP_KERNEL | __GFP_NOFAIL);
>  
>  	iip->ili_inode = ip;
>  	spin_lock_init(&iip->ili_lock);
> diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
> index c81639891e298..7b2c72bc28582 100644
> --- a/fs/xfs/xfs_refcount_item.c
> +++ b/fs/xfs/xfs_refcount_item.c
> @@ -143,7 +143,8 @@ xfs_cui_init(
>  		cuip = kmem_zalloc(xfs_cui_log_item_sizeof(nextents),
>  				0);
>  	else
> -		cuip = kmem_zone_zalloc(xfs_cui_zone, 0);
> +		cuip = kmem_cache_zalloc(xfs_cui_zone,
> +					 GFP_KERNEL | __GFP_NOFAIL);
>  
>  	xfs_log_item_init(mp, &cuip->cui_item, XFS_LI_CUI, &xfs_cui_item_ops);
>  	cuip->cui_format.cui_nextents = nextents;
> @@ -220,7 +221,7 @@ xfs_trans_get_cud(
>  {
>  	struct xfs_cud_log_item		*cudp;
>  
> -	cudp = kmem_zone_zalloc(xfs_cud_zone, 0);
> +	cudp = kmem_cache_zalloc(xfs_cud_zone, GFP_KERNEL | __GFP_NOFAIL);
>  	xfs_log_item_init(tp->t_mountp, &cudp->cud_item, XFS_LI_CUD,
>  			  &xfs_cud_item_ops);
>  	cudp->cud_cuip = cuip;
> diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
> index a86599db20a6f..dc5b0753cd519 100644
> --- a/fs/xfs/xfs_rmap_item.c
> +++ b/fs/xfs/xfs_rmap_item.c
> @@ -141,7 +141,8 @@ xfs_rui_init(
>  	if (nextents > XFS_RUI_MAX_FAST_EXTENTS)
>  		ruip = kmem_zalloc(xfs_rui_log_item_sizeof(nextents), 0);
>  	else
> -		ruip = kmem_zone_zalloc(xfs_rui_zone, 0);
> +		ruip = kmem_cache_zalloc(xfs_rui_zone,
> +					 GFP_KERNEL | __GFP_NOFAIL);
>  
>  	xfs_log_item_init(mp, &ruip->rui_item, XFS_LI_RUI, &xfs_rui_item_ops);
>  	ruip->rui_format.rui_nextents = nextents;
> @@ -243,7 +244,7 @@ xfs_trans_get_rud(
>  {
>  	struct xfs_rud_log_item		*rudp;
>  
> -	rudp = kmem_zone_zalloc(xfs_rud_zone, 0);
> +	rudp = kmem_cache_zalloc(xfs_rud_zone, GFP_KERNEL | __GFP_NOFAIL);
>  	xfs_log_item_init(tp->t_mountp, &rudp->rud_item, XFS_LI_RUD,
>  			  &xfs_rud_item_ops);
>  	rudp->rud_ruip = ruip;
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 0ad72a83edac4..ed72867b1a193 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -90,7 +90,7 @@ xfs_trans_dup(
>  
>  	trace_xfs_trans_dup(tp, _RET_IP_);
>  
> -	ntp = kmem_zone_zalloc(xfs_trans_zone, 0);
> +	ntp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
>  
>  	/*
>  	 * Initialize the new transaction structure.
> @@ -263,7 +263,7 @@ xfs_trans_alloc(
>  	 * GFP_NOFS allocation context so that we avoid lockdep false positives
>  	 * by doing GFP_KERNEL allocations inside sb_start_intwrite().
>  	 */
> -	tp = kmem_zone_zalloc(xfs_trans_zone, 0);
> +	tp = kmem_cache_zalloc(xfs_trans_zone, GFP_KERNEL | __GFP_NOFAIL);
>  	if (!(flags & XFS_TRANS_NO_WRITECOUNT))
>  		sb_start_intwrite(mp->m_super);
>  
> diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c
> index c0f73b82c0551..394d6a0aa18dc 100644
> --- a/fs/xfs/xfs_trans_dquot.c
> +++ b/fs/xfs/xfs_trans_dquot.c
> @@ -860,7 +860,8 @@ STATIC void
>  xfs_trans_alloc_dqinfo(
>  	xfs_trans_t	*tp)
>  {
> -	tp->t_dqinfo = kmem_zone_zalloc(xfs_qm_dqtrxzone, 0);
> +	tp->t_dqinfo = kmem_cache_zalloc(xfs_qm_dqtrxzone,
> +					 GFP_KERNEL | __GFP_NOFAIL);
>  }
>  
>  void
> -- 
> 2.26.2
> 

Re: [PATCH 3/5] xfs: Modify xlog_ticket_alloc() to use kernel's MM API

[Darrick J. Wong]

On Wed, Jul 22, 2020 at 11:05:16AM +0200, Carlos Maiolino wrote:
> xlog_ticket_alloc() is always called under NOFS context, except from
> unmount path, which eitherway is holding many FS locks, so, there is no
> need for its callers to keep passing allocation flags into it.
> 
> change xlog_ticket_alloc() to use default kmem_cache_zalloc(), remove
> its alloc_flags argument, and always use GFP_NOFS | __GFP_NOFAIL flags.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks decent,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Changelog:
> 	V2:
> 		- Remove alloc_flags argument from xlog_ticket_alloc()
> 		  and update patch description accordingly.
> 
>  fs/xfs/xfs_log.c      | 9 +++------
>  fs/xfs/xfs_log_cil.c  | 3 +--
>  fs/xfs/xfs_log_priv.h | 4 +---
>  3 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
> index 00fda2e8e7380..ad0c69ee89475 100644
> --- a/fs/xfs/xfs_log.c
> +++ b/fs/xfs/xfs_log.c
> @@ -433,7 +433,7 @@ xfs_log_reserve(
>  	XFS_STATS_INC(mp, xs_try_logspace);
>  
>  	ASSERT(*ticp == NULL);
> -	tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent, 0);
> +	tic = xlog_ticket_alloc(log, unit_bytes, cnt, client, permanent);
>  	*ticp = tic;
>  
>  	xlog_grant_push_ail(log, tic->t_cnt ? tic->t_unit_res * tic->t_cnt
> @@ -3408,15 +3408,12 @@ xlog_ticket_alloc(
>  	int			unit_bytes,
>  	int			cnt,
>  	char			client,
> -	bool			permanent,
> -	xfs_km_flags_t		alloc_flags)
> +	bool			permanent)
>  {
>  	struct xlog_ticket	*tic;
>  	int			unit_res;
>  
> -	tic = kmem_zone_zalloc(xfs_log_ticket_zone, alloc_flags);
> -	if (!tic)
> -		return NULL;
> +	tic = kmem_cache_zalloc(xfs_log_ticket_zone, GFP_NOFS | __GFP_NOFAIL);
>  
>  	unit_res = xfs_log_calc_unit_res(log->l_mp, unit_bytes);
>  
> diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
> index 9ed90368ab311..56c32eecffead 100644
> --- a/fs/xfs/xfs_log_cil.c
> +++ b/fs/xfs/xfs_log_cil.c
> @@ -37,8 +37,7 @@ xlog_cil_ticket_alloc(
>  {
>  	struct xlog_ticket *tic;
>  
> -	tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0,
> -				KM_NOFS);
> +	tic = xlog_ticket_alloc(log, 0, 1, XFS_TRANSACTION, 0);
>  
>  	/*
>  	 * set the current reservation to zero so we know to steal the basic
> diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
> index 75a62870b63af..1c6fdbf3d5066 100644
> --- a/fs/xfs/xfs_log_priv.h
> +++ b/fs/xfs/xfs_log_priv.h
> @@ -464,9 +464,7 @@ xlog_ticket_alloc(
>  	int		unit_bytes,
>  	int		count,
>  	char		client,
> -	bool		permanent,
> -	xfs_km_flags_t	alloc_flags);
> -
> +	bool		permanent);
>  
>  static inline void
>  xlog_write_adv_cnt(void **ptr, int *len, int *off, size_t bytes)
> -- 
> 2.26.2
> 

Re: [PATCH 4/5] xfs: remove xfs_zone_{alloc,zalloc} helpers

[Darrick J. Wong]

On Wed, Jul 22, 2020 at 11:05:17AM +0200, Carlos Maiolino wrote:
> All their users have been converted to use MM API directly, no need to
> keep them around anymore.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Neato!!!!
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/kmem.c      | 21 ---------------------
>  fs/xfs/kmem.h      |  8 --------
>  fs/xfs/xfs_trace.h |  1 -
>  3 files changed, 30 deletions(-)
> 
> diff --git a/fs/xfs/kmem.c b/fs/xfs/kmem.c
> index f1366475c389c..e841ed781a257 100644
> --- a/fs/xfs/kmem.c
> +++ b/fs/xfs/kmem.c
> @@ -115,24 +115,3 @@ kmem_realloc(const void *old, size_t newsize, xfs_km_flags_t flags)
>  		congestion_wait(BLK_RW_ASYNC, HZ/50);
>  	} while (1);
>  }
> -
> -void *
> -kmem_zone_alloc(kmem_zone_t *zone, xfs_km_flags_t flags)
> -{
> -	int	retries = 0;
> -	gfp_t	lflags = kmem_flags_convert(flags);
> -	void	*ptr;
> -
> -	trace_kmem_zone_alloc(kmem_cache_size(zone), flags, _RET_IP_);
> -	do {
> -		ptr = kmem_cache_alloc(zone, lflags);
> -		if (ptr || (flags & KM_MAYFAIL))
> -			return ptr;
> -		if (!(++retries % 100))
> -			xfs_err(NULL,
> -		"%s(%u) possible memory allocation deadlock in %s (mode:0x%x)",
> -				current->comm, current->pid,
> -				__func__, lflags);
> -		congestion_wait(BLK_RW_ASYNC, HZ/50);
> -	} while (1);
> -}
> diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h
> index 34cbcfde92281..8e8555817e6d3 100644
> --- a/fs/xfs/kmem.h
> +++ b/fs/xfs/kmem.h
> @@ -85,14 +85,6 @@ kmem_zalloc_large(size_t size, xfs_km_flags_t flags)
>  #define kmem_zone	kmem_cache
>  #define kmem_zone_t	struct kmem_cache
>  
> -extern void *kmem_zone_alloc(kmem_zone_t *, xfs_km_flags_t);
> -
> -static inline void *
> -kmem_zone_zalloc(kmem_zone_t *zone, xfs_km_flags_t flags)
> -{
> -	return kmem_zone_alloc(zone, flags | KM_ZERO);
> -}
> -
>  static inline struct page *
>  kmem_to_page(void *addr)
>  {
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 50c478374a31b..627fe4fef9ac7 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -3536,7 +3536,6 @@ DEFINE_KMEM_EVENT(kmem_alloc);
>  DEFINE_KMEM_EVENT(kmem_alloc_io);
>  DEFINE_KMEM_EVENT(kmem_alloc_large);
>  DEFINE_KMEM_EVENT(kmem_realloc);
> -DEFINE_KMEM_EVENT(kmem_zone_alloc);
>  
>  TRACE_EVENT(xfs_check_new_dalign,
>  	TP_PROTO(struct xfs_mount *mp, int new_dalign, xfs_ino_t calc_rootino),
> -- 
> 2.26.2
> 

Re: [PATCH 5/5] xfs: Refactor xfs_da_state_alloc() helper

[Darrick J. Wong]

On Wed, Jul 22, 2020 at 11:05:18AM +0200, Carlos Maiolino wrote:
> Every call to xfs_da_state_alloc() also requires setting up state->args
> and state->mp
> 
> Change xfs_da_state_alloc() to receive an xfs_da_args_t as argument and
> return a xfs_da_state_t with both args and mp already set.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Will fix the typedef thing for xfs_da_state_alloc when I commit this;
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> Changelog:
> 	V3:
> 		- Originally this patch removed xfs_da_state_alloc(),
> 		  per hch's suggestion, instead of removing, it has been
> 		  refactored, to also set state->{args,mp} which removes
> 		  a few lines of code.
> 
>  fs/xfs/libxfs/xfs_attr.c      | 17 +++++------------
>  fs/xfs/libxfs/xfs_da_btree.c  |  8 ++++++--
>  fs/xfs/libxfs/xfs_da_btree.h  |  2 +-
>  fs/xfs/libxfs/xfs_dir2_node.c | 17 +++++------------
>  fs/xfs/scrub/dabtree.c        |  4 +---
>  5 files changed, 18 insertions(+), 30 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 3b1bd6e112f89..52e01fc0c5d04 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -750,9 +750,7 @@ xfs_attr_node_addname(
>  	dp = args->dp;
>  	mp = dp->i_mount;
>  restart:
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = mp;
> +	state = xfs_da_state_alloc(args);
>  
>  	/*
>  	 * Search to see if name already exists, and get back a pointer
> @@ -899,9 +897,8 @@ xfs_attr_node_addname(
>  		 * attr, not the "new" one.
>  		 */
>  		args->attr_filter |= XFS_ATTR_INCOMPLETE;
> -		state = xfs_da_state_alloc();
> -		state->args = args;
> -		state->mp = mp;
> +		state = xfs_da_state_alloc(args);
> +
>  		state->inleaf = 0;
>  		error = xfs_da3_node_lookup_int(state, &retval);
>  		if (error)
> @@ -975,9 +972,7 @@ xfs_attr_node_removename(
>  	 * Tie a string around our finger to remind us where we are.
>  	 */
>  	dp = args->dp;
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = dp->i_mount;
> +	state = xfs_da_state_alloc(args);
>  
>  	/*
>  	 * Search to see if name exists, and get back a pointer to it.
> @@ -1207,9 +1202,7 @@ xfs_attr_node_get(xfs_da_args_t *args)
>  
>  	trace_xfs_attr_node_get(args);
>  
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = args->dp->i_mount;
> +	state = xfs_da_state_alloc(args);
>  
>  	/*
>  	 * Search to see if name exists, and get back a pointer to it.
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index a4e1f01daf3d8..47da88154b434 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -79,9 +79,13 @@ kmem_zone_t *xfs_da_state_zone;	/* anchor for state struct zone */
>   * We don't put them on the stack since they're large.
>   */
>  xfs_da_state_t *
> -xfs_da_state_alloc(void)
> +xfs_da_state_alloc(xfs_da_args_t *args)
>  {
> -	return kmem_cache_zalloc(xfs_da_state_zone, GFP_NOFS | __GFP_NOFAIL);
> +	xfs_da_state_t *state = kmem_cache_zalloc(xfs_da_state_zone,
> +						  GFP_NOFS | __GFP_NOFAIL);
> +	state->args = args;
> +	state->mp = args->dp->i_mount;
> +	return state;
>  }
>  
>  /*
> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
> index 6e25de6621e4f..bb039dcb0cce4 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.h
> +++ b/fs/xfs/libxfs/xfs_da_btree.h
> @@ -219,7 +219,7 @@ enum xfs_dacmp xfs_da_compname(struct xfs_da_args *args,
>  				const unsigned char *name, int len);
>  
>  
> -xfs_da_state_t *xfs_da_state_alloc(void);
> +xfs_da_state_t *xfs_da_state_alloc(xfs_da_args_t *args);
>  void xfs_da_state_free(xfs_da_state_t *state);
>  
>  void	xfs_da3_node_hdr_from_disk(struct xfs_mount *mp,
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 6ac4aad98cd76..5d51265d29d6f 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -2015,9 +2015,7 @@ xfs_dir2_node_addname(
>  	/*
>  	 * Allocate and initialize the state (btree cursor).
>  	 */
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = args->dp->i_mount;
> +	state = xfs_da_state_alloc(args);
>  	/*
>  	 * Look up the name.  We're not supposed to find it, but
>  	 * this gives us the insertion point.
> @@ -2086,9 +2084,8 @@ xfs_dir2_node_lookup(
>  	/*
>  	 * Allocate and initialize the btree cursor.
>  	 */
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = args->dp->i_mount;
> +	state = xfs_da_state_alloc(args);
> +
>  	/*
>  	 * Fill in the path to the entry in the cursor.
>  	 */
> @@ -2139,9 +2136,7 @@ xfs_dir2_node_removename(
>  	/*
>  	 * Allocate and initialize the btree cursor.
>  	 */
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = args->dp->i_mount;
> +	state = xfs_da_state_alloc(args);
>  
>  	/* Look up the entry we're deleting, set up the cursor. */
>  	error = xfs_da3_node_lookup_int(state, &rval);
> @@ -2206,9 +2201,7 @@ xfs_dir2_node_replace(
>  	/*
>  	 * Allocate and initialize the btree cursor.
>  	 */
> -	state = xfs_da_state_alloc();
> -	state->args = args;
> -	state->mp = args->dp->i_mount;
> +	state = xfs_da_state_alloc(args);
>  
>  	/*
>  	 * We have to save new inode number and ftype since
> diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
> index 44b15015021f3..e56786f0a13c8 100644
> --- a/fs/xfs/scrub/dabtree.c
> +++ b/fs/xfs/scrub/dabtree.c
> @@ -476,9 +476,7 @@ xchk_da_btree(
>  	ds.dargs.whichfork = whichfork;
>  	ds.dargs.trans = sc->tp;
>  	ds.dargs.op_flags = XFS_DA_OP_OKNOENT;
> -	ds.state = xfs_da_state_alloc();
> -	ds.state->args = &ds.dargs;
> -	ds.state->mp = mp;
> +	ds.state = xfs_da_state_alloc(&ds.dargs);
>  	ds.sc = sc;
>  	ds.private = private;
>  	if (whichfork == XFS_ATTR_FORK) {
> -- 
> 2.26.2
>