Forced quotacheck after unclean unmount since 3.11, bisected

Hello,

I've noticed that since Linux 3.11, quotacheck always happen after unclean unmounts. Is this expected? It didn't happen on previous Linux versions.

A bisect shows that this commit is responsible:
xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD
id: 83e782e1a1cc0159888e58e14dfc8f3289663338

Relevant dmesg:

[    8.844063] XFS (md4): Mounting Filesystem
[    9.054023] XFS (md4): Starting recovery (logdev: internal)
[    9.331785] XFS (md4): Ending recovery (logdev: internal)
[    9.383856] XFS (md4): Quotacheck needed: Please wait.
[   48.427732] XFS (md4): Quotacheck: Done.

More details:
* vanilla kernel
* on top of mdadm (RAID1)
* xfsprogs 3.1.5
* mount options: noatime,nosuid,nodev,grpquota,inode64
* xfs_info:

meta-data=/dev/md4               isize=256    agcount=4, agsize=119997280 blks
         =                       sectsz=512   attr=2
data     =                       bsize=4096   blocks=479989120, imaxpct=5
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0
log      =internal               bsize=4096   blocks=234369, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =none                   extsz=4096   blocks=0, rtextents=0

Thanks,

The commit

83e782e xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD

added a new function xfs_sb_quota_from_disk() which swaps on-disk XFS_OQUOTA_* flags for in-core XFS_GQUOTA_* and XFS_PQUOTA_* flags after the superblock is read.

However, if log recovery is required, the superblock is read again, and the modified in-core flags are re-read from disk, so we have XFS_OQUOTA_* flags in memory again. This causes the XFS_QM_NEED_QUOTACHECK() test to be true, because the XFS_OQUOTA_CHKD is still set, and not XFS_GQUOTA_CHKD or XFS_PQUOTA_CHKD.

The simple one-line fix is to call xfs_sb_quota_from_disk after we re-read the superblock during log recovery.

Reported-by Cyril B. <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

Quick-tested only; it seems to resolve Cyril's testcase, but I've not done an xfstests run yet as this is somewhat of an RFC:

It feels like there may be a better/more systemic fix here, though. We can't call xfs_sb_quota_from_disk it from xfs_sb_from_disk(), because the sb read verifier wants to know what was really on disk, not what was fixed up after the fact.

We could add an "xlate_quota" arg to xfs_sb_from_disk(), but that feels a little odd; the function today quite clearly only does endian conversions, not other more detailed translations.

But if something like that is preferred, I can send a patch V2.

fs/xfs/xfs_log_recover.c +1 -0

index bce53ac..0a29de7 100644
@@ -4429,6 +4429,7 @@ xlog_do_recover(
 	/* Convert superblock from on-disk format */
 	sbp = &log->l_mp->m_sb;
 	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
+	xfs_sb_quota_from_disk(sbp);
 	ASSERT(sbp->sb_magicnum == XFS_SB_MAGIC);
 	ASSERT(xfs_sb_good_version(sbp));
 	xfs_buf_relse(bp);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

The commit

83e782e xfs: Remove incore use of XFS_OQUOTA_ENFD and XFS_OQUOTA_CHKD

added a new function xfs_sb_quota_from_disk() which swaps on-disk XFS_OQUOTA_* flags for in-core XFS_GQUOTA_* and XFS_PQUOTA_* flags after the superblock is read.

However, if log recovery is required, the superblock is read again, and the modified in-core flags are re-read from disk, so we have XFS_OQUOTA_* flags in memory again. This causes the XFS_QM_NEED_QUOTACHECK() test to be true, because the XFS_OQUOTA_CHKD is still set, and not XFS_GQUOTA_CHKD or XFS_PQUOTA_CHKD.

Change xfs_sb_from_disk to call xfs_sb_quota_from disk and always convert the disk flags to in-memory flags.

Add a lower-level function which can be called with "false" to not convert the flags, so that the sb verifier can verify exactly what was on disk, per Brian Foster's suggestion.

Reported-by Cyril B. <[email protected]>

Signed-off-by Eric Sandeen <[email protected]>

---

V2: Call xfs_sb_quota_from_disk from xfs_sb_from_disk, unless we're the verifier, which wants to see what is actually on-disk to verify it. Thanks Brian!

fs/xfs/xfs_mount.c +0 -1

index 3507cd0..16d1671 100644
@@ -324,7 +324,6 @@ reread:
 	 * Initialize the mount structure from the superblock.
 	 */
 	xfs_sb_from_disk(sbp, XFS_BUF_TO_SBP(bp));
-	xfs_sb_quota_from_disk(sbp);
 
 	/*
 	 * If we haven't validated the superblock, do so now before we try

fs/xfs/xfs_sb.c +20 -4

index 7703fa6..0f631d2 100644
@@ -386,10 +386,11 @@ xfs_sb_quota_from_disk(struct xfs_sb *sbp)
 	}
 }
 
-void
-xfs_sb_from_disk(
+static void
+__xfs_sb_from_disk(
 	struct xfs_sb	*to,
-	xfs_dsb_t	*from)
+	xfs_dsb_t	*from,
+	bool		convert_xquota)
 {
 	to->sb_magicnum = be32_to_cpu(from->sb_magicnum);
 	to->sb_blocksize = be32_to_cpu(from->sb_blocksize);

@@ -445,6 +446,17 @@ xfs_sb_from_disk(
 	to->sb_pad = 0;
 	to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
 	to->sb_lsn = be64_to_cpu(from->sb_lsn);
+	/* Convert on-disk flags to in-memory flags? */
+	if (convert_xquota)
+		xfs_sb_quota_from_disk(to);
+}
+
+void
+xfs_sb_from_disk(
+	struct xfs_sb	*to,
+	xfs_dsb_t	*from)
+{
+	__xfs_sb_from_disk(to, from, true);
 }
 
 static inline void

@@ -577,7 +589,11 @@ xfs_sb_verify(
 	struct xfs_mount *mp = bp->b_target->bt_mount;
 	struct xfs_sb	sb;
 
-	xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp));
+	/*
+	 * Use call variant which doesn't convert quota flags from disk 
+	 * format, because xfs_mount_validate_sb checks the on-disk flags.
+	 */
+	__xfs_sb_from_disk(&sb, XFS_BUF_TO_SBP(bp), false);
 
 	/*
 	 * Only check the in progress field for the primary superblock as

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs