Re: [patch 00/11] new truncate sequence

[Nick Piggin <npiggin@suse.de>]

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?