From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH 7/8] dquot: move dquot initialization responsibiliy into the filesystem Date: Tue, 2 Mar 2010 19:44:38 +0100 Message-ID: <20100302184437.GG3829@quack.suse.cz> References: <20100220115101.469826792@bombadil.infradead.org> <20100220115213.803253745@bombadil.infradead.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: jack@suse.cz, linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from cantor2.suse.de ([195.135.220.15]:47869 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751751Ab0CBSo3 (ORCPT ); Tue, 2 Mar 2010 13:44:29 -0500 Content-Disposition: inline In-Reply-To: <20100220115213.803253745@bombadil.infradead.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 SUSE Labs, CR