Re: [PATCH 1/6] dquot: move remount handling into the filesystem

[Jan Kara <jack@suse.cz>]

On Wed 12-05-10 15:44:09, Christoph Hellwig wrote:
> Currently do_remount_sb calls into the dquot code to tell it about going
> from rw to ro and ro to rw.  Move this code into the filesystem to
> not depend on the dquot code in the VFS - note ocfs2 already ignores
> these calls and handles remount by itself.  This gets rif of overloading
> the quotactl calls and allows to unify the VFS and XFS codepathes in
> that area later.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
  It seems you have missed out UDF from the conversion (maybe you could
setup a checklist of filesystem to convert? :) OTOH looking at it more in
detail quota support for UDF is broken (i.e. quotaon returns EINVAL because
.quota_write is not set) for several years now and noone has complained so
I'm starting to wonder whether fixing it is worth the effort.
  Also I'm slightly concerned that previous vfs_dq_quota_on_remount was
called only after
  sb->s_flags = (sb->s_flags & ~MS_RMT_MASK) | (flags & MS_RMT_MASK);
so in particular MS_RDONLY has been cleared. Now it is called before so
some filesystem could possibly barf when it sees writes from quota system
before MS_RDONLY gets cleared. I've checked and only JFS could have this
problem since others already clear MS_RDONLY inside their foo_remount()
functions but still...

> Index: linux-2.6/fs/ext2/super.c
> ===================================================================
> --- linux-2.6.orig/fs/ext2/super.c	2010-05-10 22:42:00.615256048 +0200
> +++ linux-2.6/fs/ext2/super.c	2010-05-10 22:42:19.030253953 +0200
> @@ -1191,6 +1191,7 @@ static int ext2_remount (struct super_bl
>  	unsigned long old_mount_opt = sbi->s_mount_opt;
>  	struct ext2_mount_options old_opts;
>  	unsigned long old_sb_flags;
> +	int enable_quota = 0;
>  	int err;
>  
>  	spin_lock(&sbi->s_lock);
> @@ -1241,6 +1242,7 @@ static int ext2_remount (struct super_bl
>  			spin_unlock(&sbi->s_lock);
>  			return 0;
>  		}
> +
>  		/*
>  		 * OK, we are remounting a valid rw partition rdonly, so set
>  		 * the rdonly flag and then mark the partition as valid again.
> @@ -1248,6 +1250,14 @@ static int ext2_remount (struct super_bl
>  		es->s_state = cpu_to_le16(sbi->s_mount_state);
>  		es->s_mtime = cpu_to_le32(get_seconds());
>  		spin_unlock(&sbi->s_lock);
> +
> +		err = vfs_dq_off(sb, 1);
> +		if (err < 0 && err != -ENOSYS) {
> +			err = -EBUSY;
> +			spin_lock(&sbi->s_lock);
> +			goto restore_opts;
> +		}
> +
>  		ext2_sync_super(sb, es, 1);
>  	} else {
>  		__le32 ret = EXT2_HAS_RO_COMPAT_FEATURE(sb,
> @@ -1269,8 +1279,13 @@ static int ext2_remount (struct super_bl
>  		if (!ext2_setup_super (sb, es, 0))
>  			sb->s_flags &= ~MS_RDONLY;
>  		spin_unlock(&sbi->s_lock);
> +
>  		ext2_write_super(sb);
> +		enable_quota = 1;
>  	}
> +
> +	if (enable_quota)
> +		vfs_dq_quota_on_remount(sb);
  I kind of miss the purpose of "enable_quota" in the above...
Also the ENOSYS check was there only for filesystems which do not support
quotas. Since all the filesystems that call vfs_dq_off now obviously do
support quotas, you can just drop it.

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