diff mbox series

[CANDIDATE,v6.1,1/5] xfs: hoist refcount record merge predicates

Message ID 20230804171223.1393045-1-tytso@mit.edu (mailing list archive)
State Superseded, archived
Headers show
Series [CANDIDATE,v6.1,1/5] xfs: hoist refcount record merge predicates | expand

Commit Message

Theodore Ts'o Aug. 4, 2023, 5:12 p.m. UTC 
From: "Darrick J. Wong" <djwong@kernel.org>

commit 9d720a5a658f5135861773f26e927449bef93d61 upstream.

Hoist these multiline conditionals into separate static inline helpers
to improve readability and set the stage for corruption fixes that will
be introduced in the next patch.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com>
---
 fs/xfs/libxfs/xfs_refcount.c | 129 ++++++++++++++++++++++++++++++-----
 1 file changed, 113 insertions(+), 16 deletions(-)

Comments

Darrick J. Wong Aug. 4, 2023, 6:05 p.m. UTC | #1 
On Fri, Aug 04, 2023 at 01:12:19PM -0400, Theodore Ts'o wrote:
> From: "Darrick J. Wong" <djwong@kernel.org>
> 
> commit 9d720a5a658f5135861773f26e927449bef93d61 upstream.
> 
> Hoist these multiline conditionals into separate static inline helpers
> to improve readability and set the stage for corruption fixes that will
> be introduced in the next patch.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com>

