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@xxxxxxxxxx> > --- > 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 >
