← Back to team overview

ecryptfs-devel team mailing list archive

Re: [patch 00/11] new truncate sequence

 

On Mon Dec 07, 2009 at 01:49:02PM +0100, Nick Piggin (npiggin@xxxxxxx) 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