On Mon, Aug 28, 2017 at 11:17:01AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Move the v3 inode integrity information (crc, owner, metauuid) before we
> look at anything else in the inode so that we don't waste time on a torn
> write or a totally garbled block. This makes xfs_dinode_verify more
> consistent with the other verifiers.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
Hmm, was there a practical reason for doing this? In the common case,
the filesystem is not corrupted and we have to pass all the checks
anyways. I'd just say keep the code flow simple rather than worry about
optimizing the error case, unless there's some reason to prioritize some
error checks over others..?
Brian
> fs/xfs/libxfs/xfs_inode_buf.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
>
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index a5bcf2b..b2423a3 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -393,6 +393,19 @@ xfs_dinode_verify(
> if (dip->di_magic != cpu_to_be16(XFS_DINODE_MAGIC))
> return __this_address;
>
> + /* Verify v3 integrity information first */
> + if (dip->di_version >= 3) {
> + if (!xfs_sb_version_hascrc(&mp->m_sb))
> + return __this_address;
> + if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
> + XFS_DINODE_CRC_OFF))
> + return __this_address;
> + if (be64_to_cpu(dip->di_ino) != ino)
> + return __this_address;
> + if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
> + return __this_address;
> + }
> +
> /* don't allow invalid i_size */
> if (be64_to_cpu(dip->di_size) & (1ULL << 63))
> return __this_address;
> @@ -409,16 +422,6 @@ xfs_dinode_verify(
> if (dip->di_version < 3)
> return NULL;
>
> - if (!xfs_sb_version_hascrc(&mp->m_sb))
> - return __this_address;
> - if (!xfs_verify_cksum((char *)dip, mp->m_sb.sb_inodesize,
> - XFS_DINODE_CRC_OFF))
> - return __this_address;
> - if (be64_to_cpu(dip->di_ino) != ino)
> - return __this_address;
> - if (!uuid_equal(&dip->di_uuid, &mp->m_sb.sb_meta_uuid))
> - return __this_address;
> -
> flags = be16_to_cpu(dip->di_flags);
> flags2 = be64_to_cpu(dip->di_flags2);
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
