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: <linux-xfs@vger.kernel.org> # v6.10.0
Fixes: 83ffb5b454b172 ("xfs_scrub: start tracking scrub state in scrub_item")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/scrub/repair.c b/scrub/repair.c
index fdefc2131aa453617de857c6ed09ec4fa34d2faa..c118938fe213036abca7889c93f446220db189d2 100644 (file)
--- 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 ec4aa381a82d36a490d92e7cfe4ed6ff919f4cb5..af3a9d7186242a2ab7a92fd76e9915779a0e5285 100644 (file)
--- 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,