All of lore.kernel.org
 help / color / mirror / Atom feed
* RFC: remove the if_flags field in struct xfs_ifork
@ 2021-04-02 14:24 Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
                   ` (6 more replies)
  0 siblings, 7 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Hi all,

given how much confusion the ifork flags cause I thought we'd better
off just removing them as all the information is available from other
fields in the ifork structure anyway.

Only very lightly tested so far and thus for conceptual review.  It will
need a rebase of the attr initialization fixes anyway.

Diffstat:
 libxfs/xfs_attr.c          |   53 ++++++++++------
 libxfs/xfs_attr.h          |    1 
 libxfs/xfs_attr_leaf.c     |   13 +---
 libxfs/xfs_bmap.c          |  141 +++++++++++++--------------------------------
 libxfs/xfs_bmap.h          |    1 
 libxfs/xfs_btree_staging.c |    1 
 libxfs/xfs_dir2_block.c    |    2 
 libxfs/xfs_dir2_sf.c       |   12 +--
 libxfs/xfs_inode_fork.c    |   22 +------
 libxfs/xfs_inode_fork.h    |   14 +---
 scrub/bmap.c               |   15 +---
 scrub/symlink.c            |    2 
 xfs_aops.c                 |    3 
 xfs_attr_list.c            |    2 
 xfs_bmap_util.c            |   20 ++----
 xfs_dir2_readdir.c         |   10 +--
 xfs_dquot.c                |    8 --
 xfs_inode.c                |    9 --
 xfs_ioctl.c                |    2 
 xfs_iomap.c                |   20 ++----
 xfs_iops.c                 |    4 -
 xfs_qm.c                   |    8 --
 xfs_reflink.c              |    8 --
 xfs_symlink.c              |    6 -
 24 files changed, 141 insertions(+), 236 deletions(-)

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-05 16:33   ` Brian Foster
  2021-04-06 13:51   ` Darrick J. Wong
  2021-04-02 14:24 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
                   ` (5 subsequent siblings)
  6 siblings, 2 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Move the XFS_IFEXTENTS check from the callers into xfs_iread_extents to
