[PATCH] quota: Turn quotas off when remounting read-only

[PATCH] quota: Turn quotas off when remounting read-only

[Jan Kara]

Turn off quotas before filesystem is remounted read only. Otherwise quota will
try to write to read-only filesystem which does no good... We could also just
refuse to remount ro when quota is enabled but turning quota off is consistent
with what we do on umount.

Signed-off-by: Jan Kara <jack@suse.cz>
---
Andrew, this should fix the hang reported... Please apply. Thanks.

diff --git a/fs/super.c b/fs/super.c
index ceaf2e3..945c322 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -603,6 +603,7 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
 			mark_files_ro(sb);
 		else if (!fs_may_remount_ro(sb))
 			return -EBUSY;
+		DQUOT_OFF(sb);
 	}
 
 	if (sb->s_op->remount_fs) {

Re: [PATCH] quota: Turn quotas off when remounting read-only

[Andrew Morton]

On Thu, 7 Feb 2008 15:37:21 +0100 Jan Kara <jack@suse.cz> wrote:

> Turn off quotas before filesystem is remounted read only. Otherwise quota will
> try to write to read-only filesystem which does no good... We could also just
> refuse to remount ro when quota is enabled but turning quota off is consistent
> with what we do on umount.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> Andrew, this should fix the hang reported... Please apply. Thanks.
> 
> diff --git a/fs/super.c b/fs/super.c
> index ceaf2e3..945c322 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -603,6 +603,7 @@ int do_remount_sb(struct super_block *sb, int flags, void *data, int force)
>  			mark_files_ro(sb);
>  		else if (!fs_may_remount_ro(sb))
>  			return -EBUSY;
> +		DQUOT_OFF(sb);
>  	}
>  
>  	if (sb->s_op->remount_fs) {

Cool.  And this is applicable to 2.6.23, 2.6.22 and even earlier, isn't it?

Re: [PATCH] quota: Turn quotas off when remounting read-only

[Michael Tokarev]

Andrew Morton wrote:
> On Thu, 7 Feb 2008 15:37:21 +0100 Jan Kara <jack@suse.cz> wrote:
> 
>> Turn off quotas before filesystem is remounted read only. Otherwise quota will
>> try to write to read-only filesystem which does no good... We could also just
>> refuse to remount ro when quota is enabled but turning quota off is consistent
>> with what we do on umount.

[a nice one-liner snipped]

> Cool.  And this is applicable to 2.6.23, 2.6.22 and even earlier, isn't it?

Provided the amount of time this issue exists, I don't think it's worth
to push it to -stable.  It's an oooooooold, issue, which happens quite
rarely, and no one bothered to report it so far...  But it's not my
call... ;)

But...  I'm thinking about this scenario:

 # mount /data
 # quotaon /data
 (some maintenance stuff to be planned)
 # mount -o remount,ro /data
 (do backup etc)
 # mount -r remount,rw /data

at this point, it's expected that quota on /data is enabled.
After this patch, it's not anymore...

I think it's more usual scenario than mine (umount instead of
remount-rw).  And this change will break it.  So I'm not sure
what really to do here.  Probably refusing remount-ro if quota
is on is better...  it's annoying for sure, but at least it's
explicit, and avoids the handg too.

/mjt

Re: [PATCH] quota: Turn quotas off when remounting read-only

[Jan Kara]

