From: Darrick J. Wong Date: Mon, 8 Jun 2026 16:24:48 +0000 (-0700) Subject: xfs_scrub: actually handle NEEDSCHECK scrub items in phase 4 X-Git-Tag: v7.1.0~23 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b43e9064d665e3c3bec6931cd0ab19a7ad8c6e79;p=xfsprogs-dev.git xfs_scrub: actually handle NEEDSCHECK scrub items in phase 4 While reading the code in repair.c while triaging another Codex complaint, I remembered that phases 2 and 3 react to an un-scannable metadata object by dumping them on the repair list for phase 4 with the "NEEDSCHECK" flag set. However, repair_item() doesn't actually process those NEEDSCHECK items, which means that we could silently fail to complete a full scan. There are two ways that repair_item() gets called -- one is the action list processing in phase 4; and the other is repair_item_completely, which is called directly from phases 2, 5, and 7. Action list items never have NEEDSCHECK set because repair_item_to_action_item promotes them into "corruptions" to force a check; and the repair_item_completely callers only ever pass in actual corruptions. Therefore, we should amend repair_item() to process NEEDSCHECK action items so as not to leave a logic bomb for future authors. We should also set sri_inconsistent so that any errors that are found during the NEEDSCHECK scans result in a revalidation after the repair. However, there's a real bug in repair_item_to_action_item. If @sri says there's at least one NEEDSCHECK item but no corruption elsewhere, then we exit early instead of queuing an action list item. This apparently doesn't happen much in the online fsck QA suite becuase phases 2 and 3 aggressively try to clear NEEDSCHECK items and only deferring scans to phase 4 if they cannot make any forward progress. Cc: # v6.10.0 Fixes: 83ffb5b454b172 ("xfs_scrub: start tracking scrub state in scrub_item") Signed-off-by: "Darrick J. Wong" Reviewed-by: Christoph Hellwig --- diff --git a/scrub/repair.c b/scrub/repair.c index fdefc213..c118938f 100644 --- a/scrub/repair.c +++ b/scrub/repair.c @@ -260,10 +260,11 @@ repair_item_dependencies_ok( if (!(dep_mask & 1)) continue; /* - * If this lower level object also needs repair, we can't fix - * the higher level item. + * If this lower level object also needs repair or hasn't been + * scanned yet, we can't fix the higher level item. */ - if (sri->sri_state[b] & SCRUB_ITEM_NEEDSREPAIR) + if (sri->sri_state[b] & (SCRUB_ITEM_NEEDSREPAIR | + SCRUB_ITEM_NEEDSCHECK)) return false; } @@ -855,7 +856,11 @@ repair_item( if (ret) return ret; - return repair_item_class(ctx, sri, -1, SCRUB_ITEM_PREEN, flags); + ret = repair_item_class(ctx, sri, -1, SCRUB_ITEM_PREEN, flags); + if (ret) + return ret; + + return repair_item_class(ctx, sri, -1, SCRUB_ITEM_NEEDSCHECK, flags); } /* Create an action item around a scrub item that needs repairs. */ @@ -868,7 +873,7 @@ repair_item_to_action_item( struct action_item *aitem; unsigned int scrub_type; - if (repair_item_count_needsrepair(sri) == 0) + if (repair_item_count_needswork(sri) == 0) return 0; aitem = malloc(sizeof(struct action_item)); @@ -893,6 +898,7 @@ repair_item_to_action_item( if (state[scrub_type] & SCRUB_ITEM_NEEDSCHECK) { state[scrub_type] &= ~SCRUB_ITEM_NEEDSCHECK; state[scrub_type] |= SCRUB_ITEM_CORRUPT; + aitem->sri.sri_inconsistent = true; } } diff --git a/scrub/repair.h b/scrub/repair.h index ec4aa381..af3a9d71 100644 --- a/scrub/repair.h +++ b/scrub/repair.h @@ -96,6 +96,20 @@ repair_item_count_needsrepair( return nr; } +static inline unsigned int +repair_item_count_needswork( + const struct scrub_item *sri) +{ + unsigned int scrub_type; + unsigned int nr = 0; + + foreach_scrub_type(scrub_type) + if (sri->sri_state[scrub_type] & (SCRUB_ITEM_REPAIR_ANY | + SCRUB_ITEM_NEEDSCHECK)) + nr++; + return nr; +} + static inline int repair_item_completely( struct scrub_ctx *ctx,