Re: [PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/22/18 12:13 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> 
> In process_node_dir2, we need to distinguish between a directory with a
> single leafn block (yes, they exist) having no interior da nodes, and a
> directory with a da tree that incorrectly points to dablk 0.  If we
> happened to fill out any part of the da cursor then we have a da btree
> with garbage in it; otherwise, we have a single leafn block.
> 
> This was found by repair repeatedly rebuilding a directory containing a
> single leafn block (xfs/495).
> 
> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> Reported-by: Dave Chinner <david@xxxxxxxxxxxxx>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

I'm finding the commit log hard to parse/understand.

Let's reference

71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"

for starters, but can we do something like this....?

===

As explained in

71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"

a single root LEAFN block can exist in a directory until it grows further.

This is why, normally, we skip directories with a root marked
XFS_DIR2_LEAFN_MAGIC, as detected by the left-most leaf block being 
found at file block 0.

However, if we traversed any level of a btree to get here (as
indicated by da_cursor.active > 0), then a leaf block claiming block
0 indicates corruption, and we should handle it as such, and rebuild
the directory.

This was found by repair repeatedly rebuilding a directory containing a
single leafn block (xfs/495).

Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
Reported-by: Dave Chinner <david@xxxxxxxxxxxxx>
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>

===

I'm not hung up on rewriting the commit log, but if you like it give me
thumbs up.  I needed to think it through that way to grok the change.
Anyway, for the change now that I do grok it,

Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>

> ---
> v2: now that the regression test has landed, mention that in the
> changelog
> ---
>  repair/dir2.c |   15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index ba5763ed..e67ec590 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -1243,10 +1243,21 @@ process_node_dir2(
>  
>  	/*
>  	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC
> +	 *
> +	 * Be careful here: If any level of the da cursor was filled out then
> +	 * the directory has a da btree containing an invalid before pointer to
> +	 * dblock 0, and we should move on to rebuilding the directory.  If no
> +	 * levels in the da cursor got filled out, then we just have a single
> +	 * leafn block and we're done.
>  	 */
>  	if (bno == 0) {
> -		release_da_cursor(mp, &da_cursor, 0);
> -		return 0;
> +		if (da_cursor.active > 0) {
> +			err_release_da_cursor(mp, &da_cursor, 0);
> +			return 1;
> +		} else {
> +			release_da_cursor(mp, &da_cursor, 0);
> +			return 0;
> +		}
>  	} else {
>  		/*
>  		 * Now pass cursor and bno into leaf-block processing routine.
> 



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux