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

Bill O'Donnell
2025-04-15T18:56:34Z

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