* [PATCH v2] xfs_repair: fix link counts update following repair of a bad block
@ 2025-04-15 15:01 bodonnel
2025-04-15 17:07 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: bodonnel @ 2025-04-15 15:01 UTC (permalink / raw)
To: linux-xfs; +Cc: sandeen, djwong, Bill O'Donnell
From: Bill O'Donnell <bodonnel@redhat.com>
Updating nlinks, following repair of a bad block needs a bit of work.
In unique cases, 2 runs of xfs_repair is needed to adjust the count to
the proper value. This patch modifies location of longform_dir2_entry_check,
moving longform_dir2_entry_check_data to run after the check_dir3_header
error check. This results in the hashtab to be correctly filled and those
entries don't end up in lost+found, and nlinks is properly adjusted on the
first xfs_repair pass.
Suggested-by: Eric Sandeen <sandeen@sandeen.net>
Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
---
v2: add logic to cover shortform directory.
repair/phase6.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/repair/phase6.c b/repair/phase6.c
index dbc090a54139..8fc1c3896d2b 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -2426,6 +2426,23 @@ longform_dir2_entry_check(
/* check v5 metadata */
if (xfs_has_crc(mp)) {
+ longform_dir2_entry_check_data(mp, ip, num_illegal,
+ need_dot,
+ irec, ino_offset, bp, hashtab,
+ &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
+ error = check_dir3_header(mp, bp, ino);
+ if (error) {
+ fixit++;
+ if (fmt == XFS_DIR2_FMT_BLOCK)
+ goto out_fix;
+
+ libxfs_buf_relse(bp);
+ bp = NULL;
+ continue;
+ }
+ }
+ else {
+ /* No crc. Directory appears to be shortform. */
error = check_dir3_header(mp, bp, ino);
if (error) {
fixit++;
@@ -2438,9 +2455,6 @@ longform_dir2_entry_check(
}
}
- longform_dir2_entry_check_data(mp, ip, num_illegal, need_dot,
- irec, ino_offset, bp, hashtab,
- &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
if (fmt == XFS_DIR2_FMT_BLOCK)
break;
--
2.49.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xfs_repair: fix link counts update following repair of a bad block
2025-04-15 15:01 [PATCH v2] xfs_repair: fix link counts update following repair of a bad block bodonnel
@ 2025-04-15 17:07 ` Darrick J. Wong
2025-04-15 17:17 ` Bill O'Donnell
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2025-04-15 17:07 UTC (permalink / raw)
To: bodonnel; +Cc: linux-xfs, sandeen
On Tue, Apr 15, 2025 at 10:01:04AM -0500, bodonnel@redhat.com wrote:
> From: Bill O'Donnell <bodonnel@redhat.com>
>
> Updating nlinks, following repair of a bad block needs a bit of work.
> In unique cases, 2 runs of xfs_repair is needed to adjust the count to
> the proper value. This patch modifies location of longform_dir2_entry_check,
> moving longform_dir2_entry_check_data to run after the check_dir3_header
> error check. This results in the hashtab to be correctly filled and those
> entries don't end up in lost+found, and nlinks is properly adjusted on the
> first xfs_repair pass.
>
> Suggested-by: Eric Sandeen <sandeen@sandeen.net>
>
> Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> ---
> v2: add logic to cover shortform directory.
>
>
> repair/phase6.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index dbc090a54139..8fc1c3896d2b 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -2426,6 +2426,23 @@ longform_dir2_entry_check(
>
> /* check v5 metadata */
> if (xfs_has_crc(mp)) {
> + longform_dir2_entry_check_data(mp, ip, num_illegal,
> + need_dot,
> + irec, ino_offset, bp, hashtab,
> + &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
> + error = check_dir3_header(mp, bp, ino);
> + if (error) {
> + fixit++;
I think what you're trying to do here is to get
longform_dir2_entry_check_data to try to find directory entries in the
directory block (no matter how damaged it is). Then if the dir3 header
fields are wrong, we bump fixit so that the directory gets rebuilt from
the salvaged directory entries. Right?
So I think you could structure this more like:
/* salvage any dirents that look ok */
longform_dir2_entry_check_data(...);
/* check v5 metadata */
if (xfs_has_crc(mp)) {
error = check_dir3_header(mp, bp, ino);
if (error) {
fixit++;
if (fmt == XFS_DIR2_FMT_BLOCK)
goto out_fix;
libxfs_buf_relse(bp);
bp = NULL;
continue;
}
}
if (fmt == XFS_DIR2_FMT_BLOCK)
break;
libxfs_buf_relse(bp);
bp = NULL;
}
> + if (fmt == XFS_DIR2_FMT_BLOCK)
> + goto out_fix;
> +
> + libxfs_buf_relse(bp);
> + bp = NULL;
> + continue;
> + }
> + }
> + else {
> + /* No crc. Directory appears to be shortform. */
> error = check_dir3_header(mp, bp, ino);
dir3 headers (as opposed to dir2 headers) are a crc-only feature, so
this isn't correct either.
> if (error) {
> fixit++;
> @@ -2438,9 +2455,6 @@ longform_dir2_entry_check(
> }
> }
>
> - longform_dir2_entry_check_data(mp, ip, num_illegal, need_dot,
> - irec, ino_offset, bp, hashtab,
> - &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
and removing this call means that we never scan a V4 directory at all.
--D
> if (fmt == XFS_DIR2_FMT_BLOCK)
> break;
>
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] xfs_repair: fix link counts update following repair of a bad block
2025-04-15 17:07 ` Darrick J. Wong
@ 2025-04-15 17:17 ` Bill O'Donnell
0 siblings, 0 replies; 3+ messages in thread
From: Bill O'Donnell @ 2025-04-15 17:17 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs, sandeen
On Tue, Apr 15, 2025 at 10:07:57AM -0700, Darrick J. Wong wrote:
> On Tue, Apr 15, 2025 at 10:01:04AM -0500, bodonnel@redhat.com wrote:
> > From: Bill O'Donnell <bodonnel@redhat.com>
> >
> > Updating nlinks, following repair of a bad block needs a bit of work.
> > In unique cases, 2 runs of xfs_repair is needed to adjust the count to
> > the proper value. This patch modifies location of longform_dir2_entry_check,
> > moving longform_dir2_entry_check_data to run after the check_dir3_header
> > error check. This results in the hashtab to be correctly filled and those
> > entries don't end up in lost+found, and nlinks is properly adjusted on the
> > first xfs_repair pass.
> >
> > Suggested-by: Eric Sandeen <sandeen@sandeen.net>
> >
> > Signed-off-by: Bill O'Donnell <bodonnel@redhat.com>
> > ---
> > v2: add logic to cover shortform directory.
> >
> >
> > repair/phase6.c | 20 +++++++++++++++++---
> > 1 file changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/repair/phase6.c b/repair/phase6.c
> > index dbc090a54139..8fc1c3896d2b 100644
> > --- a/repair/phase6.c
> > +++ b/repair/phase6.c
> > @@ -2426,6 +2426,23 @@ longform_dir2_entry_check(
> >
> > /* check v5 metadata */
> > if (xfs_has_crc(mp)) {
> > + longform_dir2_entry_check_data(mp, ip, num_illegal,
> > + need_dot,
> > + irec, ino_offset, bp, hashtab,
> > + &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
> > + error = check_dir3_header(mp, bp, ino);
> > + if (error) {
> > + fixit++;
>
> I think what you're trying to do here is to get
> longform_dir2_entry_check_data to try to find directory entries in the
> directory block (no matter how damaged it is). Then if the dir3 header
> fields are wrong, we bump fixit so that the directory gets rebuilt from
> the salvaged directory entries. Right?
Yes.
>
> So I think you could structure this more like:
>
> /* salvage any dirents that look ok */
> longform_dir2_entry_check_data(...);
>
> /* check v5 metadata */
> if (xfs_has_crc(mp)) {
> error = check_dir3_header(mp, bp, ino);
> if (error) {
> fixit++;
> if (fmt == XFS_DIR2_FMT_BLOCK)
> goto out_fix;
>
> libxfs_buf_relse(bp);
> bp = NULL;
> continue;
> }
> }
>
> if (fmt == XFS_DIR2_FMT_BLOCK)
> break;
>
> libxfs_buf_relse(bp);
> bp = NULL;
> }
Ah, this makes better sense.
>
> > + if (fmt == XFS_DIR2_FMT_BLOCK)
> > + goto out_fix;
> > +
> > + libxfs_buf_relse(bp);
> > + bp = NULL;
> > + continue;
> > + }
> > + }
> > + else {
> > + /* No crc. Directory appears to be shortform. */
> > error = check_dir3_header(mp, bp, ino);
>
> dir3 headers (as opposed to dir2 headers) are a crc-only feature, so
> this isn't correct either.
Agree.
>
> > if (error) {
> > fixit++;
> > @@ -2438,9 +2455,6 @@ longform_dir2_entry_check(
> > }
> > }
> >
> > - longform_dir2_entry_check_data(mp, ip, num_illegal, need_dot,
> > - irec, ino_offset, bp, hashtab,
> > - &freetab, da_bno, fmt == XFS_DIR2_FMT_BLOCK);
>
> and removing this call means that we never scan a V4 directory at all.
Doh! I'll fix it up and send a v3. Thanks for your review.
-Bill
>
> --D
>
> > if (fmt == XFS_DIR2_FMT_BLOCK)
> > break;
> >
> > --
> > 2.49.0
> >
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-04-15 17:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-15 15:01 [PATCH v2] xfs_repair: fix link counts update following repair of a bad block bodonnel
2025-04-15 17:07 ` Darrick J. Wong
2025-04-15 17:17 ` Bill O'Donnell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox