[03/46] xfs: refactor btree node scrubbing

Message ID	20191107182410.12660-4-hch@lst.de (mailing list archive)
State	Accepted
Headers	show Return-Path: <SRS0=aC/G=Y7=vger.kernel.org=linux-xfs-owner@kernel.org> From: Christoph Hellwig <hch@lst.de> To: linux-xfs@vger.kernel.org Subject: [PATCH 03/46] xfs: refactor btree node scrubbing Date: Thu, 7 Nov 2019 19:23:27 +0100 Message-Id: <20191107182410.12660-4-hch@lst.de> In-Reply-To: <20191107182410.12660-1-hch@lst.de> References: <20191107182410.12660-1-hch@lst.de> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-xfs-owner@vger.kernel.org Precedence: bulk
Series	[01/46] xfs: move incore structures out of xfs_da_format.h \| expand [01/46] xfs: move incore structures out of xfs_da_format.h [02/46] xfs: use unsigned int for all size values in struct xfs_da_geometry [03/46] xfs: refactor btree node scrubbing [04/46] xfs: devirtualize ->node_hdr_from_disk [05/46] xfs: devirtualize ->node_hdr_to_disk [06/46] xfs: add a btree entries pointer to struct xfs_da3_icnode_hdr [07/46] xfs: move the node header size to struct xfs_da_geometry [08/46] xfs: devirtualize ->leaf_hdr_from_disk [09/46] xfs: devirtualize ->leaf_hdr_to_disk [10/46] xfs: add a entries pointer to struct xfs_dir3_icleaf_hdr [11/46] xfs: move the dir2 leaf header size to struct xfs_da_geometry [12/46] xfs: move the max dir2 leaf entries count to struct xfs_da_geometry [13/46] xfs: devirtualize ->free_hdr_from_disk [14/46] xfs: devirtualize ->free_hdr_to_disk [15/46] xfs: make the xfs_dir3_icfree_hdr available to xfs_dir2_node_addname_int [16/46] xfs: add a bests pointer to struct xfs_dir3_icfree_hdr [17/46] xfs: move the dir2 free header size to struct xfs_da_geometry [18/46] xfs: move the max dir2 free bests count to struct xfs_da_geometry [19/46] xfs: devirtualize ->db_to_fdb and ->db_to_fdindex [20/46] xfs: devirtualize ->sf_get_parent_ino and ->sf_put_parent_ino [21/46] xfs: devirtualize ->sf_entsize and ->sf_nextentry [22/46] xfs: devirtualize ->sf_get_ino and ->sf_put_ino [23/46] xfs: devirtualize ->sf_get_ftype and ->sf_put_ftype [24/46] xfs: remove the unused ->data_first_entry_p method [25/46] xfs: remove the data_dot_offset field in struct xfs_dir_ops [26/46] xfs: remove the data_dotdot_offset field in struct xfs_dir_ops [27/46] xfs: remove the ->data_dot_entry_p and ->data_dotdot_entry_p methods [28/46] xfs: remove the ->data_unused_p method [29/46] xfs: cleanup xfs_dir2_block_getdents [30/46] xfs: cleanup xfs_dir2_leaf_getdents [31/46] xfs: cleanup xchk_dir_rec [32/46] xfs: cleanup xchk_directory_data_bestfree [33/46] xfs: cleanup xfs_dir2_block_to_sf [34/46] xfs: cleanup xfs_dir2_data_freescan_int [35/46] xfs: cleanup __xfs_dir3_data_check [36/46] xfs: remove the now unused ->data_entry_p method [37/46] xfs: replace xfs_dir3_data_endp with xfs_dir3_data_end_offset [38/46] xfs: devirtualize ->data_entsize [39/46] xfs: devirtualize ->data_entry_tag_p [40/46] xfs: move the dir2 data block fixed offsets to struct xfs_da_geometry [41/46] xfs: cleanup xfs_dir2_data_entsize [42/46] xfs: devirtualize ->data_bestfree_p [43/46] xfs: devirtualize ->data_get_ftype and ->data_put_ftype [44/46] xfs: remove the now unused dir ops infrastructure [45/46] xfs: merge xfs_dir2_data_freescan and xfs_dir2_data_freescan_int [46/46] xfs: always pass a valid hdr to xfs_dir3_leaf_check_int

Message ID

20191107182410.12660-4-hch@lst.de (mailing list archive)

