Skip to content

Commit a33f5c3

Browse files
committed
Merge tag 'xfs-5.17-merge-3' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
Pull xfs fixes from Darrick Wong: "These are the last few obvious fixes that I found while stress testing online fsck for XFS prior to initiating a design review of the whole giant machinery. - Fix a minor locking inconsistency in readdir - Fix incorrect fs feature bit validation for secondary superblocks" * tag 'xfs-5.17-merge-3' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux: xfs: fix online fsck handling of v5 feature bits on secondary supers xfs: take the ILOCK when readdir inspects directory mapping data
2 parents 112450d + 4a9bca8 commit a33f5c3

File tree

3 files changed

+72
-46
lines changed

3 files changed

+72
-46
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);

fs/xfs/xfs_dir2_readdir.c

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -138,15 +138,15 @@ xfs_dir2_sf_getdents(
138138
STATIC int
139139
xfs_dir2_block_getdents(
140140
struct xfs_da_args *args,
141-
struct dir_context *ctx)
141+
struct dir_context *ctx,
142+
unsigned int *lock_mode)
142143
{
143144
struct xfs_inode *dp = args->dp; /* incore directory inode */
144145
struct xfs_buf *bp; /* buffer for block */
145146
int error; /* error return value */
146147
int wantoff; /* starting block offset */
147148
xfs_off_t cook;
148149
struct xfs_da_geometry *geo = args->geo;
149-
int lock_mode;
150150
unsigned int offset, next_offset;
151151
unsigned int end;
152152

@@ -156,12 +156,13 @@ xfs_dir2_block_getdents(
156156
if (xfs_dir2_dataptr_to_db(geo, ctx->pos) > geo->datablk)
157157
return 0;
158158

159-
lock_mode = xfs_ilock_data_map_shared(dp);
160159
error = xfs_dir3_block_read(args->trans, dp, &bp);
161-
xfs_iunlock(dp, lock_mode);
162160
if (error)
163161
return error;
164162

163+
xfs_iunlock(dp, *lock_mode);
164+
*lock_mode = 0;
165+
165166
/*
166167
* Extract the byte offset we start at from the seek pointer.
167168
* We'll skip entries before this.
@@ -344,7 +345,8 @@ STATIC int
344345
xfs_dir2_leaf_getdents(
345346
struct xfs_da_args *args,
346347
struct dir_context *ctx,
347-
size_t bufsize)
348+
size_t bufsize,
349+
unsigned int *lock_mode)
348350
{
349351
struct xfs_inode *dp = args->dp;
350352
struct xfs_mount *mp = dp->i_mount;
@@ -356,7 +358,6 @@ xfs_dir2_leaf_getdents(
356358
xfs_dir2_off_t curoff; /* current overall offset */
357359
int length; /* temporary length value */
358360
int byteoff; /* offset in current block */
359-
int lock_mode;
360361
unsigned int offset = 0;
361362
int error = 0; /* error return value */
362363

@@ -390,13 +391,16 @@ xfs_dir2_leaf_getdents(
390391
bp = NULL;
391392
}
392393

393-
lock_mode = xfs_ilock_data_map_shared(dp);
394+
if (*lock_mode == 0)
395+
*lock_mode = xfs_ilock_data_map_shared(dp);
394396
error = xfs_dir2_leaf_readbuf(args, bufsize, &curoff,
395397
&rablk, &bp);
396-
xfs_iunlock(dp, lock_mode);
397398
if (error || !bp)
398399
break;
399400

401+
xfs_iunlock(dp, *lock_mode);
402+
*lock_mode = 0;
403+
400404
xfs_dir3_data_check(dp, bp);
401405
/*
402406
* Find our position in the block.
@@ -496,7 +500,7 @@ xfs_dir2_leaf_getdents(
496500
*
497501
* If supplied, the transaction collects locked dir buffers to avoid
498502
* nested buffer deadlocks. This function does not dirty the
499-
* transaction. The caller should ensure that the inode is locked
503+
* transaction. The caller must hold the IOLOCK (shared or exclusive)
500504
* before calling this function.
501505
*/
502506
int
@@ -507,29 +511,40 @@ xfs_readdir(
507511
size_t bufsize)
508512
{
509513
struct xfs_da_args args = { NULL };
510-
int rval;
511-
int v;
514+
unsigned int lock_mode;
515+
int isblock;
516+
int error;
512517

513518
trace_xfs_readdir(dp);
514519

515520
if (xfs_is_shutdown(dp->i_mount))
516521
return -EIO;
517522

518523
ASSERT(S_ISDIR(VFS_I(dp)->i_mode));
524+
ASSERT(xfs_isilocked(dp, XFS_IOLOCK_SHARED | XFS_IOLOCK_EXCL));
519525
XFS_STATS_INC(dp->i_mount, xs_dir_getdents);
520526

521527
args.dp = dp;
522528
args.geo = dp->i_mount->m_dir_geo;
523529
args.trans = tp;
524530

525531
if (dp->i_df.if_format == XFS_DINODE_FMT_LOCAL)
526-
rval = xfs_dir2_sf_getdents(&args, ctx);
527-
else if ((rval = xfs_dir2_isblock(&args, &v)))
528-
;
529-
else if (v)
530-
rval = xfs_dir2_block_getdents(&args, ctx);
531-
else
532-
rval = xfs_dir2_leaf_getdents(&args, ctx, bufsize);
532+
return xfs_dir2_sf_getdents(&args, ctx);
533533

534-
return rval;
534+
lock_mode = xfs_ilock_data_map_shared(dp);
535+
error = xfs_dir2_isblock(&args, &isblock);
536+
if (error)
537+
goto out_unlock;
538+
539+
if (isblock) {
540+
error = xfs_dir2_block_getdents(&args, ctx, &lock_mode);
541+
goto out_unlock;
542+
}
543+
544+
error = xfs_dir2_leaf_getdents(&args, ctx, bufsize, &lock_mode);
545+
546+
out_unlock:
547+
if (lock_mode)
548+
xfs_iunlock(dp, lock_mode);
549+
return error;
535550
}

0 commit comments

Comments
 (0)