From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch 3/5] fs: introduce new truncate sequence Date: Tue, 18 Aug 2009 11:19:38 +0200 Message-ID: <20090818091938.GN9962@wotan.suse.de> References: <20090816102533.329473921@suse.de> <20090816102856.527647594@suse.de> <20090816193929.GA22219@infradead.org> <20090817164144.GJ9962@wotan.suse.de> <20090817180612.GA9144@lst.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christoph Hellwig , Andrew Morton , linux-fsdevel@vger.kernel.org To: Christoph Hellwig Return-path: Received: from cantor2.suse.de ([195.135.220.15]:57014 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752941AbZHRJTi (ORCPT ); Tue, 18 Aug 2009 05:19:38 -0400 Content-Disposition: inline In-Reply-To: <20090817180612.GA9144@lst.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon, Aug 17, 2009 at 08:06:12PM +0200, Christoph Hellwig wrote: > On Mon, Aug 17, 2009 at 06:41:44PM +0200, Nick Piggin wrote: > > Just wonder what you think of this for an updated patch. Main changes > > since last time are changelog changed, more comments, and addition of > > simple_setattr and inode_set_attributes. I was going to leave > > simple_setattr for later, but now I found when converting the simple > > filesystem ramfs that we basically need it anyway. > > > > inode_set_attributes is also following your suggestion and if we use > > it then we don't have to do that masking away ATTR_SIZE then calling > > inode_setattr that you didn't like (and I agree with). > > I like this a lot. But please change the inode_set_attributes name, > it's awkward and totally falls out of the scheme. It defintively should > be _setattr. Not sure about what to use for the . > Maybe just generic_setattr, mirroring generic_getattr? Yeah I agree the name wasn't so good. I guess generic_setattr is better yes. I will call it that and we can feel free to change it if there is a better idea before it is merged into Linus. > Btw, one idea on how to avoid having to touch all the > begin_write/end_write/direct_IO instances: What about passing another > callback to them, in addition to the get_blocks also a trim_blocks > which we call to trim blocks over i_size. That would be the old > ->truncate minus the block_truncate_page. I guess it doesn't fit into non-block filesystems, but it could just be `cleanup` or something. But on the other hand, I think it is a bit filesystem specific (eg. what to do in case of error) and it isn't much problem to add them to write_begin/end functions. But this is a good question if any fs developers have a strong preference one way or the other. I think I will just leave it as is for now because I have done a few conversions already, but we can always change it later.