State

Accepted

Headers

From: Christoph Hellwig <hch@lst.de>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 03/46] xfs: refactor btree node scrubbing
Date: Thu,  7 Nov 2019 19:23:27 +0100
Message-Id: <20191107182410.12660-4-hch@lst.de>
In-Reply-To: <20191107182410.12660-1-hch@lst.de>
References: <20191107182410.12660-1-hch@lst.de>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-xfs-owner@vger.kernel.org
Precedence: bulk

Series

[01/46] xfs: move incore structures out of xfs_da_format.h | expand

Commit Message

Christoph Hellwig Nov. 7, 2019, 6:23 p.m. UTC

Break up xchk_da_btree_entry and handle looking up leaf node entries
in the attr / dir callbacks, so that only the generic node handling
is left in the common core code.  Note that the checks for the crc
enabled blocks are removed, as the scrubbing code already remaps the
magic numbers earlier.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/xfs/scrub/attr.c    | 11 +++++-----
 fs/xfs/scrub/dabtree.c | 48 ++++++++++--------------------------------
 fs/xfs/scrub/dabtree.h |  3 +--
 fs/xfs/scrub/dir.c     | 12 ++++++++---
 4 files changed, 27 insertions(+), 47 deletions(-)

Comments

Darrick J. Wong Nov. 7, 2019, 10:43 p.m. UTC | #1

