public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <darrick.wong@oracle.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/4] xfs: validate the realtime geometry in xfs_validate_sb_common
Date: Tue, 24 Mar 2020 22:53:28 -0700	[thread overview]
Message-ID: <20200325055328.GU29339@magnolia> (raw)
In-Reply-To: <20200325050028.GG10776@dread.disaster.area>

On Wed, Mar 25, 2020 at 04:00:28PM +1100, Dave Chinner wrote:
> On Tue, Mar 24, 2020 at 08:24:43PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > 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@oracle.com>
> > ---
> >  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 :)

Copy-pastaing the weird style of the rest of that function. :)

I'll fix it & resend.

--D

> -Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

  reply	other threads:[~2020-03-25  5:53 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25  3:24 [PATCH 0/4] xfs: random fixes Darrick J. Wong
2020-03-25  3:24 ` [PATCH 1/4] xfs: prohibit fs freezing when using empty transactions Darrick J. Wong
2020-03-25  4:53   ` Dave Chinner
2020-03-25  5:56     ` Darrick J. Wong
2020-03-25  6:06   ` [PATCH v2 " Darrick J. Wong
2020-03-25 23:26     ` Dave Chinner
2020-04-01 11:22   ` [PATCH " Chandan Rajendra
2020-03-25  3:24 ` [PATCH 2/4] xfs: validate the realtime geometry in xfs_validate_sb_common Darrick J. Wong
2020-03-25  5:00   ` Dave Chinner
2020-03-25  5:53     ` Darrick J. Wong [this message]
2020-03-25  6:07   ` [PATCH v2 " Darrick J. Wong
2020-03-25 23:27     ` Dave Chinner
2020-04-01 13:49   ` [PATCH " Chandan Rajendra
2020-03-25  3:24 ` [PATCH 3/4] xfs: drop all altpath buffers at the end of the sibling check Darrick J. Wong
2020-03-25  5:04   ` Dave Chinner
2020-04-02  3:05   ` Chandan Rajendra
2020-03-25  3:24 ` [PATCH 4/4] xfs: directory bestfree check should release buffers Darrick J. Wong
2020-03-25  5:05   ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200325055328.GU29339@magnolia \
    --to=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox