Re: [PATCH V1.1] xfs_repair: Search for conflicts in inode_tree_ptrs[] when processing uncertain inodes

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, May 23, 2022 at 02:04:10PM +0530, Chandan Babu R wrote:
> When processing an uncertain inode chunk record, if we lose 2 blocks worth of
> inodes or 25% of the chunk, xfs_repair decides to ignore the chunk. Otherwise,
> xfs_repair adds a new chunk record to inode_tree_ptrs[agno], marking each
> inode as either free or used. However, before adding the new chunk record,
> xfs_repair has to check for the existance of a conflicting record.
> 
> The existing code incorrectly checks for the conflicting record in
> inode_uncertain_tree_ptrs[agno]. This check will succeed since the inode chunk
> record being processed was originally obtained from
> inode_uncertain_tree_ptrs[agno].
> 
> This commit fixes the bug by changing xfs_repair to search
> inode_tree_ptrs[agno] for conflicts.

Just out of curiosity -- how did you come across this bug?  I /think/ it
looks reasonable, but want to know more context...

> Signed-off-by: Chandan Babu R <chandan.babu@oracle.com>
> ---
> Changelog:
> V1 -> V1.1:
>    1. Fix commit message.
>    
>  repair/dino_chunks.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
> index 11b0eb5f..80c52a43 100644
> --- a/repair/dino_chunks.c
> +++ b/repair/dino_chunks.c
> @@ -229,8 +229,7 @@ verify_inode_chunk(xfs_mount_t		*mp,
>  		/*
>  		 * ok, put the record into the tree, if no conflict.
>  		 */
> -		if (find_uncertain_inode_rec(agno,
> -				XFS_AGB_TO_AGINO(mp, start_agbno)))
> +		if (find_inode_rec(mp, agno, XFS_AGB_TO_AGINO(mp, start_agbno)))

...because the big question I have is: why not check both the certain
and the uncertain records for confliects?

--D

>  			return(0);
>  
>  		start_agino = XFS_AGB_TO_AGINO(mp, start_agbno);
> -- 
> 2.35.1
>