On Mon, Apr 10, 2023 at 06:06:38PM -0700, Darrick J. Wong wrote:
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -1171,6 +1171,8 @@ xfs_iread_extents(
> goto out;
> }
> ASSERT(ir.loaded == xfs_iext_count(ifp));
> + smp_mb();
> + ifp->if_needextents = 0;
Shouldn't this be a WRITE_ONCE?
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 6b21760184d9..eadae924dc42 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -174,6 +174,8 @@ xfs_iformat_btree(
> int level;
>
> ifp = xfs_ifork_ptr(ip, whichfork);
> + ifp->if_needextents = 1;
Same here.
> dfp = (xfs_bmdr_block_t *)XFS_DFORK_PTR(dip, whichfork);
> size = XFS_BMAP_BROOT_SPACE(mp, dfp);
> nrecs = be16_to_cpu(dfp->bb_numrecs);
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
> index d3943d6ad0b9..e69c78c35c96 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.h
> +++ b/fs/xfs/libxfs/xfs_inode_fork.h
> @@ -24,6 +24,7 @@ struct xfs_ifork {
> xfs_extnum_t if_nextents; /* # of extents in this fork */
> short if_broot_bytes; /* bytes allocated for root */
> int8_t if_format; /* format of this fork */
> + int8_t if_needextents; /* extents have not been read */
> };
I don't think this should be signed.
> static inline bool xfs_need_iread_extents(struct xfs_ifork *ifp)
> {
> - return ifp->if_format == XFS_DINODE_FMT_BTREE && ifp->if_height == 0;
> + return ifp->if_format == XFS_DINODE_FMT_BTREE && ifp->if_needextents;
.. and this should use READ_ONCE and drop the if_format check given
that if_needextents is only set in xfs_iformat_btree.
