Re: [PATCH 7/8] dquot: move dquot initialization responsibiliy into the filesystem

[Jan Kara <jack@suse.cz>]

  Hi Christoph,

On Sat 20-02-10 06:51:08, Christoph Hellwig wrote:
> Currently various places in the VFS call vfs_dq_init directly.  This means
> we tie the quota code into the VFS.  Get rid of that and make the
> filesystem responsibly for the initialization.   For most metadata operations
> this is a straight forward move into the methods, but for truncate and
> open it's a bit more complicated.
> 
> For truncate we currently only call vfs_dq_init for the sys_truncate case
> because open already takes care of it for ftruncate and open(O_TRUNC) - the
> new code causes an additional vfs_dq_init for those which is harmless.
> 
> For open the initialization is moved from do_filp_open into the open method,
> which means it happens slightly earlier now, and only for regular files.
> The latter is fine because we don't need to initialize it for operations
> on special files, and we already do it as part of the namespace operations
> for directories.
> 
> Add a dquot_file_open helper that filesystems that support generic quotas
> can use to fill in ->open.
  I've looked at the patch. I have several comments:
It seems that conversion of UDF is completely missing from the patch.
Also it seems that UFS is missing conversion of ufs_setattr to call
vfs_dq_init when ATTR_SIZE is set. Finally, ocfs2 is missing conversion
of ocfs2_open so that it calls vfs_dq_init.
  Also I have two minor suggeestions for improvement below...

> Index: linux-2.6/fs/reiserfs/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/reiserfs/inode.c	2010-02-20 11:59:10.802007224 +0100
> +++ linux-2.6/fs/reiserfs/inode.c	2010-02-20 12:00:45.554256073 +0100
> @@ -3073,6 +3076,8 @@ int reiserfs_setattr(struct dentry *dent
>  	ia_valid = attr->ia_valid &= ~(ATTR_KILL_SUID|ATTR_KILL_SGID);
>  
>  	depth = reiserfs_write_lock_once(inode->i_sb);
> +	if (ia_valid & ATTR_SIZE)
> +		vfs_dq_init(inode);
>  	if (attr->ia_valid & ATTR_SIZE) {
>  		/* version 2 items will be caught by the s_maxbytes check
>  		 ** done for us in vmtruncate
  We can merge the two conditions above.

> Index: linux-2.6/fs/ocfs2/file.c
> ===================================================================
> --- linux-2.6.orig/fs/ocfs2/file.c	2010-02-20 11:55:53.836005270 +0100
> +++ linux-2.6/fs/ocfs2/file.c	2010-02-20 12:00:45.648255722 +0100
> @@ -976,6 +979,9 @@ int ocfs2_setattr(struct dentry *dentry,
>  	if (status)
>  		return status;
>  
> +	if (attr->ia_valid & ATTR_SIZE)
> +		vfs_dq_init(inode);
> +
>  	size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
>  	if (size_change) {
>  		status = ocfs2_rw_lock(inode, 1);
  We can merge the above two conditions...

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