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:
> > On Wed, Sep 23, 2009 at 12:27:42AM +0100, Al Viro wrote:
> > > On Tue, Sep 22, 2009 at 10:51:38PM +0100, Al Viro wrote:
> > > > It is...  I really don't like some parts of that:
> > > > 	* failing on foo_truncate() is all nice and proper, but
> > > > what to do about the quota transfer that has already happened?
> > > > Note that e.g. ext2_setattr() simply doesn't change uid/gid in
> > > > case of ext2_setsize() failure.  It does transfer quota, though.
> > > > And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
> > > > set.
> > > > 	* way, _way_ too much boilerplate around the aforementioned
> > > > quota transfers.
> > > > 	* if we don't do quota handling for minix and sysv (fair enough,
> > > > no quota support implemented for those), why the devil are we doing it
> > > > in simple_setattr()?
> > > > 
> > > > Anyway, #new-truncate in the usual VFS tree contains all "new truncate
> > > > scheme" stuff I've collected from the lists so far.  Additions are welcome...
> > > 
> > > BTW, why are we messing with truncation in ->write_end() e.g. for ext2?
> > 
> > BTW^2, what's going to unmap the stale buffers in the page if
> > ext2_write_begin() runs into an error?  ext2_truncate() (and thus vmtruncate()
> > that used to be called from block_write_begin()) used to do that, but
> > ext2_truncate_blocks() won't...
> > 
> > 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?

Seems probably reasonable... I've been at a conference and travelling
last week (sitting in an airport now), so apologies for intermittent
communication.

I'll take a look at it when I get home.