For the whole series,
Acked-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_refcount.c | 129 ++++++++++++++++++++++++++++++-----
>  1 file changed, 113 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
> index 3f34bafe18dd..4408893333a6 100644
> --- a/fs/xfs/libxfs/xfs_refcount.c
> +++ b/fs/xfs/libxfs/xfs_refcount.c
> @@ -815,11 +815,119 @@ xfs_refcount_find_right_extents(
>  /* Is this extent valid? */
>  static inline bool
>  xfs_refc_valid(
> -	struct xfs_refcount_irec	*rc)
> +	const struct xfs_refcount_irec	*rc)
>  {
>  	return rc->rc_startblock != NULLAGBLOCK;
>  }
>  
> +static inline bool
> +xfs_refc_want_merge_center(
> +	const struct xfs_refcount_irec	*left,
> +	const struct xfs_refcount_irec	*cleft,
> +	const struct xfs_refcount_irec	*cright,
> +	const struct xfs_refcount_irec	*right,
> +	bool				cleft_is_cright,
> +	enum xfs_refc_adjust_op		adjust,
> +	unsigned long long		*ulenp)
> +{
> +	unsigned long long		ulen = left->rc_blockcount;
> +
> +	/*
> +	 * To merge with a center record, both shoulder records must be
> +	 * adjacent to the record we want to adjust.  This is only true if
> +	 * find_left and find_right made all four records valid.
> +	 */
> +	if (!xfs_refc_valid(left)  || !xfs_refc_valid(right) ||
> +	    !xfs_refc_valid(cleft) || !xfs_refc_valid(cright))
> +		return false;
> +
> +	/* There must only be one record for the entire range. */
> +	if (!cleft_is_cright)
> +		return false;
> +
> +	/* The shoulder record refcounts must match the new refcount. */
> +	if (left->rc_refcount != cleft->rc_refcount + adjust)
> +		return false;
> +	if (right->rc_refcount != cleft->rc_refcount + adjust)
> +		return false;
> +
> +	/*
> +	 * The new record cannot exceed the max length.  ulen is a ULL as the
> +	 * individual record block counts can be up to (u32 - 1) in length
> +	 * hence we need to catch u32 addition overflows here.
> +	 */
> +	ulen += cleft->rc_blockcount + right->rc_blockcount;
> +	if (ulen >= MAXREFCEXTLEN)
> +		return false;
> +
> +	*ulenp = ulen;
> +	return true;
> +}
> +
> +static inline bool
> +xfs_refc_want_merge_left(
> +	const struct xfs_refcount_irec	*left,
> +	const struct xfs_refcount_irec	*cleft,
> +	enum xfs_refc_adjust_op		adjust)
> +{
> +	unsigned long long		ulen = left->rc_blockcount;
> +
> +	/*
> +	 * For a left merge, the left shoulder record must be adjacent to the
> +	 * start of the range.  If this is true, find_left made left and cleft
> +	 * contain valid contents.
> +	 */
> +	if (!xfs_refc_valid(left) || !xfs_refc_valid(cleft))
> +		return false;
> +
> +	/* Left shoulder record refcount must match the new refcount. */
> +	if (left->rc_refcount != cleft->rc_refcount + adjust)
> +		return false;
> +
> +	/*
> +	 * The new record cannot exceed the max length.  ulen is a ULL as the
> +	 * individual record block counts can be up to (u32 - 1) in length
> +	 * hence we need to catch u32 addition overflows here.
> +	 */
> +	ulen += cleft->rc_blockcount;
> +	if (ulen >= MAXREFCEXTLEN)
> +		return false;
> +
> +	return true;
> +}
> +
> +static inline bool
> +xfs_refc_want_merge_right(
> +	const struct xfs_refcount_irec	*cright,
> +	const struct xfs_refcount_irec	*right,
> +	enum xfs_refc_adjust_op		adjust)
> +{
> +	unsigned long long		ulen = right->rc_blockcount;
> +
> +	/*
> +	 * For a right merge, the right shoulder record must be adjacent to the
> +	 * end of the range.  If this is true, find_right made cright and right
> +	 * contain valid contents.
> +	 */
> +	if (!xfs_refc_valid(right) || !xfs_refc_valid(cright))
> +		return false;
> +
> +	/* Right shoulder record refcount must match the new refcount. */
> +	if (right->rc_refcount != cright->rc_refcount + adjust)
> +		return false;
> +
> +	/*
> +	 * The new record cannot exceed the max length.  ulen is a ULL as the
> +	 * individual record block counts can be up to (u32 - 1) in length
> +	 * hence we need to catch u32 addition overflows here.
> +	 */
> +	ulen += cright->rc_blockcount;
> +	if (ulen >= MAXREFCEXTLEN)
> +		return false;
> +
> +	return true;
> +}
> +
>  /*
>   * Try to merge with any extents on the boundaries of the adjustment range.
>   */
> @@ -861,23 +969,15 @@ xfs_refcount_merge_extents(
>  		 (cleft.rc_blockcount == cright.rc_blockcount);
>  
>  	/* Try to merge left, cleft, and right.  cleft must == cright. */
> -	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount +
> -			right.rc_blockcount;
> -	if (xfs_refc_valid(&left) && xfs_refc_valid(&right) &&
> -	    xfs_refc_valid(&cleft) && xfs_refc_valid(&cright) && cequal &&
> -	    left.rc_refcount == cleft.rc_refcount + adjust &&
> -	    right.rc_refcount == cleft.rc_refcount + adjust &&
> -	    ulen < MAXREFCEXTLEN) {
> +	if (xfs_refc_want_merge_center(&left, &cleft, &cright, &right, cequal,
> +				adjust, &ulen)) {
>  		*shape_changed = true;
>  		return xfs_refcount_merge_center_extents(cur, &left, &cleft,
>  				&right, ulen, aglen);
>  	}
>  
>  	/* Try to merge left and cleft. */
> -	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount;
> -	if (xfs_refc_valid(&left) && xfs_refc_valid(&cleft) &&
> -	    left.rc_refcount == cleft.rc_refcount + adjust &&
> -	    ulen < MAXREFCEXTLEN) {
> +	if (xfs_refc_want_merge_left(&left, &cleft, adjust)) {
>  		*shape_changed = true;
>  		error = xfs_refcount_merge_left_extent(cur, &left, &cleft,
>  				agbno, aglen);
> @@ -893,10 +993,7 @@ xfs_refcount_merge_extents(
>  	}
>  
>  	/* Try to merge cright and right. */
> -	ulen = (unsigned long long)right.rc_blockcount + cright.rc_blockcount;
> -	if (xfs_refc_valid(&right) && xfs_refc_valid(&cright) &&
> -	    right.rc_refcount == cright.rc_refcount + adjust &&
> -	    ulen < MAXREFCEXTLEN) {
> +	if (xfs_refc_want_merge_right(&cright, &right, adjust)) {
>  		*shape_changed = true;
>  		return xfs_refcount_merge_right_extent(cur, &right, &cright,
>  				aglen);
> -- 
> 2.31.0
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 3f34bafe18dd..4408893333a6 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -815,11 +815,119 @@  xfs_refcount_find_right_extents(
 /* Is this extent valid? */
 static inline bool
 xfs_refc_valid(
-	struct xfs_refcount_irec	*rc)
+	const struct xfs_refcount_irec	*rc)
 {
 	return rc->rc_startblock != NULLAGBLOCK;
 }
 
+static inline bool
+xfs_refc_want_merge_center(
+	const struct xfs_refcount_irec	*left,
+	const struct xfs_refcount_irec	*cleft,
+	const struct xfs_refcount_irec	*cright,
+	const struct xfs_refcount_irec	*right,
+	bool				cleft_is_cright,
+	enum xfs_refc_adjust_op		adjust,
+	unsigned long long		*ulenp)
+{
+	unsigned long long		ulen = left->rc_blockcount;
+
+	/*
+	 * To merge with a center record, both shoulder records must be
+	 * adjacent to the record we want to adjust.  This is only true if
+	 * find_left and find_right made all four records valid.
+	 */
+	if (!xfs_refc_valid(left)  || !xfs_refc_valid(right) ||
+	    !xfs_refc_valid(cleft) || !xfs_refc_valid(cright))
+		return false;
+
+	/* There must only be one record for the entire range. */
+	if (!cleft_is_cright)
+		return false;
+
+	/* The shoulder record refcounts must match the new refcount. */
+	if (left->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+	if (right->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cleft->rc_blockcount + right->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	*ulenp = ulen;
+	return true;
+}
+
+static inline bool
+xfs_refc_want_merge_left(
+	const struct xfs_refcount_irec	*left,
+	const struct xfs_refcount_irec	*cleft,
+	enum xfs_refc_adjust_op		adjust)
+{
+	unsigned long long		ulen = left->rc_blockcount;
+
+	/*
+	 * For a left merge, the left shoulder record must be adjacent to the
+	 * start of the range.  If this is true, find_left made left and cleft
+	 * contain valid contents.
+	 */
+	if (!xfs_refc_valid(left) || !xfs_refc_valid(cleft))
+		return false;
+
+	/* Left shoulder record refcount must match the new refcount. */
+	if (left->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cleft->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	return true;
+}
+
+static inline bool
+xfs_refc_want_merge_right(
+	const struct xfs_refcount_irec	*cright,
+	const struct xfs_refcount_irec	*right,
+	enum xfs_refc_adjust_op		adjust)
+{
+	unsigned long long		ulen = right->rc_blockcount;
+
+	/*
+	 * For a right merge, the right shoulder record must be adjacent to the
+	 * end of the range.  If this is true, find_right made cright and right
+	 * contain valid contents.
+	 */
+	if (!xfs_refc_valid(right) || !xfs_refc_valid(cright))
+		return false;
+
+	/* Right shoulder record refcount must match the new refcount. */
+	if (right->rc_refcount != cright->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cright->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	return true;
+}
+
 /*
  * Try to merge with any extents on the boundaries of the adjustment range.
  */
@@ -861,23 +969,15 @@  xfs_refcount_merge_extents(
 		 (cleft.rc_blockcount == cright.rc_blockcount);
 
 	/* Try to merge left, cleft, and right.  cleft must == cright. */
-	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount +
-			right.rc_blockcount;
-	if (xfs_refc_valid(&left) && xfs_refc_valid(&right) &&
-	    xfs_refc_valid(&cleft) && xfs_refc_valid(&cright) && cequal &&
-	    left.rc_refcount == cleft.rc_refcount + adjust &&
-	    right.rc_refcount == cleft.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_center(&left, &cleft, &cright, &right, cequal,
+				adjust, &ulen)) {
 		*shape_changed = true;
 		return xfs_refcount_merge_center_extents(cur, &left, &cleft,
 				&right, ulen, aglen);
 	}
 
 	/* Try to merge left and cleft. */
-	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount;
-	if (xfs_refc_valid(&left) && xfs_refc_valid(&cleft) &&
-	    left.rc_refcount == cleft.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_left(&left, &cleft, adjust)) {
 		*shape_changed = true;
 		error = xfs_refcount_merge_left_extent(cur, &left, &cleft,
 				agbno, aglen);
@@ -893,10 +993,7 @@  xfs_refcount_merge_extents(
 	}
 
 	/* Try to merge cright and right. */
-	ulen = (unsigned long long)right.rc_blockcount + cright.rc_blockcount;
-	if (xfs_refc_valid(&right) && xfs_refc_valid(&cright) &&
-	    right.rc_refcount == cright.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_right(&cright, &right, adjust)) {
 		*shape_changed = true;
 		return xfs_refcount_merge_right_extent(cur, &right, &cright,
 				aglen);