On Thu, Nov 07, 2019 at 07:23:27PM +0100, Christoph Hellwig wrote:
> Break up xchk_da_btree_entry and handle looking up leaf node entries
> in the attr / dir callbacks, so that only the generic node handling
> is left in the common core code.  Note that the checks for the crc
> enabled blocks are removed, as the scrubbing code already remaps the
> magic numbers earlier.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/scrub/attr.c    | 11 +++++-----
>  fs/xfs/scrub/dabtree.c | 48 ++++++++++--------------------------------
>  fs/xfs/scrub/dabtree.h |  3 +--
>  fs/xfs/scrub/dir.c     | 12 ++++++++---
>  4 files changed, 27 insertions(+), 47 deletions(-)
> 
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index 0edc7f8eb96e..d9f0dd444b80 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -398,15 +398,14 @@ xchk_xattr_block(
>  STATIC int
>  xchk_xattr_rec(
>  	struct xchk_da_btree		*ds,
> -	int				level,
> -	void				*rec)
> +	int				level)
>  {
>  	struct xfs_mount		*mp = ds->state->mp;
> -	struct xfs_attr_leaf_entry	*ent = rec;
> -	struct xfs_da_state_blk		*blk;
> +	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
>  	struct xfs_attr_leaf_name_local	*lentry;
>  	struct xfs_attr_leaf_name_remote	*rentry;
>  	struct xfs_buf			*bp;
> +	struct xfs_attr_leaf_entry	*ent;
>  	xfs_dahash_t			calc_hash;
>  	xfs_dahash_t			hash;
>  	int				nameidx;
> @@ -414,7 +413,9 @@ xchk_xattr_rec(
>  	unsigned int			badflags;
>  	int				error;
>  
> -	blk = &ds->state->path.blk[level];
> +	ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
> +
> +	ent = xfs_attr3_leaf_entryp(blk->bp->b_addr) + blk->index;
>  
>  	/* Check the whole block, if necessary. */
>  	error = xchk_xattr_block(ds, level);
> diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
> index 77ff9f97bcda..d1248c223c7f 100644
> --- a/fs/xfs/scrub/dabtree.c
> +++ b/fs/xfs/scrub/dabtree.c
> @@ -77,40 +77,17 @@ xchk_da_set_corrupt(
>  			__return_address);
>  }
>  
> -/* Find an entry at a certain level in a da btree. */
> -STATIC void *
> -xchk_da_btree_entry(
> -	struct xchk_da_btree	*ds,
> -	int			level,
> -	int			rec)
> +static struct xfs_da_node_entry *
> +xchk_da_btree_node_entry(
> +	struct xchk_da_btree		*ds,
> +	int				level)
>  {
> -	char			*ents;
> -	struct xfs_da_state_blk	*blk;
> -	void			*baddr;
> +	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
>  
> -	/* Dispatch the entry finding function. */
> -	blk = &ds->state->path.blk[level];
> -	baddr = blk->bp->b_addr;
> -	switch (blk->magic) {
> -	case XFS_ATTR_LEAF_MAGIC:
> -	case XFS_ATTR3_LEAF_MAGIC:
> -		ents = (char *)xfs_attr3_leaf_entryp(baddr);
> -		return ents + (rec * sizeof(struct xfs_attr_leaf_entry));
> -	case XFS_DIR2_LEAFN_MAGIC:
> -	case XFS_DIR3_LEAFN_MAGIC:
> -		ents = (char *)ds->dargs.dp->d_ops->leaf_ents_p(baddr);
> -		return ents + (rec * sizeof(struct xfs_dir2_leaf_entry));
> -	case XFS_DIR2_LEAF1_MAGIC:
> -	case XFS_DIR3_LEAF1_MAGIC:
> -		ents = (char *)ds->dargs.dp->d_ops->leaf_ents_p(baddr);
> -		return ents + (rec * sizeof(struct xfs_dir2_leaf_entry));
> -	case XFS_DA_NODE_MAGIC:
> -	case XFS_DA3_NODE_MAGIC:
> -		ents = (char *)ds->dargs.dp->d_ops->node_tree_p(baddr);
> -		return ents + (rec * sizeof(struct xfs_da_node_entry));
> -	}
> +	ASSERT(blk->magic == XFS_DA_NODE_MAGIC);
>  
> -	return NULL;
> +	return (void *)ds->dargs.dp->d_ops->node_tree_p(blk->bp->b_addr) +
> +		(blk->index * sizeof(struct xfs_da_node_entry));
>  }
>  
>  /* Scrub a da btree hash (key). */
> @@ -136,7 +113,7 @@ xchk_da_btree_hash(
>  
>  	/* Is this hash no larger than the parent hash? */
>  	blks = ds->state->path.blk;
> -	entry = xchk_da_btree_entry(ds, level - 1, blks[level - 1].index);

This eliminates the only user of blks, which means the variable can be
removed.  The rest looks fine though.

--D

> +	entry = xchk_da_btree_node_entry(ds, level - 1);
>  	parent_hash = be32_to_cpu(entry->hashval);
>  	if (parent_hash < hash)
>  		xchk_da_set_corrupt(ds, level);
> @@ -479,7 +456,6 @@ xchk_da_btree(
>  	struct xfs_mount		*mp = sc->mp;
>  	struct xfs_da_state_blk		*blks;
>  	struct xfs_da_node_entry	*key;
> -	void				*rec;
>  	xfs_dablk_t			blkno;
>  	int				level;
>  	int				error;
> @@ -538,9 +514,7 @@ xchk_da_btree(
>  			}
>  
>  			/* Dispatch record scrubbing. */
> -			rec = xchk_da_btree_entry(&ds, level,
> -					blks[level].index);
> -			error = scrub_fn(&ds, level, rec);
> +			error = scrub_fn(&ds, level);
>  			if (error)
>  				break;
>  			if (xchk_should_terminate(sc, &error) ||
> @@ -562,7 +536,7 @@ xchk_da_btree(
>  		}
>  
>  		/* Hashes in order for scrub? */
> -		key = xchk_da_btree_entry(&ds, level, blks[level].index);
> +		key = xchk_da_btree_node_entry(&ds, level);
>  		error = xchk_da_btree_hash(&ds, level, &key->hashval);
>  		if (error)
>  			goto out;
> diff --git a/fs/xfs/scrub/dabtree.h b/fs/xfs/scrub/dabtree.h
> index cb3f0003245b..1f3515c6d5a8 100644
> --- a/fs/xfs/scrub/dabtree.h
> +++ b/fs/xfs/scrub/dabtree.h
> @@ -28,8 +28,7 @@ struct xchk_da_btree {
>  	int			tree_level;
>  };
>  
> -typedef int (*xchk_da_btree_rec_fn)(struct xchk_da_btree *ds,
> -		int level, void *rec);
> +typedef int (*xchk_da_btree_rec_fn)(struct xchk_da_btree *ds, int level);
>  
>  /* Check for da btree operation errors. */
>  bool xchk_da_process_error(struct xchk_da_btree *ds, int level, int *error);
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index 1e2e11721eb9..97f274f7cd38 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -179,14 +179,14 @@ xchk_dir_actor(
>  STATIC int
>  xchk_dir_rec(
>  	struct xchk_da_btree		*ds,
> -	int				level,
> -	void				*rec)
> +	int				level)
>  {
> +	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
>  	struct xfs_mount		*mp = ds->state->mp;
> -	struct xfs_dir2_leaf_entry	*ent = rec;
>  	struct xfs_inode		*dp = ds->dargs.dp;
>  	struct xfs_dir2_data_entry	*dent;
>  	struct xfs_buf			*bp;
> +	struct xfs_dir2_leaf_entry	*ent;
>  	char				*p, *endp;
>  	xfs_ino_t			ino;
>  	xfs_dablk_t			rec_bno;
> @@ -198,6 +198,12 @@ xchk_dir_rec(
>  	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) +
> +		(blk->index * sizeof(struct xfs_dir2_leaf_entry));
> +
>  	/* Check the hash of the entry. */
>  	error = xchk_da_btree_hash(ds, level, &ent->hashval);
>  	if (error)
> -- 
> 2.20.1
>

Christoph Hellwig Nov. 8, 2019, 5:28 a.m. UTC | #2

> >  /* Scrub a da btree hash (key). */
> > @@ -136,7 +113,7 @@ xchk_da_btree_hash(
> >  
> >  	/* Is this hash no larger than the parent hash? */
> >  	blks = ds->state->path.blk;
> > -	entry = xchk_da_btree_entry(ds, level - 1, blks[level - 1].index);
> 
> This eliminates the only user of blks, which means the variable can be
> removed.  The rest looks fine though.

Fixed.

diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 0edc7f8eb96e..d9f0dd444b80 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -398,15 +398,14 @@  xchk_xattr_block(
 STATIC int
 xchk_xattr_rec(
 	struct xchk_da_btree		*ds,
-	int				level,
-	void				*rec)
+	int				level)
 {
 	struct xfs_mount		*mp = ds->state->mp;
-	struct xfs_attr_leaf_entry	*ent = rec;
-	struct xfs_da_state_blk		*blk;
+	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
 	struct xfs_attr_leaf_name_local	*lentry;
 	struct xfs_attr_leaf_name_remote	*rentry;
 	struct xfs_buf			*bp;
+	struct xfs_attr_leaf_entry	*ent;
 	xfs_dahash_t			calc_hash;
 	xfs_dahash_t			hash;
 	int				nameidx;
@@ -414,7 +413,9 @@  xchk_xattr_rec(
 	unsigned int			badflags;
 	int				error;
 
-	blk = &ds->state->path.blk[level];
+	ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
+
+	ent = xfs_attr3_leaf_entryp(blk->bp->b_addr) + blk->index;
 
 	/* Check the whole block, if necessary. */
 	error = xchk_xattr_block(ds, level);
diff --git a/fs/xfs/scrub/dabtree.c b/fs/xfs/scrub/dabtree.c
index 77ff9f97bcda..d1248c223c7f 100644
--- a/fs/xfs/scrub/dabtree.c
+++ b/fs/xfs/scrub/dabtree.c
@@ -77,40 +77,17 @@  xchk_da_set_corrupt(
 			__return_address);
 }
 
-/* Find an entry at a certain level in a da btree. */
-STATIC void *
-xchk_da_btree_entry(
-	struct xchk_da_btree	*ds,
-	int			level,
-	int			rec)
+static struct xfs_da_node_entry *
+xchk_da_btree_node_entry(
+	struct xchk_da_btree		*ds,
+	int				level)
 {
-	char			*ents;
-	struct xfs_da_state_blk	*blk;
-	void			*baddr;
+	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
 
-	/* Dispatch the entry finding function. */
-	blk = &ds->state->path.blk[level];
-	baddr = blk->bp->b_addr;
-	switch (blk->magic) {
-	case XFS_ATTR_LEAF_MAGIC:
-	case XFS_ATTR3_LEAF_MAGIC:
-		ents = (char *)xfs_attr3_leaf_entryp(baddr);
-		return ents + (rec * sizeof(struct xfs_attr_leaf_entry));
-	case XFS_DIR2_LEAFN_MAGIC:
-	case XFS_DIR3_LEAFN_MAGIC:
-		ents = (char *)ds->dargs.dp->d_ops->leaf_ents_p(baddr);
-		return ents + (rec * sizeof(struct xfs_dir2_leaf_entry));
-	case XFS_DIR2_LEAF1_MAGIC:
-	case XFS_DIR3_LEAF1_MAGIC:
-		ents = (char *)ds->dargs.dp->d_ops->leaf_ents_p(baddr);
-		return ents + (rec * sizeof(struct xfs_dir2_leaf_entry));
-	case XFS_DA_NODE_MAGIC:
-	case XFS_DA3_NODE_MAGIC:
-		ents = (char *)ds->dargs.dp->d_ops->node_tree_p(baddr);
-		return ents + (rec * sizeof(struct xfs_da_node_entry));
-	}
+	ASSERT(blk->magic == XFS_DA_NODE_MAGIC);
 
-	return NULL;
+	return (void *)ds->dargs.dp->d_ops->node_tree_p(blk->bp->b_addr) +
+		(blk->index * sizeof(struct xfs_da_node_entry));
 }
 
 /* Scrub a da btree hash (key). */
@@ -136,7 +113,7 @@  xchk_da_btree_hash(
 
 	/* Is this hash no larger than the parent hash? */
 	blks = ds->state->path.blk;
-	entry = xchk_da_btree_entry(ds, level - 1, blks[level - 1].index);
+	entry = xchk_da_btree_node_entry(ds, level - 1);
 	parent_hash = be32_to_cpu(entry->hashval);
 	if (parent_hash < hash)
 		xchk_da_set_corrupt(ds, level);
@@ -479,7 +456,6 @@  xchk_da_btree(
 	struct xfs_mount		*mp = sc->mp;
 	struct xfs_da_state_blk		*blks;
 	struct xfs_da_node_entry	*key;
-	void				*rec;
 	xfs_dablk_t			blkno;
 	int				level;
 	int				error;
@@ -538,9 +514,7 @@  xchk_da_btree(
 			}
 
 			/* Dispatch record scrubbing. */
-			rec = xchk_da_btree_entry(&ds, level,
-					blks[level].index);
-			error = scrub_fn(&ds, level, rec);
+			error = scrub_fn(&ds, level);
 			if (error)
 				break;
 			if (xchk_should_terminate(sc, &error) ||
@@ -562,7 +536,7 @@  xchk_da_btree(
 		}
 
 		/* Hashes in order for scrub? */
-		key = xchk_da_btree_entry(&ds, level, blks[level].index);
+		key = xchk_da_btree_node_entry(&ds, level);
 		error = xchk_da_btree_hash(&ds, level, &key->hashval);
 		if (error)
 			goto out;
diff --git a/fs/xfs/scrub/dabtree.h b/fs/xfs/scrub/dabtree.h
index cb3f0003245b..1f3515c6d5a8 100644
--- a/fs/xfs/scrub/dabtree.h
+++ b/fs/xfs/scrub/dabtree.h
@@ -28,8 +28,7 @@  struct xchk_da_btree {
 	int			tree_level;
 };
 
-typedef int (*xchk_da_btree_rec_fn)(struct xchk_da_btree *ds,
-		int level, void *rec);
+typedef int (*xchk_da_btree_rec_fn)(struct xchk_da_btree *ds, int level);
 
 /* Check for da btree operation errors. */
 bool xchk_da_process_error(struct xchk_da_btree *ds, int level, int *error);
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index 1e2e11721eb9..97f274f7cd38 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -179,14 +179,14 @@  xchk_dir_actor(
 STATIC int
 xchk_dir_rec(
 	struct xchk_da_btree		*ds,
-	int				level,
-	void				*rec)
+	int				level)
 {
+	struct xfs_da_state_blk		*blk = &ds->state->path.blk[level];
 	struct xfs_mount		*mp = ds->state->mp;
-	struct xfs_dir2_leaf_entry	*ent = rec;
 	struct xfs_inode		*dp = ds->dargs.dp;
 	struct xfs_dir2_data_entry	*dent;
 	struct xfs_buf			*bp;
+	struct xfs_dir2_leaf_entry	*ent;
 	char				*p, *endp;
 	xfs_ino_t			ino;
 	xfs_dablk_t			rec_bno;
@@ -198,6 +198,12 @@  xchk_dir_rec(
 	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) +
+		(blk->index * sizeof(struct xfs_dir2_leaf_entry));
+
 	/* Check the hash of the entry. */
 	error = xchk_da_btree_hash(ds, level, &ent->hashval);
 	if (error)

[03/46] xfs: refactor btree node scrubbing

Commit Message

Comments

Patch