Re: [PATCH 09/34] xfs: add a entries pointer to struct xfs_dir3_icleaf_hdr

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

On Fri, Nov 01, 2019 at 03:06:54PM -0700, Christoph Hellwig wrote:
> All callers of the ->node_tree_p dir operation already have a struct
> xfs_dir3_icleaf_hdr from a previous call to xfs_da_leaf_hdr_from_disk at
> hand, or just need slight changes to the calling conventions to do so.
> Add a pointer to the entries to struct xfs_dir3_icleaf_hdr to clean up
> this pattern.  To make this possible the xfs_dir3_leaf_log_ents function
> grow a new argument to pass the xfs_dir3_icleaf_hdr that call callers
> already have, and xfs_dir2_leaf_lookup_int returns the
> xfs_dir3_icleaf_hdr to the callers so that they can later use it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Mostly looks ok...

<snip>

> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index b402a2391f49..07cea5751783 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -18,6 +18,7 @@ struct xfs_dir3_icleaf_hdr {
>  	uint16_t		magic;
>  	uint16_t		count;
>  	uint16_t		stale;
> +	struct xfs_dir2_leaf_entry *ents;

...except for the same suggestion I had a few patches ago about being
more explicit that this is a pointer to an array of raw on-disk
structures which haven't been converted to cpu endianness.

--D

>  };
>  
>  struct xfs_dir3_icfree_hdr {
> @@ -25,7 +26,6 @@ struct xfs_dir3_icfree_hdr {
>  	uint32_t		firstdb;
>  	uint32_t		nvalid;
>  	uint32_t		nused;
> -
>  };
>  
>  /* xfs_dir2.c */
> @@ -86,7 +86,8 @@ extern void xfs_dir3_leaf_compact_x1(struct xfs_dir3_icleaf_hdr *leafhdr,
>  extern int xfs_dir3_leaf_get_buf(struct xfs_da_args *args, xfs_dir2_db_t bno,
>  		struct xfs_buf **bpp, uint16_t magic);
>  extern void xfs_dir3_leaf_log_ents(struct xfs_da_args *args,
> -		struct xfs_buf *bp, int first, int last);
> +		struct xfs_dir3_icleaf_hdr *hdr, struct xfs_buf *bp, int first,
> +		int last);
>  extern void xfs_dir3_leaf_log_header(struct xfs_da_args *args,
>  		struct xfs_buf *bp);
>  extern int xfs_dir2_leaf_lookup(struct xfs_da_args *args);
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index 5b004d1f6bef..27fdf8978467 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -195,13 +195,15 @@ xchk_dir_rec(
>  	xfs_dir2_dataptr_t		ptr;
>  	xfs_dahash_t			calc_hash;
>  	xfs_dahash_t			hash;
> +	struct xfs_dir3_icleaf_hdr	hdr;
>  	unsigned int			tag;
>  	int				error;
>  
>  	ASSERT(blk->magic == XFS_DIR2_LEAF1_MAGIC ||
>  	       blk->magic == XFS_DIR2_LEAFN_MAGIC);
>  
> -	ent = (void *)ds->dargs.dp->d_ops->leaf_ents_p(blk->bp->b_addr) +
> +	xfs_dir2_leaf_hdr_from_disk(mp, &hdr, blk->bp->b_addr);
> +	ent = (void *)hdr.ents +
>  		(blk->index * sizeof(struct xfs_dir2_leaf_entry));
>  
>  	/* Check the hash of the entry. */
> @@ -481,7 +483,6 @@ xchk_directory_leaf1_bestfree(
>  	xfs_dablk_t			lblk)
>  {
>  	struct xfs_dir3_icleaf_hdr	leafhdr;
> -	struct xfs_dir2_leaf_entry	*ents;
>  	struct xfs_dir2_leaf_tail	*ltp;
>  	struct xfs_dir2_leaf		*leaf;
>  	struct xfs_buf			*dbp;
> @@ -505,7 +506,6 @@ xchk_directory_leaf1_bestfree(
>  
>  	leaf = bp->b_addr;
>  	xfs_dir2_leaf_hdr_from_disk(sc->ip->i_mount, &leafhdr, leaf);
> -	ents = d_ops->leaf_ents_p(leaf);
>  	ltp = xfs_dir2_leaf_tail_p(geo, leaf);
>  	bestcount = be32_to_cpu(ltp->bestcount);
>  	bestp = xfs_dir2_leaf_bests_p(ltp);
> @@ -533,18 +533,19 @@ xchk_directory_leaf1_bestfree(
>  	}
>  
>  	/* Leaves and bests don't overlap in leaf format. */
> -	if ((char *)&ents[leafhdr.count] > (char *)bestp) {
> +	if ((char *)&leafhdr.ents[leafhdr.count] > (char *)bestp) {
>  		xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
>  		goto out;
>  	}
>  
>  	/* Check hash value order, count stale entries.  */
>  	for (i = 0; i < leafhdr.count; i++) {
> -		hash = be32_to_cpu(ents[i].hashval);
> +		hash = be32_to_cpu(leafhdr.ents[i].hashval);
>  		if (i > 0 && lasthash > hash)
>  			xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, lblk);
>  		lasthash = hash;
> -		if (ents[i].address == cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
> +		if (leafhdr.ents[i].address ==
> +		    cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
>  			stale++;
>  	}
>  	if (leafhdr.stale != stale)
> -- 
> 2.20.1
>