Skip to content

Commit 4a9bca8

Browse files
author
Darrick J. Wong
committed
xfs: fix online fsck handling of v5 feature bits on secondary supers
While I was auditing the code in xfs_repair that adds feature bits to existing V5 filesystems, I decided to have a look at how online fsck handles feature bits, and I found a few problems: 1) ATTR2 is added to the primary super when an xattr is set to a file, but that isn't consistently propagated to secondary supers. This isn't a corruption, merely a discrepancy that repair will fix if it ever has to restore the primary from a secondary. Hence, if we find a mismatch on a secondary, this is a preen condition, not a corruption. 2) There are more compat and ro_compat features now than there used to be, but we mask off the newer features from testing. This means we ignore inconsistencies in the INOBTCOUNT and BIGTIME features, which is wrong. Get rid of the masking and compare directly. 3) NEEDSREPAIR, when set on a secondary, is ignored by everyone. Hence a mismatch here should also be flagged for preening, and online repair should clear the flag. Right now we ignore it due to (2). 4) log_incompat features are ephemeral, since we can clear the feature bit as soon as the log no longer contains live records for a particular log feature. As such, the only copy we care about is the one in the primary super. If we find any bits set in the secondary super, we should flag that for preening, and clear the bits if the user elects to repair it. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Dave Chinner <dchinner@redhat.com>
1 parent 65552b0 commit 4a9bca8

File tree

2 files changed

+38
-27
lines changed

2 files changed

+38
-27
lines changed

fs/xfs/scrub/agheader.c

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ xchk_superblock(
281281
features_mask = cpu_to_be32(XFS_SB_VERSION2_ATTR2BIT);
282282
if ((sb->sb_features2 & features_mask) !=
283283
(cpu_to_be32(mp->m_sb.sb_features2) & features_mask))
284-
xchk_block_set_corrupt(sc, bp);
284+
xchk_block_set_preen(sc, bp);
285285

286286
if (!xfs_has_crc(mp)) {
287287
/* all v5 fields must be zero */
@@ -290,39 +290,38 @@ xchk_superblock(
290290
offsetof(struct xfs_dsb, sb_features_compat)))
291291
xchk_block_set_corrupt(sc, bp);
292292
} else {
293-
/* Check compat flags; all are set at mkfs time. */
294-
features_mask = cpu_to_be32(XFS_SB_FEAT_COMPAT_UNKNOWN);
295-
if ((sb->sb_features_compat & features_mask) !=
296-
(cpu_to_be32(mp->m_sb.sb_features_compat) & features_mask))
293+
/* compat features must match */
294+
if (sb->sb_features_compat !=
295+
cpu_to_be32(mp->m_sb.sb_features_compat))
297296
xchk_block_set_corrupt(sc, bp);
298297

299-
/* Check ro compat flags; all are set at mkfs time. */
300-
features_mask = cpu_to_be32(XFS_SB_FEAT_RO_COMPAT_UNKNOWN |
301-
XFS_SB_FEAT_RO_COMPAT_FINOBT |
302-
XFS_SB_FEAT_RO_COMPAT_RMAPBT |
303-
XFS_SB_FEAT_RO_COMPAT_REFLINK);
304-
if ((sb->sb_features_ro_compat & features_mask) !=
305-
(cpu_to_be32(mp->m_sb.sb_features_ro_compat) &
306-
features_mask))
298+
/* ro compat features must match */
299+
if (sb->sb_features_ro_compat !=
300+
cpu_to_be32(mp->m_sb.sb_features_ro_compat))
307301
xchk_block_set_corrupt(sc, bp);
308302

309-
/* Check incompat flags; all are set at mkfs time. */
310-
features_mask = cpu_to_be32(XFS_SB_FEAT_INCOMPAT_UNKNOWN |
311-
XFS_SB_FEAT_INCOMPAT_FTYPE |
312-
XFS_SB_FEAT_INCOMPAT_SPINODES |
313-
XFS_SB_FEAT_INCOMPAT_META_UUID);
314-
if ((sb->sb_features_incompat & features_mask) !=
315-
(cpu_to_be32(mp->m_sb.sb_features_incompat) &
316-
features_mask))
317-
xchk_block_set_corrupt(sc, bp);
303+
/*
304+
* NEEDSREPAIR is ignored on a secondary super, so we should
305+
* clear it when we find it, though it's not a corruption.
306+
*/
307+
features_mask = cpu_to_be32(XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR);
308+
if ((cpu_to_be32(mp->m_sb.sb_features_incompat) ^
309+
sb->sb_features_incompat) & features_mask)
310+
xchk_block_set_preen(sc, bp);
318311

319-
/* Check log incompat flags; all are set at mkfs time. */
320-
features_mask = cpu_to_be32(XFS_SB_FEAT_INCOMPAT_LOG_UNKNOWN);
321-
if ((sb->sb_features_log_incompat & features_mask) !=
322-
(cpu_to_be32(mp->m_sb.sb_features_log_incompat) &
323-
features_mask))
312+
/* all other incompat features must match */
313+
if ((cpu_to_be32(mp->m_sb.sb_features_incompat) ^
314+
sb->sb_features_incompat) & ~features_mask)
324315
xchk_block_set_corrupt(sc, bp);
325316

317+
/*
318+
* log incompat features protect newer log record types from
319+
* older log recovery code. Log recovery doesn't check the
320+
* secondary supers, so we can clear these if needed.
321+
*/
322+
if (sb->sb_features_log_incompat)
323+
xchk_block_set_preen(sc, bp);
324+
326325
/* Don't care about sb_crc */
327326

328327
if (sb->sb_spino_align != cpu_to_be32(mp->m_sb.sb_spino_align))

fs/xfs/scrub/agheader_repair.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,18 @@ xrep_superblock(
5252
xfs_buf_zero(bp, 0, BBTOB(bp->b_length));
5353
xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
5454

55+
/*
56+
* Don't write out a secondary super with NEEDSREPAIR or log incompat
57+
* features set, since both are ignored when set on a secondary.
58+
*/
59+
if (xfs_has_crc(mp)) {
60+
struct xfs_dsb *sb = bp->b_addr;
61+
62+
sb->sb_features_incompat &=
63+
~cpu_to_be32(XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR);
64+
sb->sb_features_log_incompat = 0;
65+
}
66+
5567
/* Write this to disk. */
5668
xfs_trans_buf_set_type(sc->tp, bp, XFS_BLFT_SB_BUF);
5769
xfs_trans_log_buf(sc->tp, bp, 0, BBTOB(bp->b_length) - 1);

0 commit comments

Comments
 (0)