2000-11-21 13:15:27

by Jan Kara

[permalink] [raw]
Subject: Umount & quotas

Hello.

After rewrite of umount checks some time ago (just now reading your mail
I realized I never asked) filesystem doesn't umount when quotas are
turned on on it - it fails on check (atomic_read(&mnt->mnt_count) > 2)
in do_umount().
Is this intended behaviour? If so, we can remove later DQUOT_OFF() call
and maybe make somewhere a note about changed behaviour otherwise we should fix
it...
later failed.
Maybe we can ask quota (or get it from its structures) how many files it's
using and change the test to reflect this number... Do you agree with this?

Bye
Honza


2000-11-21 13:50:16

by Alexander Viro

[permalink] [raw]
Subject: Re: Umount & quotas



On Tue, 21 Nov 2000, Jan Kara wrote:

> Hello.
>
> After rewrite of umount checks some time ago (just now reading your mail
> I realized I never asked) filesystem doesn't umount when quotas are
> turned on on it - it fails on check (atomic_read(&mnt->mnt_count) > 2)
> in do_umount().
> Is this intended behaviour? If so, we can remove later DQUOT_OFF() call
> and maybe make somewhere a note about changed behaviour otherwise we should fix
> it...

Oh, fsck...
--- fs/super.c Thu Nov 2 22:38:59 2000
+++ fs/super.c.new Tue Nov 21 11:36:05 2000
@@ -1037,13 +1037,13 @@
}

spin_lock(&dcache_lock);
- if (atomic_read(&mnt->mnt_count) > 2) {
- spin_unlock(&dcache_lock);
- mntput(mnt);
- return -EBUSY;
- }

if (mnt->mnt_instances.next != mnt->mnt_instances.prev) {
+ if (atomic_read(&mnt->mnt_count) > 2) {
+ spin_unlock(&dcache_lock);
+ mntput(mnt);
+ return -EBUSY;
+ }
if (sb->s_type->fs_flags & FS_SINGLE)
put_filesystem(sb->s_type);
/* We hold two references, so mntput() is safe */

Not ideal, but not worse than the variant before the fs/super.c rewrite.
And yes, that's what was intended to be there. Said that, removing the
automagical DQUOT_OFF completely looks like a good idea. If there is no
serious reason to keep it in the kernel I would prefer to move it to
userland. We already have to do quota-related stuff if umount(2) fails...

2000-11-21 17:43:45

by Jan Kara

[permalink] [raw]
Subject: Re: Umount & quotas

Hello.

> --- fs/super.c Thu Nov 2 22:38:59 2000
> +++ fs/super.c.new Tue Nov 21 11:36:05 2000
> @@ -1037,13 +1037,13 @@
> }
>
> spin_lock(&dcache_lock);
> - if (atomic_read(&mnt->mnt_count) > 2) {
> - spin_unlock(&dcache_lock);
> - mntput(mnt);
> - return -EBUSY;
> - }
>
> if (mnt->mnt_instances.next != mnt->mnt_instances.prev) {
> + if (atomic_read(&mnt->mnt_count) > 2) {
> + spin_unlock(&dcache_lock);
> + mntput(mnt);
> + return -EBUSY;
> + }
> if (sb->s_type->fs_flags & FS_SINGLE)
> put_filesystem(sb->s_type);
> /* We hold two references, so mntput() is safe */
>
> Not ideal, but not worse than the variant before the fs/super.c rewrite.
> And yes, that's what was intended to be there. Said that, removing the
> automagical DQUOT_OFF completely looks like a good idea. If there is no
> serious reason to keep it in the kernel I would prefer to move it to
> userland. We already have to do quota-related stuff if umount(2) fails...
I think the only problem is that some people may rely on this behaviour.
There isn't probably any other reason as races with quotaoff before
someone does write are there even if we do quotaoff in kernel.
And these races aren't probably worth the work with fixing...
But I'm not sure whether we can just now stand up and say 'Umount doesn't
do quotaoff any more...'

Honza