Re: [PATCH] xfs_repair: Bump link count if longform_dir2_rebuild yields shortform dir

[Bill O'Donnell <bodonnel@redhat.com>]

On Tue, Apr 15, 2025 at 01:44:25PM -0500, Eric Sandeen wrote:
> On 4/15/25 1:23 PM, Bill O'Donnell wrote:
> > On Tue, Apr 15, 2025 at 01:09:23PM -0500, user.mail wrote:
> >> From: Eric Sandeen <sandeen@redhat.com>
> >>
> >> If longform_dir2_rebuild() has so few entries in *hashtab that it results
> >> in a short form directory, bump the link count manually as shortform
> >> directories have no explicit "." entry.
> >>
> >> Without this, repair will end with i.e.:
> >>
> >> resetting inode 131 nlinks from 2 to 1
> >>
> >> in this case, because it thinks this directory inode only has 1 link
> >> discovered, and then a 2nd repair will fix it:
> >>
> >> resetting inode 131 nlinks from 1 to 2
> >>
> >> because shortform_dir2_entry_check() explicitly adds the extra ref when
> >> the (newly-created)shortform directory is checked:
> >>
> >>         /*
> >>          * no '.' entry in shortform dirs, just bump up ref count by 1
> >>          * '..' was already (or will be) accounted for and checked when
> >>          * the directory is reached or will be taken care of when the
> >>          * directory is moved to orphanage.
> >>          */
> >>         add_inode_ref(current_irec, current_ino_offset);
> >>
> >> Avoid this by adding the extra ref if we convert from longform to
> >> shortform.
> >>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> Signed-off-by: user.mail <sandeen@redhat.com>
> >> ---
> > 
> > I was about to send a v3 of my patch to handle this (fix link counts
> > update...) based on djwong's review. This looks cleaner. Thanks!
> 
> This is related to, but independent of, your patch (see my self-reply).
> Please continue to fix your case, so that all entries do not end up in
> lost+found when the header is bad.

Ah, right. Looks good still ;)

Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>

> 
> Thanks,
> -Eric
> 
> > Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>
> > 
> > 
> >>  repair/phase6.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/repair/phase6.c b/repair/phase6.c
> >> index dbc090a5..8804278a 100644
> >> --- a/repair/phase6.c
> >> +++ b/repair/phase6.c
> >> @@ -1392,6 +1392,13 @@ _("name create failed in ino %" PRIu64 " (%d)\n"), ino, error);
> >>  _("name create failed (%d) during rebuild\n"), error);
> >>  	}
> >>  
> >> +	/*
> >> +	 * If we added too few entries to retain longform, add the extra
> >> +	 * ref for . as this is now a shortform directory.
> >> +	 */
> >> +	if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL)
> >> +		add_inode_ref(irec, ino_offset);
> >> +
> >>  	return;
> >>  
> >>  out_bmap_cancel:
> >> -- 
> >> 2.49.0
> >>
> > 
> > 
>