Merge of the 'write_inode' branch from the VFS tree

Merge of the 'write_inode' branch from the VFS tree

[Trond Myklebust]

Hi Al,

When you submitted the VFS changes for this merge window, I was hoping
you would include the 'write_inode' branch. I've been waiting for them
in order to push the NFS writeback improvements to Linus.

Would you be willing to either push the write_inode branch to Linus or
to Ack my doing so as part of the NFS push?

Cheers
  Trond

Re: Merge of the 'write_inode' branch from the VFS tree

[Al Viro]

On Fri, Mar 05, 2010 at 10:26:40AM -0500, Trond Myklebust wrote:
> Hi Al,
> 
> When you submitted the VFS changes for this merge window, I was hoping
> you would include the 'write_inode' branch. I've been waiting for them
> in order to push the NFS writeback improvements to Linus.
> 
> Would you be willing to either push the write_inode branch to Linus or
> to Ack my doing so as part of the NFS push?

Said branch has managed to grow conflicts with XFS commits already in
the mainline ;-/  With commits postdating the write_inode ones by a week
or so and having the same author.

I'm going to push the next VFS pile in about half an hour and get to the
write_inode situation.  I'm not sure what's the best course here.  Note
that since you've pulled it, you also have conflicts with what's in the
mainline.  I can do *another* backmerge (already had one due to gfs2 trivial
conflicts) and push the result.  Which will suck, since XFS conflicts
are not entirely trivial and we'll get a really ugly merge node, with
conflict resolution both hidden and not quite obvious.

Or I can do a new branch, put updated pair of patches there (hch has sent
the updated variants my way) and ask you to rebuild NFS tree.  Which will
also suck, since it adds PITA for you and you are completely innocent in
that clusterfuck.

Suggestions?  I'd love to get out of that mess with minimal PITA for
everyone involved and minimally messed tree...

One thing for sure - I'm not going to do that kind of "guaranteed to be
unchanged" shared branches again, TYVM.

[git pull] vfs part 3 (write_inode mess)

[Al Viro]

On Fri, Mar 05, 2010 at 03:48:23PM +0000, Al Viro wrote:
> I'm going to push the next VFS pile in about half an hour and get to the
> write_inode situation.  I'm not sure what's the best course here.  Note
> that since you've pulled it, you also have conflicts with what's in the
> mainline.  I can do *another* backmerge (already had one due to gfs2 trivial
> conflicts) and push the result.  Which will suck, since XFS conflicts
> are not entirely trivial and we'll get a really ugly merge node, with
> conflict resolution both hidden and not quite obvious.

OK, a backmerge it is.  Linus, could you please pull
git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ write_inode
or suggest a saner way to do that?

I've done backmerges of two points in mainline (trees that got merged
into mainline, actually) that created conflicts.  So at that point it's
(a) descendent of what's been pulled into NFS tree and (b) merges clean
with mainline.  All for two patches (at commit 716c28c..) ;-/

It's independent from the previous VFS pull; there's more stuff, hopefully
for later today, but the worst of the mess should be gone with that one.

Shortlog:
Al Viro (2):
      Merge commit 'e402746a945ceb9d0486a8e3d5917c9228fa4404' into write_inode
      Merge commit '07fec73625dc0db6f9aed68019918208a2ca53f5' into write_inode

Christoph Hellwig (2):
      make sure data is on disk before calling ->write_inode
      pass writeback_control to ->write_inode

