On Tue, Oct 24, 2023 at 08:44:16AM +0200, Christoph Hellwig wrote:
> Update the per-folio stable writes flag dependening on which device an
> inode resides on.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_inode.h | 8 ++++++++
> fs/xfs/xfs_ioctl.c | 9 +++++++++
> fs/xfs/xfs_iops.c | 7 +++++++
> 3 files changed, 24 insertions(+)
>
> diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> index 0c5bdb91152e1c..682959c8f78cb0 100644
> --- a/fs/xfs/xfs_inode.h
> +++ b/fs/xfs/xfs_inode.h
> @@ -561,6 +561,14 @@ extern void xfs_setup_inode(struct xfs_inode *ip);
> extern void xfs_setup_iops(struct xfs_inode *ip);
> extern void xfs_diflags_to_iflags(struct xfs_inode *ip, bool init);
>
> +static inline void xfs_update_stable_writes(struct xfs_inode *ip)
> +{
> + if (bdev_stable_writes(xfs_inode_buftarg(ip)->bt_bdev))
> + mapping_set_stable_writes(VFS_I(ip)->i_mapping);
> + else
> + mapping_clear_stable_writes(VFS_I(ip)->i_mapping);
> +}
> +
> /*
> * When setting up a newly allocated inode, we need to call
> * xfs_finish_inode_setup() once the inode is fully instantiated at
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 55bb01173cde8c..67bf613b3c86bc 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1147,6 +1147,15 @@ xfs_ioctl_setattr_xflags(
> ip->i_diflags2 = i_flags2;
>
> xfs_diflags_to_iflags(ip, false);
> +
> + /*
> + * Make the stable writes flag match that of the device the inode
> + * resides on when flipping the RT flag.
> + */
> + if (S_ISREG(VFS_I(ip)->i_mode) &&
> + XFS_IS_REALTIME_INODE(ip) != (fa->fsx_xflags & FS_XFLAG_REALTIME))
> + xfs_update_stable_writes(ip);
Hmm. Won't the masking operation here result in the if test comparing 0
or FS_XFLAG_REALTIME to 0 or 1?
Oh. FS_XFLAG_REALTIME == 1, so that's not an issue in this one case.
That's a bit subtle though, I'd have preferred
XFS_IS_REALTIME_INODE(ip) != !!(fa->fsx_xflags & FS_XFLAG_REALTIME))
to make it more obvious that the if test isn't comparing apples to
oranges.
> +
> xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG);
> xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
> XFS_STATS_INC(mp, xs_ig_attrchg);
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 2b3b05c28e9e48..b8ec045708c318 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -1298,6 +1298,13 @@ xfs_setup_inode(
> gfp_mask = mapping_gfp_mask(inode->i_mapping);
> mapping_set_gfp_mask(inode->i_mapping, (gfp_mask & ~(__GFP_FS)));
>
> + /*
> + * For real-time inodes update the stable write flags to that of the RT
> + * device instead of the data device.
> + */
> + if (S_ISREG(inode->i_mode) && XFS_IS_REALTIME_INODE(ip))
> + xfs_update_stable_writes(ip);
I wonder if xfs_update_stable_writes should become an empty function for
the CONFIG_XFS_RT=n case, to avoid the atomic flags update?
(The extra code is probably not worth the microoptimization.)
With the !! nit addressed,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> +
> /*
> * If there is no attribute fork no ACL can exist on this inode,
> * and it can't have any file capabilities attached to it either.
> --
> 2.39.2
>
