On Tue, Mar 24, 2020 at 08:24:43PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Validate the geometry of the realtime geometry when we mount the
> filesystem, so that we don't abruptly shut down the filesystem later on.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_sb.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 2f60fc3c99a0..dee0a1a594dc 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -328,6 +328,41 @@ xfs_validate_sb_common(
> return -EFSCORRUPTED;
> }
>
> + /* Validate the realtime geometry; stolen from xfs_repair */
> + if (unlikely(
> + sbp->sb_rextsize * sbp->sb_blocksize > XFS_MAX_RTEXTSIZE ||
Whacky whitespace before the ||
> + sbp->sb_rextsize * sbp->sb_blocksize < XFS_MIN_RTEXTSIZE)) {
> + xfs_notice(mp,
> + "realtime extent sanity check failed");
> + return -EFSCORRUPTED;
> + }
We really don't need unlikely() in code like this. the compiler
already considers code that returns inside an if statement as
"unlikely" because it's the typical error handling pattern, for
cases like this it really isn't necessary.
> +
> + if (sbp->sb_rblocks == 0) {
> + if (unlikely(
> + sbp->sb_rextents != 0 ||
> + sbp->sb_rbmblocks != 0 ||
> + sbp->sb_rextslog != 0 ||
> + sbp->sb_frextents != 0)) {
Ditto on the unlikely and the whitespace. That code looks weird...
if (sbp->sb_rextents || sbp->sb_rbmblocks ||
sbp->sb_rextslog || sbp->sb_frextents) {
> + xfs_notice(mp,
> + "realtime zeroed geometry sanity check failed");
> + return -EFSCORRUPTED;
> + }
> + } else {
> + xfs_rtblock_t rexts;
> + uint32_t temp;
> +
> + rexts = div_u64_rem(sbp->sb_rblocks, sbp->sb_rextsize, &temp);
> + if (unlikely(
> + rexts != sbp->sb_rextents ||
> + sbp->sb_rextslog != xfs_highbit32(sbp->sb_rextents) ||
And again.
At least you're consistent, Darrick :)
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
