On Thu, May 20, 2021 at 09:42:27AM -0700, Darrick J. Wong wrote:
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 045118c7bf78..dce267dbea5f 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -589,8 +589,21 @@ xfs_inode_validate_extsize(
> inherit_flag = (flags & XFS_DIFLAG_EXTSZINHERIT);
> extsize_bytes = XFS_FSB_TO_B(mp, extsize);
>
> + /*
> + * Historically, XFS didn't check that the extent size hint was an
> + * integer multiple of the rt extent size on a directory with both
> + * rtinherit and extszinherit flags set. This results in math errors
> + * in the rt allocator and inode verifier errors when the unaligned
> + * hint value propagates into new realtime files. Since there might
> + * be filesystems in the wild, the best we can do for now is to
> + * mitigate the harms by stopping the propagation.
> + *
> + * The next time we add a new inode feature, the if test below should
> + * also trigger if that new feature is enabled and (rtinherit_flag &&
> + * inherit_flag).
> + */
I don't understand how this comment relates to the change below, and
in fact I don't understand the last paragraph at all.
> if (rt_flag)
> - blocksize_bytes = mp->m_sb.sb_rextsize << mp->m_sb.sb_blocklog;
> + blocksize_bytes = XFS_FSB_TO_B(mp, mp->m_sb.sb_rextsize);
This just looks like a cleanup, that is replace the open coded version
of XFS_FSB_TO_B with the actual helper.
> + /*
> + * Clear invalid extent size hints set on files with rtinherit and
> + * extszinherit set. See the comments in xfs_inode_validate_extsize
> + * for details.
> + */
Maybe that comment should be here as it makes a whole lot more sense
where we do the actual clearing.
> + if ((ip->i_diflags & XFS_DIFLAG_RTINHERIT) &&
> + (ip->i_diflags & XFS_DIFLAG_EXTSZINHERIT) &&
> + (ip->i_extsize % ip->i_mount->m_sb.sb_rextsize) > 0) {
> + ip->i_diflags &= ~(XFS_DIFLAG_EXTSIZE | XFS_DIFLAG_EXTSZINHERIT);
Overly long line.
> + xfs_failaddr_t failaddr;
> umode_t mode = VFS_I(ip)->i_mode;
>
> if (S_ISDIR(mode)) {
> if (pip->i_diflags & XFS_DIFLAG_RTINHERIT)
> - di_flags |= XFS_DIFLAG_RTINHERIT;
> + ip->i_diflags |= XFS_DIFLAG_RTINHERIT;
The removal of the di_flags seems like an unrelated (though nice)
cleanup.
