* Forced quotacheck after unclean unmount since 3.11, bisected
@ 2014-07-22 13:16 Cyril B.
2014-07-22 16:48 ` Eric Sandeen
2014-07-22 17:10 ` [PATCH] xfs: avoid false quotacheck after unclean shutdown Eric Sandeen
0 siblings, 2 replies; 6+ messages in thread
From: Cyril B. @ 2014-07-22 13:16 UTC (permalink / raw)
To: xfs
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,
--
Cyril B.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Forced quotacheck after unclean unmount since 3.11, bisected
2014-07-22 13:16 Forced quotacheck after unclean unmount since 3.11, bisected Cyril B.
@ 2014-07-22 16:48 ` Eric Sandeen
2014-07-22 17:10 ` [PATCH] xfs: avoid false quotacheck after unclean shutdown Eric Sandeen
1 sibling, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2014-07-22 16:48 UTC (permalink / raw)
To: cbay, xfs
On 7/22/14, 8:16 AM, Cyril B. wrote:
> 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.
I can reproduce this, I'll take a look - thanks for bisecting!
-Eric
> 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,
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] xfs: avoid false quotacheck after unclean shutdown
2014-07-22 13:16 Forced quotacheck after unclean unmount since 3.11, bisected Cyril B.
2014-07-22 16:48 ` Eric Sandeen
@ 2014-07-22 17:10 ` Eric Sandeen
2014-07-23 13:20 ` Brian Foster
2014-07-24 23:29 ` [PATCH V2] " Eric Sandeen
1 sibling, 2 replies; 6+ messages in thread
From: Eric Sandeen @ 2014-07-22 17:10 UTC (permalink / raw)
To: cbay, 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.
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. <cbay@excellency.fr>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
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.
diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index bce53ac..0a29de7 100644
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: avoid false quotacheck after unclean shutdown
2014-07-22 17:10 ` [PATCH] xfs: avoid false quotacheck after unclean shutdown Eric Sandeen
@ 2014-07-23 13:20 ` Brian Foster
2014-07-24 23:29 ` [PATCH V2] " Eric Sandeen
1 sibling, 0 replies; 6+ messages in thread
From: Brian Foster @ 2014-07-23 13:20 UTC (permalink / raw)
To: Eric Sandeen; +Cc: cbay, xfs
On Tue, Jul 22, 2014 at 12:10:14PM -0500, Eric Sandeen wrote:
> 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.
>
First off, this looks reasonable to me given the current code.
> Reported-by: Cyril B. <cbay@excellency.fr>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> 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.
>
I was wondering why this is the case because the verifier runs in the
I/O path, so it would hit before we actually return with the buffer in
hand in the mount codepath. Looking a bit further, I see that we call
xfs_sb_from_disk() from within the verifier (xfs_sb_verify()) and
apparently some flag validation is done on the result of that. So I
suspect that's what you mean here and probably why
xfs_sb_quota_from_disk() is outside of xfs_sb_from_disk().
FWIW, xfs_sb_quota_to_disk() is called within xfs_sb_to_disk(). IMO, the
more clear thing to do is make xfs_sb_to_disk() and xfs_sb_from_disk()
consistent in behavior and let the special verifier case deal with the
quirk. In other words, rename it to something like
__xfs_sb_from_disk(..., bool convert_flags) and conditionally call
xfs_sb_quota_from_disk(). Let the verifier call that variant directly
with false and then #define xfs_sb_from_disk to the true variant. Just
my .02. ;)
Brian
> 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.
>
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index bce53ac..0a29de7 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2] xfs: avoid false quotacheck after unclean shutdown
2014-07-22 17:10 ` [PATCH] xfs: avoid false quotacheck after unclean shutdown Eric Sandeen
2014-07-23 13:20 ` Brian Foster
@ 2014-07-24 23:29 ` Eric Sandeen
2014-07-28 15:02 ` Brian Foster
1 sibling, 1 reply; 6+ messages in thread
From: Eric Sandeen @ 2014-07-24 23:29 UTC (permalink / raw)
To: cbay, 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. <cbay@excellency.fr>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
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!
diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
index 3507cd0..16d1671 100644
--- a/fs/xfs/xfs_mount.c
+++ b/fs/xfs/xfs_mount.c
@@ -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
diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
index 7703fa6..0f631d2 100644
--- a/fs/xfs/xfs_sb.c
+++ b/fs/xfs/xfs_sb.c
@@ -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
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2] xfs: avoid false quotacheck after unclean shutdown
2014-07-24 23:29 ` [PATCH V2] " Eric Sandeen
@ 2014-07-28 15:02 ` Brian Foster
0 siblings, 0 replies; 6+ messages in thread
From: Brian Foster @ 2014-07-28 15:02 UTC (permalink / raw)
To: Eric Sandeen; +Cc: cbay, xfs
On Thu, Jul 24, 2014 at 06:29:34PM -0500, Eric Sandeen wrote:
> 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. <cbay@excellency.fr>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
Looks good to me, thanks!
Reviewed-by: Brian Foster <bfoster@redhat.com>
>
> 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!
>
>
> diff --git a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c
> index 3507cd0..16d1671 100644
> --- a/fs/xfs/xfs_mount.c
> +++ b/fs/xfs/xfs_mount.c
> @@ -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
> diff --git a/fs/xfs/xfs_sb.c b/fs/xfs/xfs_sb.c
> index 7703fa6..0f631d2 100644
> --- a/fs/xfs/xfs_sb.c
> +++ b/fs/xfs/xfs_sb.c
> @@ -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
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-28 15:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-22 13:16 Forced quotacheck after unclean unmount since 3.11, bisected Cyril B.
2014-07-22 16:48 ` Eric Sandeen
2014-07-22 17:10 ` [PATCH] xfs: avoid false quotacheck after unclean shutdown Eric Sandeen
2014-07-23 13:20 ` Brian Foster
2014-07-24 23:29 ` [PATCH V2] " Eric Sandeen
2014-07-28 15:02 ` Brian Foster
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.