LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] quota: Turn quotas off when remounting read-only
@ 2008-02-07 14:37 Jan Kara
  2008-02-07 18:36 ` Andrew Morton
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2008-02-07 14:37 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, Michael Tokarev

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) {

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] quota: Turn quotas off when remounting read-only
  2008-02-07 14:37 [PATCH] quota: Turn quotas off when remounting read-only Jan Kara
@ 2008-02-07 18:36 ` Andrew Morton
  2008-02-07 19:27   ` Michael Tokarev
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2008-02-07 18:36 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-kernel, Michael Tokarev, stable

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?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] quota: Turn quotas off when remounting read-only
  2008-02-07 18:36 ` Andrew Morton
@ 2008-02-07 19:27   ` Michael Tokarev
  2008-02-11 12:39     ` Jan Kara
  0 siblings, 1 reply; 6+ messages in thread
From: Michael Tokarev @ 2008-02-07 19:27 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Jan Kara, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] quota: Turn quotas off when remounting read-only
  2008-02-07 19:27   ` Michael Tokarev
@ 2008-02-11 12:39     ` Jan Kara
  2008-02-15 14:10       ` Jan Engelhardt
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kara @ 2008-02-11 12:39 UTC (permalink / raw)
  To: Michael Tokarev; +Cc: Andrew Morton, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] quota: Turn quotas off when remounting read-only
  2008-02-11 12:39     ` Jan Kara
@ 2008-02-15 14:10       ` Jan Engelhardt
  2008-02-15 14:21         ` Michael Tokarev
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Engelhardt @ 2008-02-15 14:10 UTC (permalink / raw)
  To: Jan Kara; +Cc: Michael Tokarev, Andrew Morton, linux-kernel, stable


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?


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] quota: Turn quotas off when remounting read-only
  2008-02-15 14:10       ` Jan Engelhardt
@ 2008-02-15 14:21         ` Michael Tokarev
  0 siblings, 0 replies; 6+ messages in thread
From: Michael Tokarev @ 2008-02-15 14:21 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Jan Kara, Andrew Morton, linux-kernel, stable

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-02-15 14:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-02-07 14:37 [PATCH] quota: Turn quotas off when remounting read-only Jan Kara
2008-02-07 18:36 ` Andrew Morton
2008-02-07 19:27   ` Michael Tokarev
2008-02-11 12:39     ` Jan Kara
2008-02-15 14:10       ` Jan Engelhardt
2008-02-15 14:21         ` Michael Tokarev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).