On Mon, Jun 04, 2018 at 09:03:41AM +1000, Dave Chinner wrote: > On Mon, Jun 04, 2018 at 08:34:35AM +1000, Dave Chinner wrote: > > On Sun, Jun 03, 2018 at 06:19:54PM -0400, Wen Xu wrote: > > > Hi XFS developers and maintainers, > > > Please check: https://bugzilla.kernel.org/show_bug.cgi?id=199915 > > > > Please report bugs straight to this list - it's much easier to > > track and faster to discuss bugs through email than it is through > > bugzilla. > > Image log size is too small for the filesystem config. If this was > a v5 filesystem, then the image would refuse to mount right there. > > As it is, a CONFIG_XFS_DEBUG=y kernel assert fails at mount in > xfs_check_agi_unlinked(). > > #ifdef DEBUG > STATIC void > xfs_check_agi_unlinked( > struct xfs_agi *agi) > { > int i; > > for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) > ASSERT(agi->agi_unlinked[i]); > } > #else > #define xfs_check_agi_unlinked(agi) > #endif > > With the patch below, the image fails to mount with an error: > > [ 89.695774] XFS (loop0): Mounting V4 Filesystem > [ 89.696941] XFS (loop0): Log size 864 blocks too small, minimum size is 942 blocks > [ 89.698603] XFS (loop0): Log size out of supported range. > [ 89.699833] XFS (loop0): Continuing onwards, but if log hangs are experienced then please report this message in the bug report. > [ 89.703651] XFS (loop0): Metadata corruption detected at xfs_agi_verify+0x153/0x170, xfs_agi block 0x2 > [ 89.705349] XFS (loop0): Unmount and run xfs_repair > [ 89.706179] XFS (loop0): First 128 bytes of corrupted metadata buffer: > [ 89.707317] 00000000bef77f4d: 58 41 47 49 00 00 00 01 00 00 00 00 00 00 10 00 XAGI............ > [ 89.708824] 000000003cd6f10c: 00 00 00 40 00 00 00 03 00 00 00 01 00 00 00 00 ...@............ > [ 89.710309] 000000002ec75c4c: 00 00 00 00 00 00 00 00 00 00 00 ff ff ff ff ff ................ > [ 89.711749] 00000000d83fa14d: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 89.713127] 00000000c41242dd: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 89.714484] 000000006c41c134: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 89.715841] 00000000b8f9ac70: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ................ > [ 89.717145] 000000005258d181: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00 ................ > [ 89.718444] XFS (loop0): metadata I/O error in "xfs_trans_read_buf_map" at daddr 0x2 len 1 error 117 > [ 89.719834] XFS (loop0): xfs_imap_lookup: xfs_ialloc_read_agi() returned error -117, agno 0 > [ 89.721099] XFS (loop0): failed to read root inode > > Please rework the test image so that it mounts cleanly without > warnings or shutdowns with the patch below and then retest the POC > code. > > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx > > xfs: verify AGI unlinked list contains valid blocks > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > The heads of tha AGI unlinked list are only scanned on debug > kernels when the verifier runs. Change that to always scan the heads > and validate that the inode numbers are valid. > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> Looks good to me, Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> --D > > --- > fs/xfs/libxfs/xfs_ialloc.c | 23 ++++++++--------------- > 1 file changed, 8 insertions(+), 15 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c > index 1ef6ff2ca002..ec5ea02b5553 100644 > --- a/fs/xfs/libxfs/xfs_ialloc.c > +++ b/fs/xfs/libxfs/xfs_ialloc.c > @@ -2465,26 +2465,13 @@ xfs_ialloc_log_agi( > } > } > > -#ifdef DEBUG > -STATIC void > -xfs_check_agi_unlinked( > - struct xfs_agi *agi) > -{ > - int i; > - > - for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) > - ASSERT(agi->agi_unlinked[i]); > -} > -#else > -#define xfs_check_agi_unlinked(agi) > -#endif > - > static xfs_failaddr_t > xfs_agi_verify( > struct xfs_buf *bp) > { > struct xfs_mount *mp = bp->b_target->bt_mount; > struct xfs_agi *agi = XFS_BUF_TO_AGI(bp); > + int i; > > if (xfs_sb_version_hascrc(&mp->m_sb)) { > if (!uuid_equal(&agi->agi_uuid, &mp->m_sb.sb_meta_uuid)) > @@ -2520,7 +2507,13 @@ xfs_agi_verify( > if (bp->b_pag && be32_to_cpu(agi->agi_seqno) != bp->b_pag->pag_agno) > return __this_address; > > - xfs_check_agi_unlinked(agi); > + for (i = 0; i < XFS_AGI_UNLINKED_BUCKETS; i++) { > + if (agi->agi_unlinked[i] == NULLAGINO) > + continue; > + if (!xfs_verify_ino(mp, be32_to_cpu(agi->agi_unlinked[i]))) > + return __this_address; > + } > + > return NULL; > } > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
