On Fri, Aug 04, 2023 at 01:12:19PM -0400, Theodore Ts'o wrote:
> From: "Darrick J. Wong" <djwong@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
> Reviewed-by: Xiao Yang <yangx.jy@xxxxxxxxxxx>
For the whole series,
Acked-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--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
>