Diffstat:
 fs/adfs/adfs.h               |    2 +-
 fs/adfs/inode.c              |    5 +++--
 fs/affs/affs.h               |    3 ++-
 fs/affs/inode.c              |    2 +-
 fs/afs/internal.h            |    1 -
 fs/afs/super.c               |    1 -
 fs/afs/write.c               |   21 ---------------------
 fs/bfs/inode.c               |    5 +++--
 fs/btrfs/ctree.h             |    2 +-
 fs/btrfs/inode.c             |    4 ++--
 fs/exofs/exofs.h             |    2 +-
 fs/exofs/inode.c             |    4 ++--
 fs/ext2/ext2.h               |    2 +-
 fs/ext2/inode.c              |   11 +++++++++--
 fs/ext3/inode.c              |    4 ++--
 fs/ext4/ext4.h               |    2 +-
 fs/ext4/inode.c              |    6 +++---
 fs/fat/inode.c               |    9 +++++++--
 fs/fs-writeback.c            |   22 +++++++++++++---------
 fs/gfs2/super.c              |    5 +++--
 fs/hfs/hfs_fs.h              |    2 +-
 fs/hfs/inode.c               |    2 +-
 fs/hfsplus/super.c           |    3 ++-
 fs/jfs/inode.c               |    5 ++++-
 fs/jfs/jfs_inode.h           |    2 +-
 fs/minix/inode.c             |    8 +++++---
 fs/nfs/inode.c               |   10 +++-------
 fs/nfs/internal.h            |    2 +-
 fs/ntfs/dir.c                |    2 +-
 fs/ntfs/file.c               |    2 +-
 fs/ntfs/inode.c              |    2 +-
 fs/ntfs/inode.h              |    4 ++--
 fs/ntfs/super.c              |    8 ++++++++
 fs/omfs/inode.c              |   10 ++++++++--
 fs/reiserfs/inode.c          |    4 ++--
 fs/sysv/inode.c              |   10 ++++++++--
 fs/sysv/sysv.h               |    2 +-
 fs/ubifs/dir.c               |    2 +-
 fs/ubifs/file.c              |    8 ++++----
 fs/ubifs/super.c             |    2 +-
 fs/udf/inode.c               |    4 ++--
 fs/udf/udfdecl.h             |    2 +-
 fs/ufs/inode.c               |    5 +++--
 fs/ufs/ufs.h                 |    2 +-
 fs/xfs/linux-2.6/xfs_super.c |    8 ++------
 include/linux/ext3_fs.h      |    2 +-
 include/linux/fs.h           |    2 +-
 include/linux/reiserfs_fs.h  |    2 +-
 48 files changed, 123 insertions(+), 107 deletions(-)

Re: Merge of the 'write_inode' branch from the VFS tree

[Trond Myklebust]

On Fri, 2010-03-05 at 15:48 +0000, Al Viro wrote:

> Or I can do a new branch, put updated pair of patches there (hch has sent
> the updated variants my way) and ask you to rebuild NFS tree.  Which will
> also suck, since it adds PITA for you and you are completely innocent in
> that clusterfuck.
> 
> Suggestions?  I'd love to get out of that mess with minimal PITA for
> everyone involved and minimally messed tree...

Hi Al,

I'd be fine with rebuilding the NFS tree. I have all the patches which
depend on write_inode in their own separate branch anyway, so I'd only
have to rebase that branch and then merge it with the main NFS client
tree...

Cheers
  Trond

Re: Merge of the 'write_inode' branch from the VFS tree

[Al Viro]

On Fri, Mar 05, 2010 at 01:02:35PM -0500, Trond Myklebust wrote:
> On Fri, 2010-03-05 at 15:48 +0000, Al Viro wrote:
> 
> > Or I can do a new branch, put updated pair of patches there (hch has sent
> > the updated variants my way) and ask you to rebuild NFS tree.  Which will
> > also suck, since it adds PITA for you and you are completely innocent in
> > that clusterfuck.
> > 
> > Suggestions?  I'd love to get out of that mess with minimal PITA for
> > everyone involved and minimally messed tree...
> 
> Hi Al,
> 
> I'd be fine with rebuilding the NFS tree. I have all the patches which
> depend on write_inode in their own separate branch anyway, so I'd only
> have to rebase that branch and then merge it with the main NFS client
> tree...

Ehh...  Just after I've sent a pull request for backmerge variant...
Anyway, I've put rebased variant in the same tree, branch called
write_inode2.  Same diffstat, same shortlog (sans merges).  Either
branch will do; write_inode2 obviously has cleaner history.

Re: [git pull] vfs part 3 (write_inode mess)

[Steve Dickson]

On 03/05/2010 12:40 PM, Al Viro wrote:
> On Fri, Mar 05, 2010 at 03:48:23PM +0000, Al Viro wrote:
>> I'm going to push the next VFS pile in about half an hour and get to the
>> write_inode situation.  I'm not sure what's the best course here.  Note
>> that since you've pulled it, you also have conflicts with what's in the
>> mainline.  I can do *another* backmerge (already had one due to gfs2 trivial
>> conflicts) and push the result.  Which will suck, since XFS conflicts
>> are not entirely trivial and we'll get a really ugly merge node, with
>> conflict resolution both hidden and not quite obvious.
> 
> OK, a backmerge it is.  Linus, could you please pull
> git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ write_inode
> or suggest a saner way to do that?
> 
> I've done backmerges of two points in mainline (trees that got merged
> into mainline, actually) that created conflicts.  So at that point it's
> (a) descendent of what's been pulled into NFS tree and (b) merges clean
> with mainline.  All for two patches (at commit 716c28c..) ;-/
> 
> It's independent from the previous VFS pull; there's more stuff, hopefully
> for later today, but the worst of the mess should be gone with that one.
Has there been any kind of testing that show this approach does indeed
improve performance? Any hardcore number? 

