* [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-13 23:21 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
` (5 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 UTC (permalink / raw)
To: linux-xfs; +Cc: Brian Foster
Move the XFS_IFEXTENTS check from the callers into xfs_iread_extents to
simplify the code.
Signed-off-by: Christoph Hellwig <hch@lst.de>
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 402ecd6103605e..1b1b58af41fa7f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1227,6 +1227,9 @@ xfs_iread_extents(
struct xfs_btree_cur *cur;
int error;
+ if (ifp->if_flags & XFS_IFEXTENTS)
+ return 0;
+
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
@@ -1284,11 +1287,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) {
@@ -1336,11 +1337,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;
@@ -1359,11 +1358,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))
@@ -3991,11 +3988,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;
@@ -4475,11 +4470,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;
@@ -4758,11 +4751,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. */
@@ -5433,9 +5424,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;
@@ -5921,11 +5913,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);
@@ -6038,11 +6028,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);
@@ -6141,12 +6129,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 613e2aa7e4e7f1..924d7e343731de 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -448,11 +448,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 e79e3d1ff38dfe..1c7116abff0d69 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 03e7c39a07807a..1d2fe48ad19fb7 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 7fb63a04400fab..ecd5059d6928f7 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, "ip->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 9c7bd2d94d8d89..d37d42e554a12b 100644
--- a/fs/xfs/xfs_iomap.c
+++ b/fs/xfs/xfs_iomap.c
@@ -894,11 +894,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
@@ -1209,11 +1207,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 134d5a11eb2205..4bf949a89d0d6d 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 323506a6b33947..4dd4af6ac2ef53 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] 23+ messages in thread* Re: [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents
2021-04-12 13:38 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
@ 2021-04-13 23:21 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-13 23:21 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, Brian Foster
On Mon, Apr 12, 2021 at 03:38:13PM +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>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
Looks better this time,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 402ecd6103605e..1b1b58af41fa7f 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1227,6 +1227,9 @@ xfs_iread_extents(
> struct xfs_btree_cur *cur;
> int error;
>
> + if (ifp->if_flags & XFS_IFEXTENTS)
> + return 0;
> +
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>
> if (XFS_IS_CORRUPT(mp, ifp->if_format != XFS_DINODE_FMT_BTREE)) {
> @@ -1284,11 +1287,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) {
> @@ -1336,11 +1337,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;
> @@ -1359,11 +1358,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))
> @@ -3991,11 +3988,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;
> @@ -4475,11 +4470,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;
> @@ -4758,11 +4751,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. */
> @@ -5433,9 +5424,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;
> @@ -5921,11 +5913,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);
> @@ -6038,11 +6028,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);
> @@ -6141,12 +6129,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 613e2aa7e4e7f1..924d7e343731de 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -448,11 +448,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 e79e3d1ff38dfe..1c7116abff0d69 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 03e7c39a07807a..1d2fe48ad19fb7 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 7fb63a04400fab..ecd5059d6928f7 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, "ip->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 9c7bd2d94d8d89..d37d42e554a12b 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -894,11 +894,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
> @@ -1209,11 +1207,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 134d5a11eb2205..4bf949a89d0d6d 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 323506a6b33947..4dd4af6ac2ef53 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] 23+ messages in thread
* [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
2021-04-12 13:38 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-14 0:05 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
` (4 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 UTC (permalink / raw)
To: linux-xfs; +Cc: Brian Foster
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 902e5f7e664231..fd61c67f573925 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);
@@ -1283,7 +1303,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 1b1b58af41fa7f..e32b8228d9cc2e 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1441,38 +1441,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 i_disk_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 a49df4092c304b..f9a390ecfb1d9a 100644
--- a/fs/xfs/libxfs/xfs_bmap.h
+++ b/fs/xfs/libxfs/xfs_bmap.h
@@ -200,7 +200,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] 23+ messages in thread* Re: [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block
2021-04-12 13:38 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
@ 2021-04-14 0:05 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:05 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, Brian Foster
On Mon, Apr 12, 2021 at 03:38:14PM +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 902e5f7e664231..fd61c67f573925 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;
/me kinda wonders if this ought to be mp->m_attr_geo->fsbcount, but
seeing as adding support for large xattr blocks would require a full
audit anyway, it's probably not worth fussing over.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> +}
> +
> /*========================================================================
> * 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);
> @@ -1283,7 +1303,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 1b1b58af41fa7f..e32b8228d9cc2e 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1441,38 +1441,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 i_disk_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 a49df4092c304b..f9a390ecfb1d9a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.h
> +++ b/fs/xfs/libxfs/xfs_bmap.h
> @@ -200,7 +200,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] 23+ messages in thread
* [PATCH 3/7] xfs: simplify xfs_attr_remove_args
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
2021-04-12 13:38 ` [PATCH 1/7] xfs: move the XFS_IFEXTENTS check into xfs_iread_extents Christoph Hellwig
2021-04-12 13:38 ` [PATCH 2/7] xfs: rename and simplify xfs_bmap_one_block Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-14 0:06 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
` (3 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 UTC (permalink / raw)
To: linux-xfs; +Cc: Brian Foster
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 fd61c67f573925..43ef85678cba6b 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] 23+ messages in thread* Re: [PATCH 3/7] xfs: simplify xfs_attr_remove_args
2021-04-12 13:38 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
@ 2021-04-14 0:06 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:06 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, Brian Foster
On Mon, Apr 12, 2021 at 03:38:15PM +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>
Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 fd61c67f573925..43ef85678cba6b 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] 23+ messages in thread
* [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
` (2 preceding siblings ...)
2021-04-12 13:38 ` [PATCH 3/7] xfs: simplify xfs_attr_remove_args Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-14 0:08 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
` (2 subsequent siblings)
6 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 UTC (permalink / raw)
To: linux-xfs; +Cc: Brian Foster
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>
---
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 a8800ff03f9432..73eea7939b55e4 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] 23+ messages in thread* Re: [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork
2021-04-12 13:38 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
@ 2021-04-14 0:08 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:08 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs, Brian Foster
On Mon, Apr 12, 2021 at 03:38:16PM +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>
Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 a8800ff03f9432..73eea7939b55e4 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] 23+ messages in thread
* [PATCH 5/7] xfs: remove XFS_IFBROOT
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
` (3 preceding siblings ...)
2021-04-12 13:38 ` [PATCH 4/7] xfs: only look at the fork format in xfs_idestroy_fork Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
2021-04-14 0:13 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
2021-04-12 13:38 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
6 siblings, 2 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 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 e32b8228d9cc2e..580b36f19a26f7 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -633,7 +633,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;
@@ -677,7 +676,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.
@@ -4196,7 +4194,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
@@ -4269,7 +4267,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);
}
@@ -4732,7 +4730,7 @@ xfs_bmapi_remap(
ip->i_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;
}
@@ -5411,7 +5409,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;
@@ -5885,7 +5883,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;
}
@@ -6000,7 +5998,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;
}
@@ -6115,7 +6113,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 73eea7939b55e4..02ad722004d3f4 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 06682ff49a5bfc..8ffaa7cc1f7c3f 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] 23+ messages in thread* Re: [PATCH 5/7] xfs: remove XFS_IFBROOT
2021-04-12 13:38 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
@ 2021-04-12 16:03 ` Brian Foster
2021-04-14 15:23 ` Darrick J. Wong
2021-04-14 0:13 ` Darrick J. Wong
1 sibling, 1 reply; 23+ messages in thread
From: Brian Foster @ 2021-04-12 16:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:17PM +0200, Christoph Hellwig wrote:
> 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_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,
IIRC, these bits are used in xfsprogs for efficient btree repair. Taking
a closer look, I see AG metadata btree repair implementations, but
nothing that seems to use the ifake variant. Am I missing something or
is this code currently unused?
In any event, the comments for xfs_btree_stage_ifakeroot() suggest that
->if_format should be initialized properly when a fake inode fork is
transferred to a cursor and the rest of the patch looks fine to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 73eea7939b55e4..02ad722004d3f4 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 06682ff49a5bfc..8ffaa7cc1f7c3f 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 [flat|nested] 23+ messages in thread* Re: [PATCH 5/7] xfs: remove XFS_IFBROOT
2021-04-12 16:03 ` Brian Foster
@ 2021-04-14 15:23 ` Darrick J. Wong
0 siblings, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 15:23 UTC (permalink / raw)
To: Brian Foster; +Cc: Christoph Hellwig, linux-xfs
On Mon, Apr 12, 2021 at 12:03:42PM -0400, Brian Foster wrote:
> On Mon, Apr 12, 2021 at 03:38:17PM +0200, Christoph Hellwig wrote:
> > 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_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,
>
> IIRC, these bits are used in xfsprogs for efficient btree repair. Taking
> a closer look, I see AG metadata btree repair implementations, but
> nothing that seems to use the ifake variant. Am I missing something or
> is this code currently unused?
Currently unused. The first user will be bmbt reconstruction, whenever
I get back to that, some day. Now that upper levels can relog deferred
ops when the log head approaches the tail, online btree repair can hang
on to EFIs for the new tree blocks during the bulkload without risking a
log livelock.
That series is stuck behind:
1 Quotaoff cleanups (suggested as part of:)
2 Incore inode walk refactoring (suggested as part of:)
3 Preserving inode sickness reports through aborted ireclaim (bugs
found while working on:)
4 Deferred inode inactivation (customer requirement / needed for rmapbt
repairs)
5 Reducing transaction reservations when reflink/rmap are enabled
(dchinner complaint)
6 Various scrub enhancements
7 Repair infrastructure changes needed for online bulkloading
8 Online repair of AG btrees
It's 9th in line (and 81st in the patch stack), so it's going to be at
least another ~5 kernel cycles until review completes on the preceeding
patchsets so that repair gets back to the front of the line.
--D
> In any event, the comments for xfs_btree_stage_ifakeroot() suggest that
> ->if_format should be initialized properly when a fake inode fork is
> transferred to a cursor and the rest of the patch looks fine to me:
>
> Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> > diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> > index 73eea7939b55e4..02ad722004d3f4 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 06682ff49a5bfc..8ffaa7cc1f7c3f 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 [flat|nested] 23+ messages in thread
* Re: [PATCH 5/7] xfs: remove XFS_IFBROOT
2021-04-12 13:38 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
@ 2021-04-14 0:13 ` Darrick J. Wong
1 sibling, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:17PM +0200, Christoph Hellwig wrote:
> 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>
Looks pretty straightforward to me...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 e32b8228d9cc2e..580b36f19a26f7 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -633,7 +633,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;
> @@ -677,7 +676,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.
> @@ -4196,7 +4194,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
> @@ -4269,7 +4267,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);
> }
> @@ -4732,7 +4730,7 @@ xfs_bmapi_remap(
> ip->i_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;
> }
> @@ -5411,7 +5409,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;
> @@ -5885,7 +5883,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;
> }
> @@ -6000,7 +5998,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;
> }
> @@ -6115,7 +6113,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 73eea7939b55e4..02ad722004d3f4 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 06682ff49a5bfc..8ffaa7cc1f7c3f 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 [flat|nested] 23+ messages in thread
* [PATCH 6/7] xfs: remove XFS_IFINLINE
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
` (4 preceding siblings ...)
2021-04-12 13:38 ` [PATCH 5/7] xfs: remove XFS_IFBROOT Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
2021-04-14 0:33 ` Darrick J. Wong
2021-04-12 13:38 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
6 siblings, 2 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 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 43ef85678cba6b..96146f425e503d 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 23e2bf3341a015..1ab7a73b5a9a46 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_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 580b36f19a26f7..0af3edf8443c73 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -805,7 +805,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;
@@ -850,7 +849,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 7824af54637513..75e1421f69c458 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_disk_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 bd89de61301c85..b031be033838f6 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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_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 02ad722004d3f4..3f2c16bf82e8c6 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 8ffaa7cc1f7c3f..ac8b2182ce8c57 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 ad7b85e248c78e..599ee277bba2f4 100644
--- a/fs/xfs/scrub/symlink.c
+++ b/fs/xfs/scrub/symlink.c
@@ -51,7 +51,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 1d2fe48ad19fb7..da1cc683560c75 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_disk_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 607b3f263b0644..8f2f74a496bd24 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(ip->i_df.if_format == XFS_DINODE_FMT_LOCAL);
/*
* The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
@@ -1401,7 +1401,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 1a625920ddff94..d4b3567d87943f 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(ip->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] 23+ messages in thread* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
2021-04-12 13:38 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
@ 2021-04-12 16:03 ` Brian Foster
2021-04-14 0:33 ` Darrick J. Wong
1 sibling, 0 replies; 23+ messages in thread
From: Brian Foster @ 2021-04-12 16:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:18PM +0200, Christoph Hellwig wrote:
> 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>
> ---
Reviewed-by: Brian Foster <bfoster@redhat.com>
> 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 43ef85678cba6b..96146f425e503d 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 23e2bf3341a015..1ab7a73b5a9a46 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_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 580b36f19a26f7..0af3edf8443c73 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -805,7 +805,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;
> @@ -850,7 +849,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 7824af54637513..75e1421f69c458 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_disk_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 bd89de61301c85..b031be033838f6 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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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 02ad722004d3f4..3f2c16bf82e8c6 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 8ffaa7cc1f7c3f..ac8b2182ce8c57 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 ad7b85e248c78e..599ee277bba2f4 100644
> --- a/fs/xfs/scrub/symlink.c
> +++ b/fs/xfs/scrub/symlink.c
> @@ -51,7 +51,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 1d2fe48ad19fb7..da1cc683560c75 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_disk_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 607b3f263b0644..8f2f74a496bd24 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(ip->i_df.if_format == XFS_DINODE_FMT_LOCAL);
>
> /*
> * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
> @@ -1401,7 +1401,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 1a625920ddff94..d4b3567d87943f 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(ip->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 [flat|nested] 23+ messages in thread* Re: [PATCH 6/7] xfs: remove XFS_IFINLINE
2021-04-12 13:38 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
@ 2021-04-14 0:33 ` Darrick J. Wong
1 sibling, 0 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:18PM +0200, Christoph Hellwig wrote:
> 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>
Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 43ef85678cba6b..96146f425e503d 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 23e2bf3341a015..1ab7a73b5a9a46 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_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 580b36f19a26f7..0af3edf8443c73 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -805,7 +805,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;
> @@ -850,7 +849,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 7824af54637513..75e1421f69c458 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_disk_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 bd89de61301c85..b031be033838f6 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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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_disk_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_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_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 02ad722004d3f4..3f2c16bf82e8c6 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 8ffaa7cc1f7c3f..ac8b2182ce8c57 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 ad7b85e248c78e..599ee277bba2f4 100644
> --- a/fs/xfs/scrub/symlink.c
> +++ b/fs/xfs/scrub/symlink.c
> @@ -51,7 +51,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 1d2fe48ad19fb7..da1cc683560c75 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_disk_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 607b3f263b0644..8f2f74a496bd24 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(ip->i_df.if_format == XFS_DINODE_FMT_LOCAL);
>
> /*
> * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if
> @@ -1401,7 +1401,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 1a625920ddff94..d4b3567d87943f 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(ip->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 [flat|nested] 23+ messages in thread
* [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-12 13:38 remove the if_flags field in struct xfs_ifork Christoph Hellwig
` (5 preceding siblings ...)
2021-04-12 13:38 ` [PATCH 6/7] xfs: remove XFS_IFINLINE Christoph Hellwig
@ 2021-04-12 13:38 ` Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
2021-04-14 0:37 ` Darrick J. Wong
6 siblings, 2 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-12 13:38 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 | 10 ++--------
fs/xfs/xfs_ioctl.c | 2 +-
fs/xfs/xfs_iomap.c | 4 ++--
fs/xfs/xfs_symlink.c | 2 +-
12 files changed, 19 insertions(+), 49 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 1ab7a73b5a9a46..556184b6306105 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 0af3edf8443c73..7e3b9b01431e57 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -605,7 +605,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);
@@ -805,7 +805,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;
@@ -849,7 +848,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;
@@ -1098,7 +1096,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:
@@ -1224,16 +1221,11 @@ xfs_iread_extents(
struct xfs_btree_cur *cur;
int error;
- if (ifp->if_flags & XFS_IFEXTENTS)
+ if (!xfs_need_iread_extents(ifp))
return 0;
ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
- 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);
@@ -1248,8 +1240,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 b031be033838f6..46d18bf9d5e158 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 3f2c16bf82e8c6..1d174909f9bdf5 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 ac8b2182ce8c57..a6f7897b6887b1 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 924d7e343731de..b5ebf1d1b4db45 100644
--- a/fs/xfs/scrub/bmap.c
+++ b/fs/xfs/scrub/bmap.c
@@ -447,7 +447,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))
@@ -673,10 +673,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 fb66c5d20d261b..9b08db45ce8549 100644
--- a/fs/xfs/xfs_aops.c
+++ b/fs/xfs/xfs_aops.c
@@ -291,8 +291,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 1c7116abff0d69..a5e9d7d34023f2 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))
@@ -625,7 +625,7 @@ xfs_can_free_eofblocks(
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 fa2d377e251415..17c2d8b18283c9 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;
@@ -843,7 +840,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:
@@ -855,7 +851,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;
@@ -875,7 +870,6 @@ xfs_init_new_inode(
if (init_xattrs && xfs_sb_version_hasattr(&mp->m_sb)) {
ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
ip->i_afp = xfs_ifork_alloc(XFS_DINODE_FMT_EXTENTS, 0);
- ip->i_afp->if_flags = XFS_IFEXTENTS;
}
/*
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 708b77341a703f..bf490bfae6cbb1 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1126,7 +1126,7 @@ xfs_fill_fsxattr(
if (ip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE)
fa->fsx_cowextsize = XFS_FSB_TO_B(mp, ip->i_cowextsize);
fa->fsx_projid = ip->i_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 d37d42e554a12b..d154f42e2dc684 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.
@@ -667,7 +667,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 d4b3567d87943f..b4fa702823834b 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] 23+ messages in thread* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-12 13:38 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
@ 2021-04-12 16:03 ` Brian Foster
2021-04-14 0:37 ` Darrick J. Wong
1 sibling, 0 replies; 23+ messages in thread
From: Brian Foster @ 2021-04-12 16:03 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:19PM +0200, Christoph Hellwig wrote:
> 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>
> ---
Looks sane to me:
Reviewed-by: Brian Foster <bfoster@redhat.com>
> 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 | 10 ++--------
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_iomap.c | 4 ++--
> fs/xfs/xfs_symlink.c | 2 +-
> 12 files changed, 19 insertions(+), 49 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 1ab7a73b5a9a46..556184b6306105 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 0af3edf8443c73..7e3b9b01431e57 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -605,7 +605,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);
> @@ -805,7 +805,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;
> @@ -849,7 +848,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;
> @@ -1098,7 +1096,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:
> @@ -1224,16 +1221,11 @@ xfs_iread_extents(
> struct xfs_btree_cur *cur;
> int error;
>
> - if (ifp->if_flags & XFS_IFEXTENTS)
> + if (!xfs_need_iread_extents(ifp))
> return 0;
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>
> - 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);
> @@ -1248,8 +1240,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 b031be033838f6..46d18bf9d5e158 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 3f2c16bf82e8c6..1d174909f9bdf5 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 ac8b2182ce8c57..a6f7897b6887b1 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 924d7e343731de..b5ebf1d1b4db45 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -447,7 +447,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))
> @@ -673,10 +673,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 fb66c5d20d261b..9b08db45ce8549 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -291,8 +291,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 1c7116abff0d69..a5e9d7d34023f2 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))
> @@ -625,7 +625,7 @@ xfs_can_free_eofblocks(
> 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 fa2d377e251415..17c2d8b18283c9 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;
> @@ -843,7 +840,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:
> @@ -855,7 +851,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;
> @@ -875,7 +870,6 @@ xfs_init_new_inode(
> if (init_xattrs && xfs_sb_version_hasattr(&mp->m_sb)) {
> ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
> ip->i_afp = xfs_ifork_alloc(XFS_DINODE_FMT_EXTENTS, 0);
> - ip->i_afp->if_flags = XFS_IFEXTENTS;
> }
>
> /*
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 708b77341a703f..bf490bfae6cbb1 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1126,7 +1126,7 @@ xfs_fill_fsxattr(
> if (ip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE)
> fa->fsx_cowextsize = XFS_FSB_TO_B(mp, ip->i_cowextsize);
> fa->fsx_projid = ip->i_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 d37d42e554a12b..d154f42e2dc684 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.
> @@ -667,7 +667,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 d4b3567d87943f..b4fa702823834b 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 [flat|nested] 23+ messages in thread* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-12 13:38 ` [PATCH 7/7] xfs: remove XFS_IFEXTENTS Christoph Hellwig
2021-04-12 16:03 ` Brian Foster
@ 2021-04-14 0:37 ` Darrick J. Wong
2021-04-14 5:59 ` Christoph Hellwig
2021-04-15 23:14 ` Dave Chinner
1 sibling, 2 replies; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 0:37 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Mon, Apr 12, 2021 at 03:38:19PM +0200, Christoph Hellwig wrote:
> 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>
The series seems reasonable to me, though I wonder if Dave will have
strong(er) opinions?
<shrug> Seeing how we already had a go-round where Dave and stumbled
over each other about the somewhat duplicative flags and format fields
I'm inclined to take this sooner or later just to eliminate the
ambiguity...
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 | 10 ++--------
> fs/xfs/xfs_ioctl.c | 2 +-
> fs/xfs/xfs_iomap.c | 4 ++--
> fs/xfs/xfs_symlink.c | 2 +-
> 12 files changed, 19 insertions(+), 49 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 1ab7a73b5a9a46..556184b6306105 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 0af3edf8443c73..7e3b9b01431e57 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -605,7 +605,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);
> @@ -805,7 +805,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;
> @@ -849,7 +848,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;
> @@ -1098,7 +1096,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:
> @@ -1224,16 +1221,11 @@ xfs_iread_extents(
> struct xfs_btree_cur *cur;
> int error;
>
> - if (ifp->if_flags & XFS_IFEXTENTS)
> + if (!xfs_need_iread_extents(ifp))
> return 0;
>
> ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
>
> - 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);
> @@ -1248,8 +1240,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 b031be033838f6..46d18bf9d5e158 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 3f2c16bf82e8c6..1d174909f9bdf5 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 ac8b2182ce8c57..a6f7897b6887b1 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 924d7e343731de..b5ebf1d1b4db45 100644
> --- a/fs/xfs/scrub/bmap.c
> +++ b/fs/xfs/scrub/bmap.c
> @@ -447,7 +447,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))
> @@ -673,10 +673,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 fb66c5d20d261b..9b08db45ce8549 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -291,8 +291,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 1c7116abff0d69..a5e9d7d34023f2 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))
> @@ -625,7 +625,7 @@ xfs_can_free_eofblocks(
> 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 fa2d377e251415..17c2d8b18283c9 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;
> @@ -843,7 +840,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:
> @@ -855,7 +851,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;
> @@ -875,7 +870,6 @@ xfs_init_new_inode(
> if (init_xattrs && xfs_sb_version_hasattr(&mp->m_sb)) {
> ip->i_forkoff = xfs_default_attroffset(ip) >> 3;
> ip->i_afp = xfs_ifork_alloc(XFS_DINODE_FMT_EXTENTS, 0);
> - ip->i_afp->if_flags = XFS_IFEXTENTS;
> }
>
> /*
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 708b77341a703f..bf490bfae6cbb1 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1126,7 +1126,7 @@ xfs_fill_fsxattr(
> if (ip->i_diflags2 & XFS_DIFLAG2_COWEXTSIZE)
> fa->fsx_cowextsize = XFS_FSB_TO_B(mp, ip->i_cowextsize);
> fa->fsx_projid = ip->i_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 d37d42e554a12b..d154f42e2dc684 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.
> @@ -667,7 +667,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 d4b3567d87943f..b4fa702823834b 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 [flat|nested] 23+ messages in thread* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-14 0:37 ` Darrick J. Wong
@ 2021-04-14 5:59 ` Christoph Hellwig
2021-04-14 15:27 ` Darrick J. Wong
2021-04-15 23:14 ` Dave Chinner
1 sibling, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-14 5:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Tue, Apr 13, 2021 at 05:37:44PM -0700, Darrick J. Wong wrote:
> <shrug> Seeing how we already had a go-round where Dave and stumbled
> over each other about the somewhat duplicative flags and format fields
> I'm inclined to take this sooner or later just to eliminate the
> ambiguity...
Do you want me to resend for the comment that Brian wants to see, or
do you want to just fold that in?
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-14 5:59 ` Christoph Hellwig
@ 2021-04-14 15:27 ` Darrick J. Wong
2021-04-14 15:29 ` Christoph Hellwig
0 siblings, 1 reply; 23+ messages in thread
From: Darrick J. Wong @ 2021-04-14 15:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: linux-xfs
On Wed, Apr 14, 2021 at 07:59:23AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 13, 2021 at 05:37:44PM -0700, Darrick J. Wong wrote:
> > <shrug> Seeing how we already had a go-round where Dave and stumbled
> > over each other about the somewhat duplicative flags and format fields
> > I'm inclined to take this sooner or later just to eliminate the
> > ambiguity...
>
> Do you want me to resend for the comment that Brian wants to see, or
> do you want to just fold that in?
Hm? I thought Brian was describing a comment that's already in
xfs_btree_staging.c around line 195 as his reason for adding an RVB?
--D
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-14 15:27 ` Darrick J. Wong
@ 2021-04-14 15:29 ` Christoph Hellwig
0 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-04-14 15:29 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Wed, Apr 14, 2021 at 08:27:18AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 14, 2021 at 07:59:23AM +0200, Christoph Hellwig wrote:
> > On Tue, Apr 13, 2021 at 05:37:44PM -0700, Darrick J. Wong wrote:
> > > <shrug> Seeing how we already had a go-round where Dave and stumbled
> > > over each other about the somewhat duplicative flags and format fields
> > > I'm inclined to take this sooner or later just to eliminate the
> > > ambiguity...
> >
> > Do you want me to resend for the comment that Brian wants to see, or
> > do you want to just fold that in?
>
> Hm? I thought Brian was describing a comment that's already in
> xfs_btree_staging.c around line 195 as his reason for adding an RVB?
Oh, indeed. I somehow misread his mail as asking to add this comment.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 7/7] xfs: remove XFS_IFEXTENTS
2021-04-14 0:37 ` Darrick J. Wong
2021-04-14 5:59 ` Christoph Hellwig
@ 2021-04-15 23:14 ` Dave Chinner
1 sibling, 0 replies; 23+ messages in thread
From: Dave Chinner @ 2021-04-15 23:14 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, linux-xfs
On Tue, Apr 13, 2021 at 05:37:44PM -0700, Darrick J. Wong wrote:
> On Mon, Apr 12, 2021 at 03:38:19PM +0200, Christoph Hellwig wrote:
> > 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>
>
> The series seems reasonable to me, though I wonder if Dave will have
> strong(er) opinions?
No, I've already said I'm fine with making these changes to remove
the ambiguity. I've only briefly scan of the patches, but I didn't
see anything that I disagree with that would make me stop, context
switch and ask to be changed...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 23+ messages in thread