Re: [PATCH 34/46] xfs: cleanup xfs_dir2_data_freescan_int

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Nov 07, 2019 at 07:23:58PM +0100, Christoph Hellwig wrote:
> Use an offset as the main means for iteration, and only do pointer
> arithmetics to find the data/unused entries.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
>  fs/xfs/libxfs/xfs_dir2_data.c | 48 +++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 3ecec8e1c5f6..50e3fa092ff9 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -562,16 +562,15 @@ xfs_dir2_data_freeremove(
>   */
>  void
>  xfs_dir2_data_freescan_int(
> -	struct xfs_da_geometry	*geo,
> -	const struct xfs_dir_ops *ops,
> -	struct xfs_dir2_data_hdr *hdr,
> -	int			*loghead)
> +	struct xfs_da_geometry		*geo,
> +	const struct xfs_dir_ops	*ops,
> +	struct xfs_dir2_data_hdr	*hdr,
> +	int				*loghead)
>  {
> -	xfs_dir2_data_entry_t	*dep;		/* active data entry */
> -	xfs_dir2_data_unused_t	*dup;		/* unused data entry */
> -	struct xfs_dir2_data_free *bf;
> -	char			*endp;		/* end of block's data */
> -	char			*p;		/* current entry pointer */
> +	struct xfs_dir2_data_free	*bf = ops->data_bestfree_p(hdr);
> +	void				*addr = hdr;
> +	unsigned int			offset = ops->data_entry_offset;
> +	unsigned int			end;
>  
>  	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
>  	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
> @@ -581,37 +580,30 @@ xfs_dir2_data_freescan_int(
>  	/*
>  	 * Start by clearing the table.
>  	 */
> -	bf = ops->data_bestfree_p(hdr);
>  	memset(bf, 0, sizeof(*bf) * XFS_DIR2_DATA_FD_COUNT);
>  	*loghead = 1;
> -	/*
> -	 * Set up pointers.
> -	 */
> -	p = (char *)ops->data_entry_p(hdr);
> -	endp = xfs_dir3_data_endp(geo, hdr);
> -	/*
> -	 * Loop over the block's entries.
> -	 */
> -	while (p < endp) {
> -		dup = (xfs_dir2_data_unused_t *)p;
> +
> +	end = xfs_dir3_data_endp(geo, addr) - addr;
> +	while (offset < end) {
> +		struct xfs_dir2_data_unused	*dup = addr + offset;
> +		struct xfs_dir2_data_entry	*dep = addr + offset;
> +
>  		/*
>  		 * If it's a free entry, insert it.
>  		 */
>  		if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
> -			ASSERT((char *)dup - (char *)hdr ==
> +			ASSERT(offset ==
>  			       be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)));
>  			xfs_dir2_data_freeinsert(hdr, bf, dup, loghead);
> -			p += be16_to_cpu(dup->length);
> +			offset += be16_to_cpu(dup->length);
> +			continue;
>  		}
> +
>  		/*
>  		 * For active entries, check their tags and skip them.
>  		 */
> -		else {
> -			dep = (xfs_dir2_data_entry_t *)p;
> -			ASSERT((char *)dep - (char *)hdr ==
> -			       be16_to_cpu(*ops->data_entry_tag_p(dep)));
> -			p += ops->data_entsize(dep->namelen);
> -		}
> +		ASSERT(offset == be16_to_cpu(*ops->data_entry_tag_p(dep)));
> +		offset += ops->data_entsize(dep->namelen);
>  	}
>  }
>  
> -- 
> 2.20.1
>