From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tyler Hicks Subject: Re: [patch 00/11] new truncate sequence Date: Mon, 7 Dec 2009 16:46:46 -0600 Message-ID: <20091207224646.GB10148@fedora-virt> References: <20090820163504.131529718@suse.de> <20090922150444.GA14566@ZenIV.linux.org.uk> <20090922200042.GA27015@lst.de> <20090922215138.GH14381@ZenIV.linux.org.uk> <20090922232742.GI14381@ZenIV.linux.org.uk> <20090922235845.GJ14381@ZenIV.linux.org.uk> <20090923022921.GK14381@ZenIV.linux.org.uk> <20091207124902.GB4394@wotan.suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Al Viro , Jan Kara , Christoph Hellwig , Andrew Morton , linux-fsdevel@vger.kernel.org, ecryptfs-devel@lists.launchpad.net, kirkland@canonical.com To: Nick Piggin Return-path: Received: from e1.ny.us.ibm.com ([32.97.182.141]:53614 "EHLO e1.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934457AbZLGWqm (ORCPT ); Mon, 7 Dec 2009 17:46:42 -0500 Received: from d01relay07.pok.ibm.com (d01relay07.pok.ibm.com [9.56.227.147]) by e1.ny.us.ibm.com (8.14.3/8.13.1) with ESMTP id nB7MiOK4015123 for ; Mon, 7 Dec 2009 17:44:24 -0500 Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay07.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id nB7MkmDX1732624 for ; Mon, 7 Dec 2009 17:46:48 -0500 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.3/8.13.1/NCO v10.0 AVout) with ESMTP id nB7Mkl4v012223 for ; Mon, 7 Dec 2009 17:46:48 -0500 Content-Disposition: inline In-Reply-To: <20091207124902.GB4394@wotan.suse.de> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon Dec 07, 2009 at 01:49:02PM +0100, Nick Piggin (npiggin@suse.de) was quoted: > On Wed, Sep 23, 2009 at 03:29:21AM +0100, Al Viro wrote: > > On Wed, Sep 23, 2009 at 12:58:45AM +0100, Al Viro wrote: > > > Frankly, I'm almost 100% convinced to postpone new-truncate merge until > > > .33-rc1; the first couple of patches (vmtruncate() unification and cleanups) > > > can go right now, but the rest obviously hadn't been beaten up enough > > > to seriously consider it for .32-rc1. > > > > FWIW, I think that a reasonable battle plan for that series would look so: > > * current first two patches > > * vmtruncate()-less analogs of block_write_begin(), nobh_write_begin() > > and blockdev_direct_IO(); original 3 functions themselves become wrappers. > > * default_setattr() > > * sort out the use of vmtruncate() in ecryptfs. > > * at that point work on individual filesystems becomes independent; > > we can convert them one by one at leisure, killing vmtruncate() uses in > > each as we go. Basically, take an fs, shift vmtruncate() calls into the > > methods (we will have full set of needed helpers) and lambda-expand each > > (remember that vmtruncate() becomes a straightforward short sequence of helper > > calls after the first stage). And replace that ->truncate() call with > > explicit foofs_truncate(), switching .truncate to NULL while we are at it. > > All equivalent transformations, all independent. Massage foo_setattr() > > as we wish. > > * once we are done, remove vmtruncate() and ->truncate() - no more > > callers for them. Ditto for leftover 3 wrappers. > > > > AFAICS, that gives a bisectable series with no "new_truncate" flags, with > > no flagday interface changes at all and mergable just fine piecewise. > > > > Comments? > > I'm having a look at this again, sorry for the delay. I would still > like to merge it in 2.6.33 if we can; at least the core parts so that > we can push fs specific patches to their various maintainers. > > Your plan for avoiding .truncate_kludge works, although I still like > that flag so we can put BUG_ONs around the place.... but it is ugly > so maybe we just skip it. > > It looks relatively simple, and I'm also updating the series to account > for the writev data loss bug. > > Ecryptfs. To start with, it isn't taking i_mutex on the lower inode when > calling vmtruncate so locking is wrong. Secondly, I don't think it is > allowed to assume vmtruncate does the right thing. The only correct > generic entry point to the lower filesystem AFAIKS is notify_change... > which would allow it to work fine with the new truncate sequence. So > any reason why they can't do that? Nope. I've rewritten the eCryptfs truncate path to do just that. I plan on getting the patch into 2.6.33-rc1. http://git.kernel.org/?p=linux/kernel/git/ecryptfs/ecryptfs-2.6.git;a=commit;h=8137e6500c0b6d627656aa4d9f48dd9349c06051