On Thu 07-02-08 22:27:27, Michael Tokarev wrote:
> Andrew Morton wrote:
> > On Thu, 7 Feb 2008 15:37:21 +0100 Jan Kara <jack@suse.cz> wrote:
> > 
> >> Turn off quotas before filesystem is remounted read only. Otherwise quota will
> >> try to write to read-only filesystem which does no good... We could also just
> >> refuse to remount ro when quota is enabled but turning quota off is consistent
> >> with what we do on umount.
> 
> [a nice one-liner snipped]
> 
> > Cool.  And this is applicable to 2.6.23, 2.6.22 and even earlier, isn't it?
> 
> Provided the amount of time this issue exists, I don't think it's worth
> to push it to -stable.  It's an oooooooold, issue, which happens quite
> rarely, and no one bothered to report it so far...  But it's not my
> call... ;)
> 
> But...  I'm thinking about this scenario:
> 
>  # mount /data
>  # quotaon /data
>  (some maintenance stuff to be planned)
>  # mount -o remount,ro /data
>  (do backup etc)
>  # mount -r remount,rw /data
> 
> at this point, it's expected that quota on /data is enabled.
> After this patch, it's not anymore...
  Yes, it previously accidentally worked this way (for an year or so,
before that we refused to remount read-only). Hmm, but maybe we could
somehow tweak quotas to be turned on when remounting read-write again.
We have all the information we need at the time of remounting read-only
so we could store it and use it later when remounting read-write. I'll have
a look into that.

> I think it's more usual scenario than mine (umount instead of
> remount-rw).  And this change will break it.  So I'm not sure
> what really to do here.  Probably refusing remount-ro if quota
> is on is better...  it's annoying for sure, but at least it's
> explicit, and avoids the handg too.
  On the other hand when you umount the filesystem, quotas are
automatically turned off and when you mount it again, they are not enabled.
So the behavior as I've changed it is consistent as well... But probably
what I wrote above is the safest solution.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] quota: Turn quotas off when remounting read-only

[Jan Engelhardt]

On Feb 11 2008 13:39, Jan Kara wrote:
>> 
>> But...  I'm thinking about this scenario:
>> 
>>  # mount /data
>>  # quotaon /data
>>  (some maintenance stuff to be planned)
>>  # mount -o remount,ro /data
>>  (do backup etc)
>>  # mount -r remount,rw /data
>> 
>> at this point, it's expected that quota on /data is enabled.
>> After this patch, it's not anymore...
>
>  Yes, it previously accidentally worked this way (for an year or so,
>before that we refused to remount read-only). Hmm, but maybe we could
>somehow tweak quotas to be turned on when remounting read-write again.
>We have all the information we need at the time of remounting read-only
>so we could store it and use it later when remounting read-write. I'll have
>a look into that.

Maybe it is possible to leave quota on all times, so that the
reporting quota ioctls continue to work even in ro mode?

Re: [PATCH] quota: Turn quotas off when remounting read-only

[Michael Tokarev]

Jan Engelhardt wrote:
> On Feb 11 2008 13:39, Jan Kara wrote:
>>> But...  I'm thinking about this scenario:
>>>
>>>  # mount /data
>>>  # quotaon /data
>>>  (some maintenance stuff to be planned)
>>>  # mount -o remount,ro /data
>>>  (do backup etc)
>>>  # mount -r remount,rw /data
>>>
>>> at this point, it's expected that quota on /data is enabled.
>>> After this patch, it's not anymore...
>>  Yes, it previously accidentally worked this way (for an year or so,
>> before that we refused to remount read-only). Hmm, but maybe we could
>> somehow tweak quotas to be turned on when remounting read-write again.
>> We have all the information we need at the time of remounting read-only
>> so we could store it and use it later when remounting read-write. I'll have
>> a look into that.
> 
> Maybe it is possible to leave quota on all times, so that the
> reporting quota ioctls continue to work even in ro mode?

Well, that'd be the best approach imho (plus check if all
ioctls which try to modify quotas fails with EROFS as
appropriate).

But the problem really is that it's unknown at this time
where it fails in the first place.  I can't reproduce my
hang "on demand" (mount-ro followed with umount when quotas
are turned on, with ext3fs - umount never finishes), yet
it has biten me for several times already.  So it must be
something rare, some small race maybe, which is difficult
to find...  Yet it finds itself at the most inappropriate
moment. ;)  I already learned to turn the quota off before
doing something with a filesystem, but sometimes I'm
forgetting this, and the result is always the same... ;)
Oh well.

/mjt