simplify the code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c  | 82 ++++++++++++++++-----------------------
 fs/xfs/scrub/bmap.c       |  9 ++---
 fs/xfs/xfs_bmap_util.c    | 16 +++-----
 fs/xfs/xfs_dir2_readdir.c |  8 ++--
 fs/xfs/xfs_dquot.c        |  8 ++--
 fs/xfs/xfs_iomap.c        | 16 +++-----
 fs/xfs/xfs_qm.c           |  8 ++--
 fs/xfs/xfs_reflink.c      |  8 ++--
 8 files changed, 62 insertions(+), 93 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 5574d345d066ed..b8cab14ca8ce8d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1223,6 +1223,9 @@ xfs_iread_extents(
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
+	if (ifp->if_flags & XFS_IFEXTENTS)
+		return 0;
+
 	if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
 		error = -EFSCORRUPTED;
 		goto out;
@@ -1278,11 +1281,9 @@ xfs_bmap_first_unused(
 
 	ASSERT(xfs_ifork_has_extents(ifp));
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	lowest = max = *first_unused;
 	for_each_xfs_iext(ifp, &icur, &got) {
@@ -1330,11 +1331,9 @@ xfs_bmap_last_before(
 		return -EFSCORRUPTED;
 	}
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	if (!xfs_iext_lookup_extent_before(ip, ifp, last_block, &icur, &got))
 		*last_block = 0;
@@ -1353,11 +1352,9 @@ xfs_bmap_last_extent(
 	struct xfs_iext_cursor	icur;
 	int			error;
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	xfs_iext_last(ifp, &icur);
 	if (!xfs_iext_get_extent(ifp, &icur, rec))
@@ -3984,11 +3981,9 @@ xfs_bmapi_read(
 
 	XFS_STATS_INC(mp, xs_blk_mapr);
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(NULL, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(NULL, ip, whichfork);
+	if (error)
+		return error;
 
 	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
 		eof = true;
@@ -4468,11 +4463,9 @@ xfs_bmapi_write(
 
 	XFS_STATS_INC(mp, xs_blk_mapw);
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			goto error0;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		goto error0;
 
 	if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.icur, &bma.got))
 		eof = true;
@@ -4751,11 +4744,9 @@ xfs_bmapi_remap(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	if (xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
 		/* make sure we only reflink into a hole. */
@@ -5426,9 +5417,10 @@ __xfs_bunmapi(
 	else
 		max_len = len;
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
-	    (error = xfs_iread_extents(tp, ip, whichfork)))
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
 		return error;
+
 	if (xfs_iext_count(ifp) == 0) {
 		*rlen = 0;
 		return 0;
@@ -5914,11 +5906,9 @@ xfs_bmap_collapse_extents(
 
 	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	if (ifp->if_flags & XFS_IFBROOT) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
@@ -6031,11 +6021,9 @@ xfs_bmap_insert_extents(
 
 	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	if (ifp->if_flags & XFS_IFBROOT) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
@@ -6134,12 +6122,10 @@ xfs_bmap_split_extent(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		/* Read in all the extents */
-		error = xfs_iread_extents(tp, ip, whichfork);
-		if (error)
-			return error;
-	}
+	/* Read in all the extents */
+	error = xfs_iread_extents(tp, ip, whichfork);
+	if (error)
+		return error;
 
 	/*
 	 * If there are not extents, or split_fsb lies in a hole we are done.
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index 33559c3a4bc3d4..fb50ec9a4303a1 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -449,11 +449,10 @@ xchk_bmap_btree(
 
 	/* Load the incore bmap cache if it's not loaded. */
 	info->was_loaded = ifp->if_flags & XFS_IFEXTENTS;
-	if (!info->was_loaded) {
-		error = xfs_iread_extents(sc->tp, ip, whichfork);
-		if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
-			goto out;
-	}
+
+	error = xfs_iread_extents(sc->tp, ip, whichfork);
+	if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
+		goto out;
 
 	/* Check the btree structure. */
 	cur = xfs_bmbt_init_cursor(mp, sc->tp, ip, whichfork);
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index e7d68318e6a55c..e7f9537634f3cf 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -225,11 +225,9 @@ xfs_bmap_count_blocks(
 
 	switch (ifp->if_format) {
 	case XFS_DINODE_FMT_BTREE:
-		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-			error = xfs_iread_extents(tp, ip, whichfork);
-			if (error)
-				return error;
-		}
+		error = xfs_iread_extents(tp, ip, whichfork);
+		if (error)
+			return error;
 
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		error = xfs_btree_count_blocks(cur, &btblocks);
@@ -471,11 +469,9 @@ xfs_getbmap(
 	first_bno = bno = XFS_BB_TO_FSBT(mp, bmv->bmv_offset);
 	len = XFS_BB_TO_FSB(mp, bmv->bmv_length);
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(NULL, ip, whichfork);
-		if (error)
-			goto out_unlock_ilock;
-	}
+	error = xfs_iread_extents(NULL, ip, whichfork);
+	if (error)
+		goto out_unlock_ilock;
 
 	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
 		/*
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 66deddd5e29698..477df4869d19bd 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -258,11 +258,9 @@ xfs_dir2_leaf_readbuf(
 	int			ra_want;
 	int			error = 0;
 
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
-		if (error)
-			goto out;
-	}
+	error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
+	if (error)
+		goto out;
 
 	/*
 	 * Look for mapped directory blocks at or above the current offset.
diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index bd8379b98374f8..83c5334fe978f2 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -748,11 +748,9 @@ xfs_dq_get_next_id(
 	start = (xfs_fsblock_t)next_id / mp->m_quotainfo->qi_dqperchunk;
 
 	lock_flags = xfs_ilock_data_map_shared(quotip);
-	if (!(quotip->i_df.if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
+	if (error)
+		return error;
 
 	if (xfs_iext_lookup_extent(quotip, &quotip->i_df, start, &cur, &got)) {
 		/* contiguous chunk, bump startoff for the id calculation */
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index e17ab7f42928a5..129a0bafb46c0d 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -893,11 +893,9 @@ xfs_buffered_write_iomap_begin(
 
 	XFS_STATS_INC(mp, xs_blk_mapw);
 
-	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
-		if (error)
-			goto out_unlock;
-	}
+	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
+	if (error)
+		goto out_unlock;
 
 	/*
 	 * Search the data fork first to look up our source mapping.  We
@@ -1208,11 +1206,9 @@ xfs_seek_iomap_begin(
 		return -EIO;
 
 	lockmode = xfs_ilock_data_map_shared(ip);
-	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
-		if (error)
-			goto out_unlock;
-	}
+	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
+	if (error)
+		goto out_unlock;
 
 	if (xfs_iext_lookup_extent(ip, &ip->i_df, offset_fsb, &icur, &imap)) {
 		/*
diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
index 6fde318b9fed27..fd835a202c51eb 100644
--- a/fs/xfs/xfs_qm.c
+++ b/fs/xfs/xfs_qm.c
@@ -1165,11 +1165,9 @@ xfs_qm_dqusage_adjust(
 	if (XFS_IS_REALTIME_INODE(ip)) {
 		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
 
-		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-			error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
-			if (error)
-				goto error0;
-		}
+		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
+		if (error)
+			goto error0;
 
 		xfs_bmap_count_leaves(ifp, &rtblks);
 	}
diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
index 725c7d8e44381b..13cd62b89c4e30 100644
--- a/fs/xfs/xfs_reflink.c
+++ b/fs/xfs/xfs_reflink.c
@@ -1392,11 +1392,9 @@ xfs_reflink_inode_has_shared_extents(
 	int				error;
 
 	ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
-	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
-		if (error)
-			return error;
-	}
+	error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
+	if (error)
+		return error;
 
 	*has_shared = false;
 	found = xfs_iext_lookup_extent(ip, ifp, 0, &icur, &got);
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-05 16:33   ` Brian Foster
  2021-04-02 14:24 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

xfs_bmap_one_block is only called for the attribute fork.  Move it to
xfs_attr.c, drop the unused whichfork argument and code only executed for
the data fork and rename the result to xfs_attr_is_leaf.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr.c | 30 +++++++++++++++++++++++++-----
 fs/xfs/libxfs/xfs_attr.h |  1 +
 fs/xfs/libxfs/xfs_bmap.c | 32 --------------------------------
 fs/xfs/libxfs/xfs_bmap.h |  1 -
 fs/xfs/xfs_attr_list.c   |  2 +-
 5 files changed, 27 insertions(+), 39 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 472b3039eabbf1..0146f70b71b1e2 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -70,6 +70,26 @@ xfs_inode_hasattr(
 	return 1;
 }
 
+/*
+ * Returns true if the there is exactly only block in the attr fork, in which
+ * case the attribute fork consists of a single leaf block entry.
+ */
+bool
+xfs_attr_is_leaf(
+	struct xfs_inode	*ip)
+{
+	struct xfs_ifork	*ifp = ip->i_afp;
+	struct xfs_iext_cursor	icur;
+	struct xfs_bmbt_irec	imap;
+
+	if (ifp->if_nextents != 1 || ifp->if_format != XFS_DINODE_FMT_EXTENTS)
+		return false;
+
+	xfs_iext_first(ifp, &icur);
+	xfs_iext_get_extent(ifp, &icur, &imap);
+	return imap.br_startoff == 0 && imap.br_blockcount == 1;
+}
+
 /*========================================================================
  * Overall external interface routines.
  *========================================================================*/
@@ -89,7 +109,7 @@ xfs_attr_get_ilocked(
 
 	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
 		return xfs_attr_shortform_getvalue(args);
-	if (xfs_bmap_one_block(args->dp, XFS_ATTR_FORK))
+	if (xfs_attr_is_leaf(args->dp))
 		return xfs_attr_leaf_get(args);
 	return xfs_attr_node_get(args);
 }
@@ -293,7 +313,7 @@ xfs_attr_set_args(
 			return error;
 	}
 
-	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
+	if (xfs_attr_is_leaf(dp)) {
 		error = xfs_attr_leaf_addname(args);
 		if (error != -ENOSPC)
 			return error;
@@ -347,7 +367,7 @@ xfs_has_attr(
 		return xfs_attr_sf_findname(args, NULL, NULL);
 	}
 
-	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
+	if (xfs_attr_is_leaf(dp)) {
 		error = xfs_attr_leaf_hasname(args, &bp);
 
 		if (bp)
@@ -374,7 +394,7 @@ xfs_attr_remove_args(
 	} else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
 		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
 		error = xfs_attr_shortform_remove(args);
-	} else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
+	} else if (xfs_attr_is_leaf(dp)) {
 		error = xfs_attr_leaf_removename(args);
 	} else {
 		error = xfs_attr_node_removename(args);
@@ -1282,7 +1302,7 @@ xfs_attr_node_removename(
 	/*
 	 * If the result is small enough, push it all into the inode.
 	 */
-	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
+	if (xfs_attr_is_leaf(dp))
 		error = xfs_attr_node_shrink(args, state);
 
 out:
diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
index 3e97a935e7121f..2b1f61987a9dec 100644
--- a/fs/xfs/libxfs/xfs_attr.h
+++ b/fs/xfs/libxfs/xfs_attr.h
@@ -85,6 +85,7 @@ int xfs_attr_inactive(struct xfs_inode *dp);
 int xfs_attr_list_ilocked(struct xfs_attr_list_context *);
 int xfs_attr_list(struct xfs_attr_list_context *);
 int xfs_inode_hasattr(struct xfs_inode *ip);
+bool xfs_attr_is_leaf(struct xfs_inode *ip);
 int xfs_attr_get_ilocked(struct xfs_da_args *args);
 int xfs_attr_get(struct xfs_da_args *args);
 int xfs_attr_set(struct xfs_da_args *args);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index b8cab14ca8ce8d..e11f8faaf8898b 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1435,38 +1435,6 @@ xfs_bmap_last_offset(
 	return 0;
 }
 
-/*
- * Returns whether the selected fork of the inode has exactly one
- * block or not.  For the data fork we check this matches di_size,
- * implying the file's range is 0..bsize-1.
- */
-int					/* 1=>1 block, 0=>otherwise */
-xfs_bmap_one_block(
-	struct xfs_inode	*ip,		/* incore inode */
-	int			whichfork)	/* data or attr fork */
-{
-	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
-	int			rval;		/* return value */
-	struct xfs_bmbt_irec	s;		/* internal version of extent */
-	struct xfs_iext_cursor icur;
-
-#ifndef DEBUG
-	if (whichfork == XFS_DATA_FORK)
-		return XFS_ISIZE(ip) == ip->i_mount->m_sb.sb_blocksize;
-#endif	/* !DEBUG */
-	if (ifp->if_nextents != 1)
-		return 0;
-	if (ifp->if_format != XFS_DINODE_FMT_EXTENTS)
-		return 0;
-	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
-	xfs_iext_first(ifp, &icur);
-	xfs_iext_get_extent(ifp, &icur, &s);
-	rval = s.br_startoff == 0 && s.br_blockcount == 1;
-	if (rval && whichfork == XFS_DATA_FORK)
-		ASSERT(XFS_ISIZE(ip) == ip->i_mount->m_sb.sb_blocksize);
-	return rval;
-}
-
 /*
  * Extent tree manipulation functions used during allocation.
  */
diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
index 6747e97a794901..59fa4834a761fe 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -199,7 +199,6 @@ int	xfs_bmap_last_before(struct xfs_trans *tp, struct xfs_inode *ip,
 		xfs_fileoff_t *last_block, int whichfork);
 int	xfs_bmap_last_offset(struct xfs_inode *ip, xfs_fileoff_t *unused,
 		int whichfork);
-int	xfs_bmap_one_block(struct xfs_inode *ip, int whichfork);
 int	xfs_bmapi_read(struct xfs_inode *ip, xfs_fileoff_t bno,
 		xfs_filblks_t len, struct xfs_bmbt_irec *mval,
 		int *nmap, int flags);
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 8f8837fe21cf02..25dcc98d50e6da 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -514,7 +514,7 @@ xfs_attr_list_ilocked(
 		return 0;
 	if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
 		return xfs_attr_shortform_list(context);
-	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
+	if (xfs_attr_is_leaf(dp))
 		return xfs_attr_leaf_list(context);
 	return xfs_attr_node_list(context);
 }
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/7] xfs: simplify xfs_attr_remove_args
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-05 16:33   ` Brian Foster
  2021-04-02 14:24 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Directly return from the subfunctions and avoid the error variable.  Also
remove the not really needed dp local variable.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 0146f70b71b1e2..6d1854d506d5ad 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -386,21 +386,16 @@ int
 xfs_attr_remove_args(
 	struct xfs_da_args      *args)
 {
-	struct xfs_inode	*dp = args->dp;
-	int			error;
+	if (!xfs_inode_hasattr(args->dp))
+		return -ENOATTR;
 
-	if (!xfs_inode_hasattr(dp)) {
-		error = -ENOATTR;
-	} else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
-		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
-		error = xfs_attr_shortform_remove(args);
-	} else if (xfs_attr_is_leaf(dp)) {
-		error = xfs_attr_leaf_removename(args);
-	} else {
-		error = xfs_attr_node_removename(args);
+	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
+		ASSERT(args->dp->i_afp->if_flags & XFS_IFINLINE);
+		return xfs_attr_shortform_remove(args);
 	}
-
-	return error;
+	if (xfs_attr_is_leaf(args->dp))
+		return xfs_attr_leaf_removename(args);
+	return xfs_attr_node_removename(args);
 }
 
 /*
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
                   ` (2 preceding siblings ...)
  2021-04-02 14:24 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-05 16:35   ` Brian Foster
  2021-04-02 14:24 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Stop using the XFS_IFEXTENTS flag, and instead switch on the fork format
in xfs_idestroy_fork to decide how to cleanup.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_inode_fork.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 1851d6f266d06b..9bdeb2d474b038 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -522,17 +522,16 @@ xfs_idestroy_fork(
 		ifp->if_broot = NULL;
 	}
 
-	/*
-	 * If the format is local, then we can't have an extents array so just
-	 * look for an inline data array.  If we're not local then we may or may
-	 * not have an extents list, so check and free it up if we do.
-	 */
-	if (ifp->if_format == XFS_DINODE_FMT_LOCAL) {
+	switch (ifp->if_format) {
+	case XFS_DINODE_FMT_LOCAL:
 		kmem_free(ifp->if_u1.if_data);
 		ifp->if_u1.if_data = NULL;
-	} else if (ifp->if_flags & XFS_IFEXTENTS) {
+		break;
+	case XFS_DINODE_FMT_EXTENTS:
+	case XFS_DINODE_FMT_BTREE:
 		if (ifp->if_height)
 			xfs_iext_destroy(ifp);
+		break;
 	}
 }
 
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 5/7] xfs: remove XFS_IFBROOT
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
                   ` (3 preceding siblings ...)
  2021-04-02 14:24 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
  2021-04-02 14:24 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
  6 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Just check for a btree format fork instead of the using the equivalent
in-memory XFS_IFBROOT flag.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_bmap.c          | 16 +++++++---------
 fs/xfs/libxfs/xfs_btree_staging.c |  1 -
 fs/xfs/libxfs/xfs_inode_fork.c    |  4 +---
 fs/xfs/libxfs/xfs_inode_fork.h    |  1 -
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index e11f8faaf8898b..8c93ee1b751286 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -631,7 +631,6 @@ xfs_bmap_btree_to_extents(
 		cur->bc_bufs[0] = NULL;
 	xfs_iroot_realloc(ip, -1, whichfork);
 	ASSERT(ifp->if_broot == NULL);
-	ASSERT((ifp->if_flags & XFS_IFBROOT) == 0);
 	ifp->if_format = XFS_DINODE_FMT_EXTENTS;
 	*logflagsp |= XFS_ILOG_CORE | xfs_ilog_fext(whichfork);
 	return 0;
@@ -675,7 +674,6 @@ xfs_bmap_extents_to_btree(
 	 * to expand the root.
 	 */
 	xfs_iroot_realloc(ip, 1, whichfork);
-	ifp->if_flags |= XFS_IFBROOT;
 
 	/*
 	 * Fill in the root.
@@ -4189,7 +4187,7 @@ xfs_bmapi_allocate(
 			return error;
 	}
 
-	if ((ifp->if_flags & XFS_IFBROOT) && !bma->cur)
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE && !bma->cur)
 		bma->cur = xfs_bmbt_init_cursor(mp, bma->tp, bma->ip, whichfork);
 	/*
 	 * Bump the number of extents we've allocated
@@ -4262,7 +4260,7 @@ xfs_bmapi_convert_unwritten(
 	 * Modify (by adding) the state flag, if writing.
 	 */
 	ASSERT(mval->br_blockcount <= len);
-	if ((ifp->if_flags & XFS_IFBROOT) && !bma->cur) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE && !bma->cur) {
 		bma->cur = xfs_bmbt_init_cursor(bma->ip->i_mount, bma->tp,
 					bma->ip, whichfork);
 	}
@@ -4725,7 +4723,7 @@ xfs_bmapi_remap(
 	ip->i_d.di_nblocks += len;
 	xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
 
-	if (ifp->if_flags & XFS_IFBROOT) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_ino.flags = 0;
 	}
@@ -5404,7 +5402,7 @@ __xfs_bunmapi(
 	end--;
 
 	logflags = 0;
-	if (ifp->if_flags & XFS_IFBROOT) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
 		ASSERT(ifp->if_format == XFS_DINODE_FMT_BTREE);
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_ino.flags = 0;
@@ -5878,7 +5876,7 @@ xfs_bmap_collapse_extents(
 	if (error)
 		return error;
 
-	if (ifp->if_flags & XFS_IFBROOT) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_ino.flags = 0;
 	}
@@ -5993,7 +5991,7 @@ xfs_bmap_insert_extents(
 	if (error)
 		return error;
 
-	if (ifp->if_flags & XFS_IFBROOT) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_ino.flags = 0;
 	}
@@ -6108,7 +6106,7 @@ xfs_bmap_split_extent(
 	new.br_blockcount = got.br_blockcount - gotblkcnt;
 	new.br_state = got.br_state;
 
-	if (ifp->if_flags & XFS_IFBROOT) {
+	if (ifp->if_format == XFS_DINODE_FMT_BTREE) {
 		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
 		cur->bc_ino.flags = 0;
 		error = xfs_bmbt_lookup_eq(cur, &got, &i);
diff --git a/fs/xfs/libxfs/xfs_btree_staging.c b/fs/xfs/libxfs/xfs_btree_staging.c
index f464a7c7cf2246..aa8dc9521c3942 100644
--- a/fs/xfs/libxfs/xfs_btree_staging.c
+++ b/fs/xfs/libxfs/xfs_btree_staging.c
@@ -387,7 +387,6 @@ xfs_btree_bload_prep_block(
 		new_size = bbl->iroot_size(cur, nr_this_block, priv);
 		ifp->if_broot = kmem_zalloc(new_size, 0);
 		ifp->if_broot_bytes = (int)new_size;
-		ifp->if_flags |= XFS_IFBROOT;
 
 		/* Initialize it and send it out. */
 		xfs_btree_init_block_int(cur->bc_mp, ifp->if_broot,
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 9bdeb2d474b038..4389a00d103359 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -60,7 +60,7 @@ xfs_init_local_fork(
 	}
 
 	ifp->if_bytes = size;
-	ifp->if_flags &= ~(XFS_IFEXTENTS | XFS_IFBROOT);
+	ifp->if_flags &= ~XFS_IFEXTENTS;
 	ifp->if_flags |= XFS_IFINLINE;
 }
 
@@ -214,7 +214,6 @@ xfs_iformat_btree(
 	xfs_bmdr_to_bmbt(ip, dfp, XFS_DFORK_SIZE(dip, ip->i_mount, whichfork),
 			 ifp->if_broot, size);
 	ifp->if_flags &= ~XFS_IFEXTENTS;
-	ifp->if_flags |= XFS_IFBROOT;
 
 	ifp->if_bytes = 0;
 	ifp->if_u1.if_root = NULL;
@@ -433,7 +432,6 @@ xfs_iroot_realloc(
 			XFS_BMBT_BLOCK_LEN(ip->i_mount));
 	} else {
 		new_broot = NULL;
-		ifp->if_flags &= ~XFS_IFBROOT;
 	}
 
 	/*
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index a0717ab0e5c574..745eae58325791 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -32,7 +32,6 @@ struct xfs_ifork {
  */
 #define	XFS_IFINLINE	0x01	/* Inline data is read in */
 #define	XFS_IFEXTENTS	0x02	/* All extent pointers are read in */
-#define	XFS_IFBROOT	0x04	/* i_broot points to the bmap b-tree root */
 
 /*
  * Worst-case increase in the fork extent count when we're adding a single
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 6/7] xfs: remove XFS_IFINLINE
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
                   ` (4 preceding siblings ...)
  2021-04-02 14:24 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  2021-04-02 21:48     ` kernel test robot
  2021-04-03  3:31     ` kernel test robot
  2021-04-02 14:24 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
  6 siblings, 2 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

Just check for an inline format fork instead of the using the equivalent
in-memory XFS_IFINLINE flag.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr.c       |  8 ++------
 fs/xfs/libxfs/xfs_attr_leaf.c  |  9 +++------
 fs/xfs/libxfs/xfs_bmap.c       |  3 +--
 fs/xfs/libxfs/xfs_dir2_block.c |  2 +-
 fs/xfs/libxfs/xfs_dir2_sf.c    | 11 +++++------
 fs/xfs/libxfs/xfs_inode_fork.c |  1 -
 fs/xfs/libxfs/xfs_inode_fork.h |  1 -
 fs/xfs/scrub/symlink.c         |  2 +-
 fs/xfs/xfs_dir2_readdir.c      |  2 +-
 fs/xfs/xfs_iops.c              |  4 ++--
 fs/xfs/xfs_symlink.c           |  4 ++--
 11 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 6d1854d506d5ad..1f150f24230c9a 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -362,10 +362,8 @@ xfs_has_attr(
 	if (!xfs_inode_hasattr(dp))
 		return -ENOATTR;
 
-	if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
-		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
+	if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
 		return xfs_attr_sf_findname(args, NULL, NULL);
-	}
 
 	if (xfs_attr_is_leaf(dp)) {
 		error = xfs_attr_leaf_hasname(args, &bp);
@@ -389,10 +387,8 @@ xfs_attr_remove_args(
 	if (!xfs_inode_hasattr(args->dp))
 		return -ENOATTR;
 
-	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
-		ASSERT(args->dp->i_afp->if_flags & XFS_IFINLINE);
+	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
 		return xfs_attr_shortform_remove(args);
-	}
 	if (xfs_attr_is_leaf(args->dp))
 		return xfs_attr_leaf_removename(args);
 	return xfs_attr_node_removename(args);
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index d6ef69ab1c67a5..2de6ff2425a449 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -654,9 +654,6 @@ xfs_attr_shortform_create(
 	if (ifp->if_format == XFS_DINODE_FMT_EXTENTS) {
 		ifp->if_flags &= ~XFS_IFEXTENTS;	/* just in case */
 		ifp->if_format = XFS_DINODE_FMT_LOCAL;
-		ifp->if_flags |= XFS_IFINLINE;
-	} else {
-		ASSERT(ifp->if_flags & XFS_IFINLINE);
 	}
 	xfs_idata_realloc(dp, sizeof(*hdr), XFS_ATTR_FORK);
 	hdr = (struct xfs_attr_sf_hdr *)ifp->if_u1.if_data;
@@ -733,7 +730,7 @@ xfs_attr_shortform_add(
 	dp->i_d.di_forkoff = forkoff;
 
 	ifp = dp->i_afp;
-	ASSERT(ifp->if_flags & XFS_IFINLINE);
+	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
 	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
 		ASSERT(0);
@@ -851,7 +848,7 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
 	trace_xfs_attr_sf_lookup(args);
 
 	ifp = args->dp->i_afp;
-	ASSERT(ifp->if_flags & XFS_IFINLINE);
+	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
 	sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
@@ -878,7 +875,7 @@ xfs_attr_shortform_getvalue(
 	struct xfs_attr_sf_entry *sfe;
 	int			i;
 
-	ASSERT(args->dp->i_afp->if_flags == XFS_IFINLINE);
+	ASSERT(args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL);
 	sf = (struct xfs_attr_shortform *)args->dp->i_afp->if_u1.if_data;
 	sfe = &sf->list[0];
 	for (i = 0; i < sf->hdr.count;
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 8c93ee1b751286..854f313749b638 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -803,7 +803,6 @@ xfs_bmap_local_to_extents_empty(
 	ASSERT(ifp->if_nextents == 0);
 
 	xfs_bmap_forkoff_reset(ip, whichfork);
-	ifp->if_flags &= ~XFS_IFINLINE;
 	ifp->if_flags |= XFS_IFEXTENTS;
 	ifp->if_u1.if_root = NULL;
 	ifp->if_height = 0;
@@ -848,7 +847,7 @@ xfs_bmap_local_to_extents(
 
 	flags = 0;
 	error = 0;
-	ASSERT((ifp->if_flags & (XFS_IFINLINE|XFS_IFEXTENTS)) == XFS_IFINLINE);
+	ASSERT(!(ifp->if_flags & XFS_IFEXTENTS));
 	memset(&args, 0, sizeof(args));
 	args.tp = tp;
 	args.mp = ip->i_mount;
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 5b59d3f7746b34..7e1df0c13fa74b 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -1096,7 +1096,7 @@ xfs_dir2_sf_to_block(
 
 	trace_xfs_dir2_sf_to_block(args);
 
-	ASSERT(ifp->if_flags & XFS_IFINLINE);
+	ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
 
 	oldsfp = (xfs_dir2_sf_hdr_t *)ifp->if_u1.if_data;
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 8c4f76bba88be1..e8a53a68625eaf 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -378,7 +378,7 @@ xfs_dir2_sf_addname(
 
 	ASSERT(xfs_dir2_sf_lookup(args) == -ENOENT);
 	dp = args->dp;
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
 	ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
 	ASSERT(dp->i_df.if_u1.if_data != NULL);
@@ -830,9 +830,8 @@ xfs_dir2_sf_create(
 		dp->i_df.if_flags &= ~XFS_IFEXTENTS;	/* just in case */
 		dp->i_df.if_format = XFS_DINODE_FMT_LOCAL;
 		xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE);
-		dp->i_df.if_flags |= XFS_IFINLINE;
 	}
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_df.if_bytes == 0);
 	i8count = pino > XFS_DIR2_MAX_SHORT_INUM;
 	size = xfs_dir2_sf_hdr_size(i8count);
@@ -877,7 +876,7 @@ xfs_dir2_sf_lookup(
 
 	xfs_dir2_sf_check(args);
 
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
 	ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
 	ASSERT(dp->i_df.if_u1.if_data != NULL);
@@ -954,7 +953,7 @@ xfs_dir2_sf_removename(
 
 	trace_xfs_dir2_sf_removename(args);
 
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	oldsize = (int)dp->i_d.di_size;
 	ASSERT(oldsize >= offsetof(struct xfs_dir2_sf_hdr, parent));
 	ASSERT(dp->i_df.if_bytes == oldsize);
@@ -1053,7 +1052,7 @@ xfs_dir2_sf_replace(
 
 	trace_xfs_dir2_sf_replace(args);
 
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_d.di_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
 	ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
 	ASSERT(dp->i_df.if_u1.if_data != NULL);
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 4389a00d103359..5d7d3bddd9a083 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -61,7 +61,6 @@ xfs_init_local_fork(
 
 	ifp->if_bytes = size;
 	ifp->if_flags &= ~XFS_IFEXTENTS;
-	ifp->if_flags |= XFS_IFINLINE;
 }
 
 /*
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 745eae58325791..5f10377cdd6c36 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -30,7 +30,6 @@ struct xfs_ifork {
 /*
  * Per-fork incore inode flags.
  */
-#define	XFS_IFINLINE	0x01	/* Inline data is read in */
 #define	XFS_IFEXTENTS	0x02	/* All extent pointers are read in */
 
 /*
diff --git a/fs/xfs/scrub/symlink.c b/fs/xfs/scrub/symlink.c
index c08be5ede0661a..436e2a3a27ef3d 100644
--- a/fs/xfs/scrub/symlink.c
+++ b/fs/xfs/scrub/symlink.c
@@ -52,7 +52,7 @@ xchk_symlink(
 	}
 
 	/* Inline symlink? */
-	if (ifp->if_flags & XFS_IFINLINE) {
+	if (ifp->if_format == XFS_DINODE_FMT_LOCAL) {
 		if (len > XFS_IFORK_DSIZE(ip) ||
 		    len > strnlen(ifp->if_u1.if_data, XFS_IFORK_DSIZE(ip)))
 			xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 477df4869d19bd..1756ed55ff0595 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -57,7 +57,7 @@ xfs_dir2_sf_getdents(
 	xfs_ino_t		ino;
 	struct xfs_da_geometry	*geo = args->geo;
 
-	ASSERT(dp->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 	ASSERT(dp->i_df.if_bytes == dp->i_d.di_size);
 	ASSERT(dp->i_df.if_u1.if_data != NULL);
 
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 5b8ac9b6cef8e7..934e205935c116 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -519,7 +519,7 @@ xfs_vn_get_link_inline(
 	struct xfs_inode	*ip = XFS_I(inode);
 	char			*link;
 
-	ASSERT(ip->i_df.if_flags & XFS_IFINLINE);
+	ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
 
 	/*
 	 * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
@@ -1402,7 +1402,7 @@ xfs_setup_iops(
 		inode->i_fop = &xfs_dir_file_operations;
 		break;
 	case S_IFLNK:
-		if (ip->i_df.if_flags & XFS_IFINLINE)
+		if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL)
 			inode->i_op = &xfs_inline_symlink_inode_operations;
 		else
 			inode->i_op = &xfs_symlink_inode_operations;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index db9c400f92584b..7b443dab47727c 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -104,7 +104,7 @@ xfs_readlink(
 
 	trace_xfs_readlink(ip);
 
-	ASSERT(!(ip->i_df.if_flags & XFS_IFINLINE));
+	ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
 
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return -EIO;
@@ -492,7 +492,7 @@ xfs_inactive_symlink(
 	 * Inline fork state gets removed by xfs_difree() so we have nothing to
 	 * do here in that case.
 	 */
-	if (ip->i_df.if_flags & XFS_IFINLINE) {
+	if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL) {
 		xfs_iunlock(ip, XFS_ILOCK_EXCL);
 		return 0;
 	}
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 7/7] xfs: remove XFS_IFEXTENTS
  2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
                   ` (5 preceding siblings ...)
  2021-04-02 14:24 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
@ 2021-04-02 14:24 ` Christoph Hellwig
  6 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-02 14:24 UTC (permalink / raw)
  To: linux-xfs

The in-memory XFS_IFEXTENTS is now only used to check if an inode with
extents still needs the extents to be read into memory before doing
operations that need the extent map.  Add a new xfs_need_iread_extents
helper that returns true for btree format forks that do not have any
entries in the in-memory extent btree, and use that instead of checking
the XFS_IFEXTENTS flag.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/libxfs/xfs_attr_leaf.c  |  4 +---
 fs/xfs/libxfs/xfs_bmap.c       | 14 ++------------
 fs/xfs/libxfs/xfs_dir2_sf.c    |  1 -
 fs/xfs/libxfs/xfs_inode_fork.c |  6 ------
 fs/xfs/libxfs/xfs_inode_fork.h | 12 ++++++------
 fs/xfs/scrub/bmap.c            |  6 +-----
 fs/xfs/xfs_aops.c              |  3 +--
 fs/xfs/xfs_bmap_util.c         |  4 ++--
 fs/xfs/xfs_inode.c             |  9 ++-------
 fs/xfs/xfs_ioctl.c             |  2 +-
 fs/xfs/xfs_iomap.c             |  4 ++--
 fs/xfs/xfs_symlink.c           |  2 +-
 12 files changed, 19 insertions(+), 48 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 2de6ff2425a449..543883742e33b4 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -651,10 +651,8 @@ xfs_attr_shortform_create(
 	trace_xfs_attr_sf_create(args);
 
 	ASSERT(ifp->if_bytes == 0);
-	if (ifp->if_format == XFS_DINODE_FMT_EXTENTS) {
-		ifp->if_flags &= ~XFS_IFEXTENTS;	/* just in case */
+	if (ifp->if_format == XFS_DINODE_FMT_EXTENTS)
 		ifp->if_format = XFS_DINODE_FMT_LOCAL;
-	}
 	xfs_idata_realloc(dp, sizeof(*hdr), XFS_ATTR_FORK);
 	hdr = (struct xfs_attr_sf_hdr *)ifp->if_u1.if_data;
 	memset(hdr, 0, sizeof(*hdr));
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 854f313749b638..fa81958ff64ad3 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -603,7 +603,7 @@ xfs_bmap_btree_to_extents(
 
 	ASSERT(cur);
 	ASSERT(whichfork != XFS_COW_FORK);
-	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
+	ASSERT(!xfs_need_iread_extents(ifp));
 	ASSERT(ifp->if_format == XFS_DINODE_FMT_BTREE);
 	ASSERT(be16_to_cpu(rblock->bb_level) == 1);
 	ASSERT(be16_to_cpu(rblock->bb_numrecs) == 1);
@@ -803,7 +803,6 @@ xfs_bmap_local_to_extents_empty(
 	ASSERT(ifp->if_nextents == 0);
 
 	xfs_bmap_forkoff_reset(ip, whichfork);
-	ifp->if_flags |= XFS_IFEXTENTS;
 	ifp->if_u1.if_root = NULL;
 	ifp->if_height = 0;
 	ifp->if_format = XFS_DINODE_FMT_EXTENTS;
@@ -847,7 +846,6 @@ xfs_bmap_local_to_extents(
 
 	flags = 0;
 	error = 0;
-	ASSERT(!(ifp->if_flags & XFS_IFEXTENTS));
 	memset(&args, 0, sizeof(args));
 	args.tp = tp;
 	args.mp = ip->i_mount;
@@ -1092,7 +1090,6 @@ xfs_bmap_add_attrfork(
 	ASSERT(ip->i_afp == NULL);
 
 	ip->i_afp = xfs_ifork_alloc(XFS_DINODE_FMT_EXTENTS, 0);
-	ip->i_afp->if_flags = XFS_IFEXTENTS;
 	logflags = 0;
 	switch (ip->i_df.if_format) {
 	case XFS_DINODE_FMT_LOCAL:
@@ -1220,14 +1217,9 @@ xfs_iread_extents(
 
 	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
 
-	if (ifp->if_flags & XFS_IFEXTENTS)
+	if (!xfs_need_iread_extents(ifp))
 		return 0;
 
-	if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
-		error = -EFSCORRUPTED;
-		goto out;
-	}
-
 	ir.loaded = 0;
 	xfs_iext_first(ifp, &ir.icur);
 	cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
@@ -1242,8 +1234,6 @@ xfs_iread_extents(
 		goto out;
 	}
 	ASSERT(ir.loaded == xfs_iext_count(ifp));
-
-	ifp->if_flags |= XFS_IFEXTENTS;
 	return 0;
 out:
 	xfs_iext_destroy(ifp);
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index e8a53a68625eaf..8e1f4f907c3c05 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -827,7 +827,6 @@ xfs_dir2_sf_create(
 	 * convert it to local format.
 	 */
 	if (dp->i_df.if_format == XFS_DINODE_FMT_EXTENTS) {
-		dp->i_df.if_flags &= ~XFS_IFEXTENTS;	/* just in case */
 		dp->i_df.if_format = XFS_DINODE_FMT_LOCAL;
 		xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE);
 	}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index 5d7d3bddd9a083..1ada6c10e01b69 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -60,7 +60,6 @@ xfs_init_local_fork(
 	}
 
 	ifp->if_bytes = size;
-	ifp->if_flags &= ~XFS_IFEXTENTS;
 }
 
 /*
@@ -150,7 +149,6 @@ xfs_iformat_extents(
 			xfs_iext_next(ifp, &icur);
 		}
 	}
-	ifp->if_flags |= XFS_IFEXTENTS;
 	return 0;
 }
 
@@ -212,7 +210,6 @@ xfs_iformat_btree(
 	 */
 	xfs_bmdr_to_bmbt(ip, dfp, XFS_DFORK_SIZE(dip, ip->i_mount, whichfork),
 			 ifp->if_broot, size);
-	ifp->if_flags &= ~XFS_IFEXTENTS;
 
 	ifp->if_bytes = 0;
 	ifp->if_u1.if_root = NULL;
@@ -622,8 +619,6 @@ xfs_iflush_fork(
 		break;
 
 	case XFS_DINODE_FMT_EXTENTS:
-		ASSERT((ifp->if_flags & XFS_IFEXTENTS) ||
-		       !(iip->ili_fields & extflag[whichfork]));
 		if ((iip->ili_fields & extflag[whichfork]) &&
 		    (ifp->if_bytes > 0)) {
 			ASSERT(ifp->if_nextents > 0);
@@ -683,7 +678,6 @@ xfs_ifork_init_cow(
 
 	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_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 5f10377cdd6c36..e92a381890da8e 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -22,16 +22,10 @@ struct xfs_ifork {
 		char		*if_data;	/* inline file data */
 	} if_u1;
 	short			if_broot_bytes;	/* bytes allocated for root */
-	unsigned char		if_flags;	/* per-fork flags */
 	int8_t			if_format;	/* format of this fork */
 	xfs_extnum_t		if_nextents;	/* # of extents in this fork */
 };
 
-/*
- * Per-fork incore inode flags.
- */
-#define	XFS_IFEXTENTS	0x02	/* All extent pointers are read in */
-
 /*
  * Worst-case increase in the fork extent count when we're adding a single
  * extent to a fork and there's no possibility of splitting an existing mapping.
@@ -236,4 +230,10 @@ int xfs_ifork_verify_local_attr(struct xfs_inode *ip);
 int xfs_iext_count_may_overflow(struct xfs_inode *ip, int whichfork,
 		int nr_to_add);
 
+/* returns true if the fork has extents but they are not read in yet. */
+static inline bool xfs_need_iread_extents(struct xfs_ifork *ifp)
+{
+	return ifp->if_format == XFS_DINODE_FMT_BTREE && ifp->if_height == 0;
+}
+
 #endif	/* __XFS_INODE_FORK_H__ */
diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
index fb50ec9a4303a1..551835dd520625 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -448,7 +448,7 @@ xchk_bmap_btree(
 	int			error;
 
 	/* Load the incore bmap cache if it's not loaded. */
-	info->was_loaded = ifp->if_flags & XFS_IFEXTENTS;
+	info->was_loaded = !xfs_need_iread_extents(ifp);
 
 	error = xfs_iread_extents(sc->tp, ip, whichfork);
 	if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
@@ -674,10 +674,6 @@ xchk_bmap(
 		/* No mappings to check. */
 		goto out;
 	case XFS_DINODE_FMT_EXTENTS:
-		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
-			xchk_fblock_set_corrupt(sc, whichfork, 0);
-			goto out;
-		}
 		break;
 	case XFS_DINODE_FMT_BTREE:
 		if (whichfork == XFS_COW_FORK) {
diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
index 1cc7c36d98e940..6d98e3148bd7ec 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -384,8 +384,7 @@ xfs_map_blocks(
 	cow_fsb = NULLFILEOFF;
 	whichfork = XFS_DATA_FORK;
 	xfs_ilock(ip, XFS_ILOCK_SHARED);
-	ASSERT(ip->i_df.if_format != XFS_DINODE_FMT_BTREE ||
-	       (ip->i_df.if_flags & XFS_IFEXTENTS));
+	ASSERT(!xfs_need_iread_extents(&ip->i_df));
 
 	/*
 	 * Check if this is offset is covered by a COW extents, and if yes use
diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
index e7f9537634f3cf..cc9a71c2139ade 100644
--- a/fs/xfs/xfs_bmap_util.c
+++ b/fs/xfs/xfs_bmap_util.c
@@ -554,7 +554,7 @@ xfs_bmap_punch_delalloc_range(
 	struct xfs_iext_cursor	icur;
 	int			error = 0;
 
-	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
+	ASSERT(!xfs_need_iread_extents(ifp));
 
 	xfs_ilock(ip, XFS_ILOCK_EXCL);
 	if (!xfs_iext_lookup_extent_before(ip, ifp, &end_fsb, &icur, &got))
@@ -609,7 +609,7 @@ xfs_can_free_eofblocks(struct xfs_inode *ip, bool force)
 		return false;
 
 	/* If we haven't read in the extent list, then don't do it now. */
-	if (!(ip->i_df.if_flags & XFS_IFEXTENTS))
+	if (xfs_need_iread_extents(&ip->i_df))
 		return false;
 
 	/*
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index c09bb39baeea99..1bf6b17c5d15fc 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -111,8 +111,7 @@ xfs_ilock_data_map_shared(
 {
 	uint			lock_mode = XFS_ILOCK_SHARED;
 
-	if (ip->i_df.if_format == XFS_DINODE_FMT_BTREE &&
-	    (ip->i_df.if_flags & XFS_IFEXTENTS) == 0)
+	if (xfs_need_iread_extents(&ip->i_df))
 		lock_mode = XFS_ILOCK_EXCL;
 	xfs_ilock(ip, lock_mode);
 	return lock_mode;
@@ -124,9 +123,7 @@ xfs_ilock_attr_map_shared(
 {
 	uint			lock_mode = XFS_ILOCK_SHARED;
 
-	if (ip->i_afp &&
-	    ip->i_afp->if_format == XFS_DINODE_FMT_BTREE &&
-	    (ip->i_afp->if_flags & XFS_IFEXTENTS) == 0)
+	if (ip->i_afp && xfs_need_iread_extents(ip->i_afp))
 		lock_mode = XFS_ILOCK_EXCL;
 	xfs_ilock(ip, lock_mode);
 	return lock_mode;
@@ -858,7 +855,6 @@ xfs_init_new_inode(
 	case S_IFBLK:
 	case S_IFSOCK:
 		ip->i_df.if_format = XFS_DINODE_FMT_DEV;
-		ip->i_df.if_flags = 0;
 		flags |= XFS_ILOG_DEV;
 		break;
 	case S_IFREG:
@@ -870,7 +866,6 @@ xfs_init_new_inode(
 		/* FALLTHROUGH */
 	case S_IFLNK:
 		ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
-		ip->i_df.if_flags = XFS_IFEXTENTS;
 		ip->i_df.if_bytes = 0;
 		ip->i_df.if_u1.if_root = NULL;
 		break;
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 99dfe89a8d08b8..6687d02d0d8794 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1124,7 +1124,7 @@ xfs_fill_fsxattr(
 	fa->fsx_cowextsize = ip->i_d.di_cowextsize <<
 			ip->i_mount->m_sb.sb_blocklog;
 	fa->fsx_projid = ip->i_d.di_projid;
-	if (ifp && (ifp->if_flags & XFS_IFEXTENTS))
+	if (ifp && !xfs_need_iread_extents(ifp))
 		fa->fsx_nextents = xfs_iext_count(ifp);
 	else
 		fa->fsx_nextents = xfs_ifork_nextents(ifp);
diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
index 129a0bafb46c0d..10711817eb2fae 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -159,7 +159,7 @@ xfs_iomap_eof_align_last_fsb(
 	struct xfs_bmbt_irec	irec;
 	struct xfs_iext_cursor	icur;
 
-	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
+	ASSERT(!xfs_need_iread_extents(ifp));
 
 	/*
 	 * Always round up the allocation request to the extent hint boundary.
@@ -666,7 +666,7 @@ xfs_ilock_for_iomap(
 	 * is an opencoded xfs_ilock_data_map_shared() call but with
 	 * non-blocking behaviour.
 	 */
-	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
+	if (xfs_need_iread_extents(&ip->i_df)) {
 		if (flags & IOMAP_NOWAIT)
 			return -EAGAIN;
 		mode = XFS_ILOCK_EXCL;
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 7b443dab47727c..afc44088e3e386 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -377,7 +377,7 @@ xfs_inactive_symlink_rmt(
 	xfs_trans_t	*tp;
 
 	mp = ip->i_mount;
-	ASSERT(ip->i_df.if_flags & XFS_IFEXTENTS);
+	ASSERT(!xfs_need_iread_extents(&ip->i_df));
 	/*
 	 * We're freeing a symlink that has some
 	 * blocks allocated to it.  Free the
-- 
2.30.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
  2021-04-02 14:24 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
@ 2021-04-02 21:48     ` kernel test robot
  2021-04-03  3:31     ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-04-02 21:48 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: kbuild-all, clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 2759 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: x86_64-randconfig-r034-20210401 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project b23a314146956dd29b719ab537608ced736fc036)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3a376a77f4296e338a26df75eb05a1b7ae0def2a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
        git checkout 3a376a77f4296e338a26df75eb05a1b7ae0def2a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/xfs/xfs_iops.c:522:9: error: use of undeclared identifier 'dp'
           ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
                  ^
   1 error generated.
--
>> fs/xfs/xfs_symlink.c:107:9: error: use of undeclared identifier 'dp'
           ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
                  ^
   1 error generated.


vim +/dp +522 fs/xfs/xfs_iops.c

   512	
   513	STATIC const char *
   514	xfs_vn_get_link_inline(
   515		struct dentry		*dentry,
   516		struct inode		*inode,
   517		struct delayed_call	*done)
   518	{
   519		struct xfs_inode	*ip = XFS_I(inode);
   520		char			*link;
   521	
 > 522		ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
   523	
   524		/*
   525		 * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
   526		 * if_data is junk.
   527		 */
   528		link = ip->i_df.if_u1.if_data;
   529		if (XFS_IS_CORRUPT(ip->i_mount, !link))
   530			return ERR_PTR(-EFSCORRUPTED);
   531		return link;
   532	}
   533	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37563 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
@ 2021-04-02 21:48     ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-04-02 21:48 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2831 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: x86_64-randconfig-r034-20210401 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project b23a314146956dd29b719ab537608ced736fc036)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/3a376a77f4296e338a26df75eb05a1b7ae0def2a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
        git checkout 3a376a77f4296e338a26df75eb05a1b7ae0def2a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> fs/xfs/xfs_iops.c:522:9: error: use of undeclared identifier 'dp'
           ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
                  ^
   1 error generated.
--
>> fs/xfs/xfs_symlink.c:107:9: error: use of undeclared identifier 'dp'
           ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
                  ^
   1 error generated.


vim +/dp +522 fs/xfs/xfs_iops.c

   512	
   513	STATIC const char *
   514	xfs_vn_get_link_inline(
   515		struct dentry		*dentry,
   516		struct inode		*inode,
   517		struct delayed_call	*done)
   518	{
   519		struct xfs_inode	*ip = XFS_I(inode);
   520		char			*link;
   521	
 > 522		ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
   523	
   524		/*
   525		 * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
   526		 * if_data is junk.
   527		 */
   528		link = ip->i_df.if_u1.if_data;
   529		if (XFS_IS_CORRUPT(ip->i_mount, !link))
   530			return ERR_PTR(-EFSCORRUPTED);
   531		return link;
   532	}
   533	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 37563 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
  2021-04-02 14:24 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
@ 2021-04-03  3:31     ` kernel test robot
  2021-04-03  3:31     ` kernel test robot
  1 sibling, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-04-03  3:31 UTC (permalink / raw)
  To: Christoph Hellwig, linux-xfs; +Cc: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4929 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: microblaze-randconfig-r036-20210401 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3a376a77f4296e338a26df75eb05a1b7ae0def2a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
        git checkout 3a376a77f4296e338a26df75eb05a1b7ae0def2a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/string.h:6,
                    from include/linux/uuid.h:12,
                    from fs/xfs/xfs_linux.h:10,
                    from fs/xfs/xfs.h:22,
                    from fs/xfs/xfs_iops.c:6:
   fs/xfs/xfs_iops.c: In function 'xfs_vn_get_link_inline':
>> fs/xfs/xfs_iops.c:522:9: error: 'dp' undeclared (first use in this function); did you mean 'ip'?
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_iops.c:522:2: note: in expansion of macro 'ASSERT'
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
   fs/xfs/xfs_iops.c:522:9: note: each undeclared identifier is reported only once for each function it appears in
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_iops.c:522:2: note: in expansion of macro 'ASSERT'
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
--
   In file included from include/linux/string.h:6,
                    from include/linux/uuid.h:12,
                    from fs/xfs/xfs_linux.h:10,
                    from fs/xfs/xfs.h:22,
                    from fs/xfs/xfs_symlink.c:7:
   fs/xfs/xfs_symlink.c: In function 'xfs_readlink':
>> fs/xfs/xfs_symlink.c:107:9: error: 'dp' undeclared (first use in this function); did you mean 'mp'?
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_symlink.c:107:2: note: in expansion of macro 'ASSERT'
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
   fs/xfs/xfs_symlink.c:107:9: note: each undeclared identifier is reported only once for each function it appears in
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_symlink.c:107:2: note: in expansion of macro 'ASSERT'
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~


vim +522 fs/xfs/xfs_iops.c

   512	
   513	STATIC const char *
   514	xfs_vn_get_link_inline(
   515		struct dentry		*dentry,
   516		struct inode		*inode,
   517		struct delayed_call	*done)
   518	{
   519		struct xfs_inode	*ip = XFS_I(inode);
   520		char			*link;
   521	
 > 522		ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
   523	
   524		/*
   525		 * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
   526		 * if_data is junk.
   527		 */
   528		link = ip->i_df.if_u1.if_data;
   529		if (XFS_IS_CORRUPT(ip->i_mount, !link))
   530			return ERR_PTR(-EFSCORRUPTED);
   531		return link;
   532	}
   533	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28131 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
@ 2021-04-03  3:31     ` kernel test robot
  0 siblings, 0 replies; 18+ messages in thread
From: kernel test robot @ 2021-04-03  3:31 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5039 bytes --]

Hi Christoph,

I love your patch! Yet something to improve:

[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
base:   https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: microblaze-randconfig-r036-20210401 (attached as .config)
compiler: microblaze-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3a376a77f4296e338a26df75eb05a1b7ae0def2a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Christoph-Hellwig/xfs-move-the-XFS_IFEXTENTS-check-into-xfs_iread_extents/20210402-232422
        git checkout 3a376a77f4296e338a26df75eb05a1b7ae0def2a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=microblaze 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/string.h:6,
                    from include/linux/uuid.h:12,
                    from fs/xfs/xfs_linux.h:10,
                    from fs/xfs/xfs.h:22,
                    from fs/xfs/xfs_iops.c:6:
   fs/xfs/xfs_iops.c: In function 'xfs_vn_get_link_inline':
>> fs/xfs/xfs_iops.c:522:9: error: 'dp' undeclared (first use in this function); did you mean 'ip'?
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_iops.c:522:2: note: in expansion of macro 'ASSERT'
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
   fs/xfs/xfs_iops.c:522:9: note: each undeclared identifier is reported only once for each function it appears in
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_iops.c:522:2: note: in expansion of macro 'ASSERT'
     522 |  ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
--
   In file included from include/linux/string.h:6,
                    from include/linux/uuid.h:12,
                    from fs/xfs/xfs_linux.h:10,
                    from fs/xfs/xfs.h:22,
                    from fs/xfs/xfs_symlink.c:7:
   fs/xfs/xfs_symlink.c: In function 'xfs_readlink':
>> fs/xfs/xfs_symlink.c:107:9: error: 'dp' undeclared (first use in this function); did you mean 'mp'?
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_symlink.c:107:2: note: in expansion of macro 'ASSERT'
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~
   fs/xfs/xfs_symlink.c:107:9: note: each undeclared identifier is reported only once for each function it appears in
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |         ^~
   include/linux/compiler.h:77:40: note: in definition of macro 'likely'
      77 | # define likely(x) __builtin_expect(!!(x), 1)
         |                                        ^
   fs/xfs/xfs_symlink.c:107:2: note: in expansion of macro 'ASSERT'
     107 |  ASSERT(dp->i_df.if_format != XFS_DINODE_FMT_LOCAL);
         |  ^~~~~~


vim +522 fs/xfs/xfs_iops.c

   512	
   513	STATIC const char *
   514	xfs_vn_get_link_inline(
   515		struct dentry		*dentry,
   516		struct inode		*inode,
   517		struct delayed_call	*done)
   518	{
   519		struct xfs_inode	*ip = XFS_I(inode);
   520		char			*link;
   521	
 > 522		ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
   523	
   524		/*
   525		 * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
   526		 * if_data is junk.
   527		 */
   528		link = ip->i_df.if_u1.if_data;
   529		if (XFS_IS_CORRUPT(ip->i_mount, !link))
   530			return ERR_PTR(-EFSCORRUPTED);
   531		return link;
   532	}
   533	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 28131 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
  2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
@ 2021-04-05 16:33   ` Brian Foster
  2021-04-06 13:51   ` Darrick J. Wong
  1 sibling, 0 replies; 18+ messages in thread
From: Brian Foster @ 2021-04-05 16:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Apr 02, 2021 at 04:24:03PM +0200, Christoph Hellwig wrote:
> Move the XFS_IFEXTENTS check from the callers into xfs_iread_extents to
> simplify the code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Looks reasonable enough on its own:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_bmap.c  | 82 ++++++++++++++++-----------------------
>  fs/xfs/scrub/bmap.c       |  9 ++---
>  fs/xfs/xfs_bmap_util.c    | 16 +++-----
>  fs/xfs/xfs_dir2_readdir.c |  8 ++--
>  fs/xfs/xfs_dquot.c        |  8 ++--
>  fs/xfs/xfs_iomap.c        | 16 +++-----
>  fs/xfs/xfs_qm.c           |  8 ++--
>  fs/xfs/xfs_reflink.c      |  8 ++--
>  8 files changed, 62 insertions(+), 93 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5574d345d066ed..b8cab14ca8ce8d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1223,6 +1223,9 @@ xfs_iread_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>  
> +	if (ifp->if_flags & XFS_IFEXTENTS)
> +		return 0;
> +
>  	if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
>  		error = -EFSCORRUPTED;
>  		goto out;
> @@ -1278,11 +1281,9 @@ xfs_bmap_first_unused(
>  
>  	ASSERT(xfs_ifork_has_extents(ifp));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	lowest = max = *first_unused;
>  	for_each_xfs_iext(ifp, &icur, &got) {
> @@ -1330,11 +1331,9 @@ xfs_bmap_last_before(
>  		return -EFSCORRUPTED;
>  	}
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (!xfs_iext_lookup_extent_before(ip, ifp, last_block, &icur, &got))
>  		*last_block = 0;
> @@ -1353,11 +1352,9 @@ xfs_bmap_last_extent(
>  	struct xfs_iext_cursor	icur;
>  	int			error;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	xfs_iext_last(ifp, &icur);
>  	if (!xfs_iext_get_extent(ifp, &icur, rec))
> @@ -3984,11 +3981,9 @@ xfs_bmapi_read(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapr);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(NULL, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
>  		eof = true;
> @@ -4468,11 +4463,9 @@ xfs_bmapi_write(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapw);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			goto error0;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		goto error0;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.icur, &bma.got))
>  		eof = true;
> @@ -4751,11 +4744,9 @@ xfs_bmapi_remap(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
>  		/* make sure we only reflink into a hole. */
> @@ -5426,9 +5417,10 @@ __xfs_bunmapi(
>  	else
>  		max_len = len;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> -	    (error = xfs_iread_extents(tp, ip, whichfork)))
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
>  		return error;
> +
>  	if (xfs_iext_count(ifp) == 0) {
>  		*rlen = 0;
>  		return 0;
> @@ -5914,11 +5906,9 @@ xfs_bmap_collapse_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (ifp->if_flags & XFS_IFBROOT) {
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> @@ -6031,11 +6021,9 @@ xfs_bmap_insert_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (ifp->if_flags & XFS_IFBROOT) {
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> @@ -6134,12 +6122,10 @@ xfs_bmap_split_extent(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		/* Read in all the extents */
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	/* Read in all the extents */
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	/*
>  	 * If there are not extents, or split_fsb lies in a hole we are done.
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index 33559c3a4bc3d4..fb50ec9a4303a1 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -449,11 +449,10 @@ xchk_bmap_btree(
>  
>  	/* Load the incore bmap cache if it's not loaded. */
>  	info->was_loaded = ifp->if_flags & XFS_IFEXTENTS;
> -	if (!info->was_loaded) {
> -		error = xfs_iread_extents(sc->tp, ip, whichfork);
> -		if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
> -			goto out;
> -	}
> +
> +	error = xfs_iread_extents(sc->tp, ip, whichfork);
> +	if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
> +		goto out;
>  
>  	/* Check the btree structure. */
>  	cur = xfs_bmbt_init_cursor(mp, sc->tp, ip, whichfork);
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index e7d68318e6a55c..e7f9537634f3cf 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -225,11 +225,9 @@ xfs_bmap_count_blocks(
>  
>  	switch (ifp->if_format) {
>  	case XFS_DINODE_FMT_BTREE:
> -		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -			error = xfs_iread_extents(tp, ip, whichfork);
> -			if (error)
> -				return error;
> -		}
> +		error = xfs_iread_extents(tp, ip, whichfork);
> +		if (error)
> +			return error;
>  
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
>  		error = xfs_btree_count_blocks(cur, &btblocks);
> @@ -471,11 +469,9 @@ xfs_getbmap(
>  	first_bno = bno = XFS_BB_TO_FSBT(mp, bmv->bmv_offset);
>  	len = XFS_BB_TO_FSB(mp, bmv->bmv_length);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, whichfork);
> -		if (error)
> -			goto out_unlock_ilock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, whichfork);
> +	if (error)
> +		goto out_unlock_ilock;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
>  		/*
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index 66deddd5e29698..477df4869d19bd 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -258,11 +258,9 @@ xfs_dir2_leaf_readbuf(
>  	int			ra_want;
>  	int			error = 0;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
> -		if (error)
> -			goto out;
> -	}
> +	error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
> +	if (error)
> +		goto out;
>  
>  	/*
>  	 * Look for mapped directory blocks at or above the current offset.
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index bd8379b98374f8..83c5334fe978f2 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -748,11 +748,9 @@ xfs_dq_get_next_id(
>  	start = (xfs_fsblock_t)next_id / mp->m_quotainfo->qi_dqperchunk;
>  
>  	lock_flags = xfs_ilock_data_map_shared(quotip);
> -	if (!(quotip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
> +	if (error)
> +		return error;
>  
>  	if (xfs_iext_lookup_extent(quotip, &quotip->i_df, start, &cur, &got)) {
>  		/* contiguous chunk, bump startoff for the id calculation */
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index e17ab7f42928a5..129a0bafb46c0d 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -893,11 +893,9 @@ xfs_buffered_write_iomap_begin(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapw);
>  
> -	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> -		if (error)
> -			goto out_unlock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> +	if (error)
> +		goto out_unlock;
>  
>  	/*
>  	 * Search the data fork first to look up our source mapping.  We
> @@ -1208,11 +1206,9 @@ xfs_seek_iomap_begin(
>  		return -EIO;
>  
>  	lockmode = xfs_ilock_data_map_shared(ip);
> -	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> -		if (error)
> -			goto out_unlock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> +	if (error)
> +		goto out_unlock;
>  
>  	if (xfs_iext_lookup_extent(ip, &ip->i_df, offset_fsb, &icur, &imap)) {
>  		/*
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 6fde318b9fed27..fd835a202c51eb 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1165,11 +1165,9 @@ xfs_qm_dqusage_adjust(
>  	if (XFS_IS_REALTIME_INODE(ip)) {
>  		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
>  
> -		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -			error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> -			if (error)
> -				goto error0;
> -		}
> +		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> +		if (error)
> +			goto error0;
>  
>  		xfs_bmap_count_leaves(ifp, &rtblks);
>  	}
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 725c7d8e44381b..13cd62b89c4e30 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1392,11 +1392,9 @@ xfs_reflink_inode_has_shared_extents(
>  	int				error;
>  
>  	ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> +	if (error)
> +		return error;
>  
>  	*has_shared = false;
>  	found = xfs_iext_lookup_extent(ip, ifp, 0, &icur, &got);
> -- 
> 2.30.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block
  2021-04-02 14:24 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
@ 2021-04-05 16:33   ` Brian Foster
  0 siblings, 0 replies; 18+ messages in thread
From: Brian Foster @ 2021-04-05 16:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Apr 02, 2021 at 04:24:04PM +0200, Christoph Hellwig wrote:
> xfs_bmap_one_block is only called for the attribute fork.  Move it to
> xfs_attr.c, drop the unused whichfork argument and code only executed for
> the data fork and rename the result to xfs_attr_is_leaf.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr.c | 30 +++++++++++++++++++++++++-----
>  fs/xfs/libxfs/xfs_attr.h |  1 +
>  fs/xfs/libxfs/xfs_bmap.c | 32 --------------------------------
>  fs/xfs/libxfs/xfs_bmap.h |  1 -
>  fs/xfs/xfs_attr_list.c   |  2 +-
>  5 files changed, 27 insertions(+), 39 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 472b3039eabbf1..0146f70b71b1e2 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -70,6 +70,26 @@ xfs_inode_hasattr(
>  	return 1;
>  }
>  
> +/*
> + * Returns true if the there is exactly only block in the attr fork, in which
> + * case the attribute fork consists of a single leaf block entry.
> + */
> +bool
> +xfs_attr_is_leaf(
> +	struct xfs_inode	*ip)
> +{
> +	struct xfs_ifork	*ifp = ip->i_afp;
> +	struct xfs_iext_cursor	icur;
> +	struct xfs_bmbt_irec	imap;
> +
> +	if (ifp->if_nextents != 1 || ifp->if_format != XFS_DINODE_FMT_EXTENTS)
> +		return false;
> +
> +	xfs_iext_first(ifp, &icur);
> +	xfs_iext_get_extent(ifp, &icur, &imap);
> +	return imap.br_startoff == 0 && imap.br_blockcount == 1;
> +}
> +
>  /*========================================================================
>   * Overall external interface routines.
>   *========================================================================*/
> @@ -89,7 +109,7 @@ xfs_attr_get_ilocked(
>  
>  	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
>  		return xfs_attr_shortform_getvalue(args);
> -	if (xfs_bmap_one_block(args->dp, XFS_ATTR_FORK))
> +	if (xfs_attr_is_leaf(args->dp))
>  		return xfs_attr_leaf_get(args);
>  	return xfs_attr_node_get(args);
>  }
> @@ -293,7 +313,7 @@ xfs_attr_set_args(
>  			return error;
>  	}
>  
> -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> +	if (xfs_attr_is_leaf(dp)) {
>  		error = xfs_attr_leaf_addname(args);
>  		if (error != -ENOSPC)
>  			return error;
> @@ -347,7 +367,7 @@ xfs_has_attr(
>  		return xfs_attr_sf_findname(args, NULL, NULL);
>  	}
>  
> -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> +	if (xfs_attr_is_leaf(dp)) {
>  		error = xfs_attr_leaf_hasname(args, &bp);
>  
>  		if (bp)
> @@ -374,7 +394,7 @@ xfs_attr_remove_args(
>  	} else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
>  		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
>  		error = xfs_attr_shortform_remove(args);
> -	} else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
> +	} else if (xfs_attr_is_leaf(dp)) {
>  		error = xfs_attr_leaf_removename(args);
>  	} else {
>  		error = xfs_attr_node_removename(args);
> @@ -1282,7 +1302,7 @@ xfs_attr_node_removename(
>  	/*
>  	 * If the result is small enough, push it all into the inode.
>  	 */
> -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> +	if (xfs_attr_is_leaf(dp))
>  		error = xfs_attr_node_shrink(args, state);
>  
>  out:
> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
> index 3e97a935e7121f..2b1f61987a9dec 100644
> --- a/fs/xfs/libxfs/xfs_attr.h
> +++ b/fs/xfs/libxfs/xfs_attr.h
> @@ -85,6 +85,7 @@ int xfs_attr_inactive(struct xfs_inode *dp);
>  int xfs_attr_list_ilocked(struct xfs_attr_list_context *);
>  int xfs_attr_list(struct xfs_attr_list_context *);
>  int xfs_inode_hasattr(struct xfs_inode *ip);
> +bool xfs_attr_is_leaf(struct xfs_inode *ip);
>  int xfs_attr_get_ilocked(struct xfs_da_args *args);
>  int xfs_attr_get(struct xfs_da_args *args);
>  int xfs_attr_set(struct xfs_da_args *args);
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index b8cab14ca8ce8d..e11f8faaf8898b 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1435,38 +1435,6 @@ xfs_bmap_last_offset(
>  	return 0;
>  }
>  
> -/*
> - * Returns whether the selected fork of the inode has exactly one
> - * block or not.  For the data fork we check this matches di_size,
> - * implying the file's range is 0..bsize-1.
> - */
> -int					/* 1=>1 block, 0=>otherwise */
> -xfs_bmap_one_block(
> -	struct xfs_inode	*ip,		/* incore inode */
> -	int			whichfork)	/* data or attr fork */
> -{
> -	struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, whichfork);
> -	int			rval;		/* return value */
> -	struct xfs_bmbt_irec	s;		/* internal version of extent */
> -	struct xfs_iext_cursor icur;
> -
> -#ifndef DEBUG
> -	if (whichfork == XFS_DATA_FORK)
> -		return XFS_ISIZE(ip) == ip->i_mount->m_sb.sb_blocksize;
> -#endif	/* !DEBUG */
> -	if (ifp->if_nextents != 1)
> -		return 0;
> -	if (ifp->if_format != XFS_DINODE_FMT_EXTENTS)
> -		return 0;
> -	ASSERT(ifp->if_flags & XFS_IFEXTENTS);
> -	xfs_iext_first(ifp, &icur);
> -	xfs_iext_get_extent(ifp, &icur, &s);
> -	rval = s.br_startoff == 0 && s.br_blockcount == 1;
> -	if (rval && whichfork == XFS_DATA_FORK)
> -		ASSERT(XFS_ISIZE(ip) == ip->i_mount->m_sb.sb_blocksize);
> -	return rval;
> -}
> -
>  /*
>   * Extent tree manipulation functions used during allocation.
>   */
> diff --git a/fs/xfs/libxfs/xfs_bmap.h b/fs/xfs/libxfs/xfs_bmap.h
> index 6747e97a794901..59fa4834a761fe 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -199,7 +199,6 @@ int	xfs_bmap_last_before(struct xfs_trans *tp, struct xfs_inode *ip,
>  		xfs_fileoff_t *last_block, int whichfork);
>  int	xfs_bmap_last_offset(struct xfs_inode *ip, xfs_fileoff_t *unused,
>  		int whichfork);
> -int	xfs_bmap_one_block(struct xfs_inode *ip, int whichfork);
>  int	xfs_bmapi_read(struct xfs_inode *ip, xfs_fileoff_t bno,
>  		xfs_filblks_t len, struct xfs_bmbt_irec *mval,
>  		int *nmap, int flags);
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 8f8837fe21cf02..25dcc98d50e6da 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -514,7 +514,7 @@ xfs_attr_list_ilocked(
>  		return 0;
>  	if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL)
>  		return xfs_attr_shortform_list(context);
> -	if (xfs_bmap_one_block(dp, XFS_ATTR_FORK))
> +	if (xfs_attr_is_leaf(dp))
>  		return xfs_attr_leaf_list(context);
>  	return xfs_attr_node_list(context);
>  }
> -- 
> 2.30.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/7] xfs: simplify xfs_attr_remove_args
  2021-04-02 14:24 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
@ 2021-04-05 16:33   ` Brian Foster
  0 siblings, 0 replies; 18+ messages in thread
From: Brian Foster @ 2021-04-05 16:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Apr 02, 2021 at 04:24:05PM +0200, Christoph Hellwig wrote:
> Directly return from the subfunctions and avoid the error variable.  Also
> remove the not really needed dp local variable.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index 0146f70b71b1e2..6d1854d506d5ad 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -386,21 +386,16 @@ int
>  xfs_attr_remove_args(
>  	struct xfs_da_args      *args)
>  {
> -	struct xfs_inode	*dp = args->dp;
> -	int			error;
> +	if (!xfs_inode_hasattr(args->dp))
> +		return -ENOATTR;
>  
> -	if (!xfs_inode_hasattr(dp)) {
> -		error = -ENOATTR;
> -	} else if (dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
> -		ASSERT(dp->i_afp->if_flags & XFS_IFINLINE);
> -		error = xfs_attr_shortform_remove(args);
> -	} else if (xfs_attr_is_leaf(dp)) {
> -		error = xfs_attr_leaf_removename(args);
> -	} else {
> -		error = xfs_attr_node_removename(args);
> +	if (args->dp->i_afp->if_format == XFS_DINODE_FMT_LOCAL) {
> +		ASSERT(args->dp->i_afp->if_flags & XFS_IFINLINE);
> +		return xfs_attr_shortform_remove(args);
>  	}
> -
> -	return error;
> +	if (xfs_attr_is_leaf(args->dp))
> +		return xfs_attr_leaf_removename(args);
> +	return xfs_attr_node_removename(args);
>  }
>  
>  /*
> -- 
> 2.30.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork
  2021-04-02 14:24 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
@ 2021-04-05 16:35   ` Brian Foster
  0 siblings, 0 replies; 18+ messages in thread
From: Brian Foster @ 2021-04-05 16:35 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Apr 02, 2021 at 04:24:06PM +0200, Christoph Hellwig wrote:
> Stop using the XFS_IFEXTENTS flag, and instead switch on the fork format
> in xfs_idestroy_fork to decide how to cleanup.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

And FWIW the remaining patches seem generally reasonable to me at a
quick read through. I'll probably have to take a closer look at the
details once this is more solidified..

Brian

>  fs/xfs/libxfs/xfs_inode_fork.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 1851d6f266d06b..9bdeb2d474b038 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -522,17 +522,16 @@ xfs_idestroy_fork(
>  		ifp->if_broot = NULL;
>  	}
>  
> -	/*
> -	 * If the format is local, then we can't have an extents array so just
> -	 * look for an inline data array.  If we're not local then we may or may
> -	 * not have an extents list, so check and free it up if we do.
> -	 */
> -	if (ifp->if_format == XFS_DINODE_FMT_LOCAL) {
> +	switch (ifp->if_format) {
> +	case XFS_DINODE_FMT_LOCAL:
>  		kmem_free(ifp->if_u1.if_data);
>  		ifp->if_u1.if_data = NULL;
> -	} else if (ifp->if_flags & XFS_IFEXTENTS) {
> +		break;
> +	case XFS_DINODE_FMT_EXTENTS:
> +	case XFS_DINODE_FMT_BTREE:
>  		if (ifp->if_height)
>  			xfs_iext_destroy(ifp);
> +		break;
>  	}
>  }
>  
> -- 
> 2.30.1
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
  2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
  2021-04-05 16:33   ` Brian Foster
@ 2021-04-06 13:51   ` Darrick J. Wong
  2021-04-06 13:54     ` Christoph Hellwig
  1 sibling, 1 reply; 18+ messages in thread
From: Darrick J. Wong @ 2021-04-06 13:51 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-xfs

On Fri, Apr 02, 2021 at 04:24:03PM +0200, Christoph Hellwig wrote:
> Move the XFS_IFEXTENTS check from the callers into xfs_iread_extents to
> simplify the code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_bmap.c  | 82 ++++++++++++++++-----------------------
>  fs/xfs/scrub/bmap.c       |  9 ++---
>  fs/xfs/xfs_bmap_util.c    | 16 +++-----
>  fs/xfs/xfs_dir2_readdir.c |  8 ++--
>  fs/xfs/xfs_dquot.c        |  8 ++--
>  fs/xfs/xfs_iomap.c        | 16 +++-----
>  fs/xfs/xfs_qm.c           |  8 ++--
>  fs/xfs/xfs_reflink.c      |  8 ++--
>  8 files changed, 62 insertions(+), 93 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 5574d345d066ed..b8cab14ca8ce8d 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1223,6 +1223,9 @@ xfs_iread_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));

Since we now call xfs_iread_extents unconditionally, this assert will
trip every time someone wants to bmapi_read the extent data after the
extent data has been loaded in, because xfs_ilock_data_map_shared tells
callers they only need to take ILOCK_SHARED... right?

--D

> +	if (ifp->if_flags & XFS_IFEXTENTS)
> +		return 0;
> +
>  	if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
>  		error = -EFSCORRUPTED;
>  		goto out;
> @@ -1278,11 +1281,9 @@ xfs_bmap_first_unused(
>  
>  	ASSERT(xfs_ifork_has_extents(ifp));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	lowest = max = *first_unused;
>  	for_each_xfs_iext(ifp, &icur, &got) {
> @@ -1330,11 +1331,9 @@ xfs_bmap_last_before(
>  		return -EFSCORRUPTED;
>  	}
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (!xfs_iext_lookup_extent_before(ip, ifp, last_block, &icur, &got))
>  		*last_block = 0;
> @@ -1353,11 +1352,9 @@ xfs_bmap_last_extent(
>  	struct xfs_iext_cursor	icur;
>  	int			error;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	xfs_iext_last(ifp, &icur);
>  	if (!xfs_iext_get_extent(ifp, &icur, rec))
> @@ -3984,11 +3981,9 @@ xfs_bmapi_read(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapr);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(NULL, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got))
>  		eof = true;
> @@ -4468,11 +4463,9 @@ xfs_bmapi_write(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapw);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			goto error0;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		goto error0;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &bma.icur, &bma.got))
>  		eof = true;
> @@ -4751,11 +4744,9 @@ xfs_bmapi_remap(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
>  		/* make sure we only reflink into a hole. */
> @@ -5426,9 +5417,10 @@ __xfs_bunmapi(
>  	else
>  		max_len = len;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS) &&
> -	    (error = xfs_iread_extents(tp, ip, whichfork)))
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
>  		return error;
> +
>  	if (xfs_iext_count(ifp) == 0) {
>  		*rlen = 0;
>  		return 0;
> @@ -5914,11 +5906,9 @@ xfs_bmap_collapse_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (ifp->if_flags & XFS_IFBROOT) {
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> @@ -6031,11 +6021,9 @@ xfs_bmap_insert_extents(
>  
>  	ASSERT(xfs_isilocked(ip, XFS_IOLOCK_EXCL | XFS_ILOCK_EXCL));
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	if (ifp->if_flags & XFS_IFBROOT) {
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
> @@ -6134,12 +6122,10 @@ xfs_bmap_split_extent(
>  	if (XFS_FORCED_SHUTDOWN(mp))
>  		return -EIO;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		/* Read in all the extents */
> -		error = xfs_iread_extents(tp, ip, whichfork);
> -		if (error)
> -			return error;
> -	}
> +	/* Read in all the extents */
> +	error = xfs_iread_extents(tp, ip, whichfork);
> +	if (error)
> +		return error;
>  
>  	/*
>  	 * If there are not extents, or split_fsb lies in a hole we are done.
> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c
> index 33559c3a4bc3d4..fb50ec9a4303a1 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -449,11 +449,10 @@ xchk_bmap_btree(
>  
>  	/* Load the incore bmap cache if it's not loaded. */
>  	info->was_loaded = ifp->if_flags & XFS_IFEXTENTS;
> -	if (!info->was_loaded) {
> -		error = xfs_iread_extents(sc->tp, ip, whichfork);
> -		if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
> -			goto out;
> -	}
> +
> +	error = xfs_iread_extents(sc->tp, ip, whichfork);
> +	if (!xchk_fblock_process_error(sc, whichfork, 0, &error))
> +		goto out;
>  
>  	/* Check the btree structure. */
>  	cur = xfs_bmbt_init_cursor(mp, sc->tp, ip, whichfork);
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index e7d68318e6a55c..e7f9537634f3cf 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -225,11 +225,9 @@ xfs_bmap_count_blocks(
>  
>  	switch (ifp->if_format) {
>  	case XFS_DINODE_FMT_BTREE:
> -		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -			error = xfs_iread_extents(tp, ip, whichfork);
> -			if (error)
> -				return error;
> -		}
> +		error = xfs_iread_extents(tp, ip, whichfork);
> +		if (error)
> +			return error;
>  
>  		cur = xfs_bmbt_init_cursor(mp, tp, ip, whichfork);
>  		error = xfs_btree_count_blocks(cur, &btblocks);
> @@ -471,11 +469,9 @@ xfs_getbmap(
>  	first_bno = bno = XFS_BB_TO_FSBT(mp, bmv->bmv_offset);
>  	len = XFS_BB_TO_FSB(mp, bmv->bmv_length);
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, whichfork);
> -		if (error)
> -			goto out_unlock_ilock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, whichfork);
> +	if (error)
> +		goto out_unlock_ilock;
>  
>  	if (!xfs_iext_lookup_extent(ip, ifp, bno, &icur, &got)) {
>  		/*
> diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
> index 66deddd5e29698..477df4869d19bd 100644
> --- a/fs/xfs/xfs_dir2_readdir.c
> +++ b/fs/xfs/xfs_dir2_readdir.c
> @@ -258,11 +258,9 @@ xfs_dir2_leaf_readbuf(
>  	int			ra_want;
>  	int			error = 0;
>  
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
> -		if (error)
> -			goto out;
> -	}
> +	error = xfs_iread_extents(args->trans, dp, XFS_DATA_FORK);
> +	if (error)
> +		goto out;
>  
>  	/*
>  	 * Look for mapped directory blocks at or above the current offset.
> diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
> index bd8379b98374f8..83c5334fe978f2 100644
> --- a/fs/xfs/xfs_dquot.c
> +++ b/fs/xfs/xfs_dquot.c
> @@ -748,11 +748,9 @@ xfs_dq_get_next_id(
>  	start = (xfs_fsblock_t)next_id / mp->m_quotainfo->qi_dqperchunk;
>  
>  	lock_flags = xfs_ilock_data_map_shared(quotip);
> -	if (!(quotip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(NULL, quotip, XFS_DATA_FORK);
> +	if (error)
> +		return error;
>  
>  	if (xfs_iext_lookup_extent(quotip, &quotip->i_df, start, &cur, &got)) {
>  		/* contiguous chunk, bump startoff for the id calculation */
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index e17ab7f42928a5..129a0bafb46c0d 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -893,11 +893,9 @@ xfs_buffered_write_iomap_begin(
>  
>  	XFS_STATS_INC(mp, xs_blk_mapw);
>  
> -	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> -		if (error)
> -			goto out_unlock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> +	if (error)
> +		goto out_unlock;
>  
>  	/*
>  	 * Search the data fork first to look up our source mapping.  We
> @@ -1208,11 +1206,9 @@ xfs_seek_iomap_begin(
>  		return -EIO;
>  
>  	lockmode = xfs_ilock_data_map_shared(ip);
> -	if (!(ip->i_df.if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> -		if (error)
> -			goto out_unlock;
> -	}
> +	error = xfs_iread_extents(NULL, ip, XFS_DATA_FORK);
> +	if (error)
> +		goto out_unlock;
>  
>  	if (xfs_iext_lookup_extent(ip, &ip->i_df, offset_fsb, &icur, &imap)) {
>  		/*
> diff --git a/fs/xfs/xfs_qm.c b/fs/xfs/xfs_qm.c
> index 6fde318b9fed27..fd835a202c51eb 100644
> --- a/fs/xfs/xfs_qm.c
> +++ b/fs/xfs/xfs_qm.c
> @@ -1165,11 +1165,9 @@ xfs_qm_dqusage_adjust(
>  	if (XFS_IS_REALTIME_INODE(ip)) {
>  		struct xfs_ifork	*ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
>  
> -		if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -			error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> -			if (error)
> -				goto error0;
> -		}
> +		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> +		if (error)
> +			goto error0;
>  
>  		xfs_bmap_count_leaves(ifp, &rtblks);
>  	}
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 725c7d8e44381b..13cd62b89c4e30 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1392,11 +1392,9 @@ xfs_reflink_inode_has_shared_extents(
>  	int				error;
>  
>  	ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK);
> -	if (!(ifp->if_flags & XFS_IFEXTENTS)) {
> -		error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> -		if (error)
> -			return error;
> -	}
> +	error = xfs_iread_extents(tp, ip, XFS_DATA_FORK);
> +	if (error)
> +		return error;
>  
>  	*has_shared = false;
>  	found = xfs_iext_lookup_extent(ip, ifp, 0, &icur, &got);
> -- 
> 2.30.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
  2021-04-06 13:51   ` Darrick J. Wong
@ 2021-04-06 13:54     ` Christoph Hellwig
  0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2021-04-06 13:54 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs

On Tue, Apr 06, 2021 at 06:51:06AM -0700, Darrick J. Wong wrote:
> > index 5574d345d066ed..b8cab14ca8ce8d 100644
> > --- a/fs/xfs/libxfs/xfs_bmap.c
> > +++ b/fs/xfs/libxfs/xfs_bmap.c
> > @@ -1223,6 +1223,9 @@ xfs_iread_extents(
> >  
> >  	ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> 
> Since we now call xfs_iread_extents unconditionally, this assert will
> trip every time someone wants to bmapi_read the extent data after the
> extent data has been loaded in, because xfs_ilock_data_map_shared tells
> callers they only need to take ILOCK_SHARED... right?

Yes.  I've already moved this down in my local tree, as xfstests
generic/001 hits this once XFS_DEBUG is enabled.  I also had to fix
the inode variable name in a few places to even make that config compile.
As said this was _very_ lightly tested - I only did a quick run on a
non-debug config, but that passed with flying colors :)

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-04-06 13:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02 14:24 RFC: remove the if_flags field in struct xfs_ifork Christoph Hellwig
2021-04-02 14:24 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
2021-04-05 16:33   ` Brian Foster
2021-04-06 13:51   ` Darrick J. Wong
2021-04-06 13:54     ` Christoph Hellwig
2021-04-02 14:24 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
2021-04-05 16:33   ` Brian Foster
2021-04-02 14:24 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
2021-04-05 16:33   ` Brian Foster
2021-04-02 14:24 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
2021-04-05 16:35   ` Brian Foster
2021-04-02 14:24 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
2021-04-02 14:24 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
2021-04-02 21:48   ` kernel test robot
2021-04-02 21:48     ` kernel test robot
2021-04-03  3:31   ` kernel test robot
2021-04-03  3:31     ` kernel test robot
2021-04-02 14:24 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.