[05/17] xfs_scrub: don't report data loss in unlinked inodes twice

Commit Message

Darrick J. Wong Feb. 6, 2025, 10:31 p.m. UTC

From: Darrick J. Wong <[e-mail]>

If parent pointers are enabled, report_ioerr_fsmap will report lost file
data and xattrs for all files, having used the parent pointer ioctls to
generate the path of the lost file.  For unlinked files, the path lookup
will fail, but we'll report the inumber of the file that lost data.

Therefore, we don't need to do a separate scan of the unlinked inodes
in report_all_media_errors after doing the fsmap scan.

Cc: <[e-mail]> # v6.10.0
Fixes: 9b5d1349ca5fb1 ("xfs_scrub: use parent pointers to report lost file data")
Signed-off-by: "Darrick J. Wong" <[e-mail]>
---
 scrub/phase6.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Christoph Hellwig Feb. 7, 2025, 4:35 a.m. UTC | #1

On Thu, Feb 06, 2025 at 02:31:57PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <[e-mail]>
> 
> If parent pointers are enabled, report_ioerr_fsmap will report lost file
> data and xattrs for all files, having used the parent pointer ioctls to
> generate the path of the lost file.  For unlinked files, the path lookup
> will fail, but we'll report the inumber of the file that lost data.

Maybe also add this to the code as a comment?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[e-mail]>

Darrick J. Wong Feb. 7, 2025, 4:46 a.m. UTC | #2

On Thu, Feb 06, 2025 at 08:35:34PM -0800, Christoph Hellwig wrote:
> On Thu, Feb 06, 2025 at 02:31:57PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <[e-mail]>
> > 
> > If parent pointers are enabled, report_ioerr_fsmap will report lost file
> > data and xattrs for all files, having used the parent pointer ioctls to
> > generate the path of the lost file.  For unlinked files, the path lookup
> > will fail, but we'll report the inumber of the file that lost data.
> 
> Maybe also add this to the code as a comment?

Will do.

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <[e-mail]>

Thanks!

--D

Patch

diff --git a/scrub/phase6.c b/scrub/phase6.c
index fc63f5aad0bd7b..2695e645004bf1 100644
--- a/scrub/phase6.c
+++ b/scrub/phase6.c
@@ -569,12 +569,12 @@  report_all_media_errors(
 	 * Scan the directory tree to get file paths if we didn't already use
 	 * directory parent pointers to report the loss.
 	 */
-	if (!can_use_pptrs(ctx)) {
-		ret = scan_fs_tree(ctx, report_dir_loss, report_dirent_loss,
-				vs);
-		if (ret)
-			return ret;
-	}
+	if (can_use_pptrs(ctx))
+		return 0;
+
+	ret = scan_fs_tree(ctx, report_dir_loss, report_dirent_loss, vs);
+	if (ret)
+		return ret;
 
 	/* Scan for unlinked files. */
 	return scrub_scan_all_inodes(ctx, report_inode_loss, 0, vs);