2007-12-18 14:04:19

by Jan Engelhardt

[permalink] [raw]
Subject: Do not reset xfsquota flags on quotaless ro mount

Hi,


In https://bugzilla.novell.com/show_bug.cgi?id=345338 it is claimed that
resetting the quota flags in the mounting sequence rw,ro,rw is a bug, but I
would say this is not the case, as quota is metadata, and the log is replayed
in ro mode even for other filesystems. Yet, it is still not nice, and I have
been trying with this patch, which does not do the right thing yet. (It does a
recovery when mounting for the 3rd time, which probably just says that I did
not know too much of xfs internals to cook up a nicely working patch.)
Opinions?

===

In the following action sequence, quota is unnecessarily recalculated
at the 3rd mount invocation:

mount -o usrquota,grpquota /dev/mapper/myxfs /mnt
umount /mnt
mount -o ro /dev/mapper/myxfs /mnt
umount /mnt
mount -o usrquota,grpquota /dev/mapper/myxfs /mnt

This patch skips clearing the quota flags on read-only mounts.

---
fs/xfs/xfs_qmops.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Index: linux-2.6/fs/xfs/xfs_qmops.c
===================================================================
--- linux-2.6.orig/fs/xfs/xfs_qmops.c
+++ linux-2.6/fs/xfs/xfs_qmops.c
@@ -134,7 +134,14 @@ static struct xfs_qmops xfs_qmcore_stub
int
xfs_qmops_get(struct xfs_mount *mp, struct xfs_mount_args *args)
{
- if (args->flags & (XFSMNT_UQUOTA | XFSMNT_PQUOTA | XFSMNT_GQUOTA)) {
+ bool c;
+
+ c = args->flags & (XFSMNT_UQUOTA | XFSMNT_GQUOTA | XFSMNT_PQUOTA);
+#ifdef CONFIG_XFS_QUOTA
+ c |= mp->m_flags & XFS_MOUNT_RDONLY;
+#endif
+
+ if (c) {
#ifdef CONFIG_XFS_QUOTA
mp->m_qm_ops = &xfs_qmcore_xfs;
#else


2007-12-18 14:39:18

by David Chinner

[permalink] [raw]
Subject: Re: Do not reset xfsquota flags on quotaless ro mount

On Tue, Dec 18, 2007 at 03:04:03PM +0100, Jan Engelhardt wrote:
> Hi,
>
>
> In https://bugzilla.novell.com/show_bug.cgi?id=345338 it is claimed that
> resetting the quota flags in the mounting sequence rw,ro,rw is a bug, but I

You mounted without quotas in the middle step, thereby invalidating
them.

> would say this is not the case, as quota is metadata, and the log is replayed
> in ro mode even for other filesystems. Yet, it is still not nice, and I have
> been trying with this patch, which does not do the right thing yet. (It does a
> recovery when mounting for the 3rd time, which probably just says that I did
> not know too much of xfs internals to cook up a nicely working patch.)
> Opinions?

Log recovery occurs on read only mounts. Log recovery on filesystems without
quota enabled ignores quota changes in the log (as quotas are not enabled
and hence we can't assume that there's dquots still on disk). Hence a
mount read only without quota enabled will leave quota inconsistent on disk.

i.e

> mount -o usrquota,grpquota /dev/mapper/myxfs /mnt
> umount /mnt
> mount -o ro /dev/mapper/myxfs /mnt

Quota is now potentially inconsistent, and hence:

> umount /mnt
> mount -o usrquota,grpquota /dev/mapper/myxfs /mnt

This mount *must* recalculate it. So, behaviour is as expected
given the above command sequence.

You should use this as the middle step:

mount -o ro,usrquota,grpquota /dev/mapper/myxfs /mnt

So that log replay replays all the journalled quota changes so
that the quota remains consistent with the rest of the filesystem.
Then you wont see a recalc when you remount rw.

Cheers,

Dave.
--
Dave Chinner
Principal Engineer
SGI Australian Software Group

2007-12-18 14:49:18

by Jan Engelhardt

[permalink] [raw]
Subject: Re: Do not reset xfsquota flags on quotaless ro mount


On Dec 19 2007 01:38, David Chinner wrote:
>>
>> In https://bugzilla.novell.com/show_bug.cgi?id=345338 it is
>> claimed that resetting the quota flags in the mounting sequence
>> rw,ro,rw is a bug, but I would say this is not the case,
>
>You mounted without quotas in the middle step, thereby invalidating
>them.
>
That's clear to me, but according to the commenter, the filesystem is
in read-only mode (that is, if no recovery was necessary for the ro
mount), hence they should not be invalidated as they cannot change
anyway. Or can they?