Just curious....

steved.

Re: [git pull] vfs part 3 (write_inode mess)

[Dave Chinner]

On Mon, Mar 08, 2010 at 03:22:37PM -0500, Steve Dickson wrote:
> On 03/05/2010 12:40 PM, Al Viro wrote:
> > On Fri, Mar 05, 2010 at 03:48:23PM +0000, Al Viro wrote:
> >> I'm going to push the next VFS pile in about half an hour and get to the
> >> write_inode situation.  I'm not sure what's the best course here.  Note
> >> that since you've pulled it, you also have conflicts with what's in the
> >> mainline.  I can do *another* backmerge (already had one due to gfs2 trivial
> >> conflicts) and push the result.  Which will suck, since XFS conflicts
> >> are not entirely trivial and we'll get a really ugly merge node, with
> >> conflict resolution both hidden and not quite obvious.
> > 
> > OK, a backmerge it is.  Linus, could you please pull
> > git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ write_inode
> > or suggest a saner way to do that?
> > 
> > I've done backmerges of two points in mainline (trees that got merged
> > into mainline, actually) that created conflicts.  So at that point it's
> > (a) descendent of what's been pulled into NFS tree and (b) merges clean
> > with mainline.  All for two patches (at commit 716c28c..) ;-/
> > 
> > It's independent from the previous VFS pull; there's more stuff, hopefully
> > for later today, but the worst of the mess should be gone with that one.
> Has there been any kind of testing that show this approach does indeed
> improve performance? Any hardcore number? 

http://oss.sgi.com/archives/xfs/2010-01/msg00556.html

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [git pull] vfs part 3 (write_inode mess)

[Trond Myklebust]

On Mon, 2010-03-08 at 15:22 -0500, Steve Dickson wrote: 
> On 03/05/2010 12:40 PM, Al Viro wrote:
> > On Fri, Mar 05, 2010 at 03:48:23PM +0000, Al Viro wrote:
> >> I'm going to push the next VFS pile in about half an hour and get to the
> >> write_inode situation.  I'm not sure what's the best course here.  Note
> >> that since you've pulled it, you also have conflicts with what's in the
> >> mainline.  I can do *another* backmerge (already had one due to gfs2 trivial
> >> conflicts) and push the result.  Which will suck, since XFS conflicts
> >> are not entirely trivial and we'll get a really ugly merge node, with
> >> conflict resolution both hidden and not quite obvious.
> > 
> > OK, a backmerge it is.  Linus, could you please pull
> > git.kernel.org/pub/scm/linux/kernel/git/viro/vfs-2.6.git/ write_inode
> > or suggest a saner way to do that?
> > 
> > I've done backmerges of two points in mainline (trees that got merged
> > into mainline, actually) that created conflicts.  So at that point it's
> > (a) descendent of what's been pulled into NFS tree and (b) merges clean
> > with mainline.  All for two patches (at commit 716c28c..) ;-/
> > 
> > It's independent from the previous VFS pull; there's more stuff, hopefully
> > for later today, but the worst of the mess should be gone with that one.
> Has there been any kind of testing that show this approach does indeed
> improve performance? Any hardcore number? 
> 
> Just curious....

The main improvement I'm seeing is in number of over the wire COMMIT
operations. With a standard 2.6.32/33 kernel without these changes, if I
do something like

   iozone -t 8 -s 512m -r 128k -i0 -i1

on my old 2GB test machines then I end up seeing 1 COMMIT going on the
wire for every 4 WRITE requests. IOW: I force the server to fsync for
every 4x256K I send it.

With the new code, I'm seeing 1 COMMIT being sent for every 50 WRITE
requests.

Writeback throughput is slightly, but not hugely improved on my test
rig. Furthermore, the maximum number of unstable writes recorded
in /proc/meminfo doesn't appear to change much. All this points to the
fact that most of those extra COMMIT calls were going out for just 1 or
2 writes, probably as a result of looping in balance_dirty_pages() while
the server was busy dealing with the first COMMIT.

It is definitely worth getting rid of that extra spam to the server,
though. Furthermore, I believe that others reported larger performance
improvements when the number of commits went down.

Cheers
  Trond