Re: [PATCH 2/2] xfs: refactor xfs_iread_extents to use xfs_btree_visit_blocks

From: Christoph Hellwig <hch@lst.de>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org, bfoster@redhat.com, hch@lst.de
Subject: Re: [PATCH 2/2] xfs: refactor xfs_iread_extents to use xfs_btree_visit_blocks
Date: Tue, 29 Oct 2019 07:28:40 +0100	[thread overview]
Message-ID: <20191029062840.GB17004@lst.de> (raw)
In-Reply-To: <157232186801.594704.5915391485002020723.stgit@magnolia>

On Mon, Oct 28, 2019 at 09:04:28PM -0700, Darrick J. Wong wrote:
> @@ -4313,6 +4314,11 @@ xfs_btree_visit_blocks(
>  			xfs_btree_copy_ptrs(cur, &lptr, ptr, 1);

>  
> +		/* Skip whatever we don't want. */
> +		if ((level == 0 && !(flags & XFS_BTREE_VISIT_RECORDS)) ||
> +		    (level > 0 && !(flags & XFS_BTREE_VISIT_LEAVES)))
> +			continue;

Nipick:  I'd make this two separate if statements as that flows a little
easier.  In fact the closing brace above is the start of a level > 0
check, so the whole thing could become:

		if (level > 0) {
			// existing code, maybe also move the comment above
			// the if here

			if (!(flags & XFS_BTREE_VISIT_RECORDS))
				continue;
		} else {
			if (!(flags & XFS_BTREE_VISIT_LEAVES))
				continue;
		}

which would be even nicer.  Otherwise this patch looks fine to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>