Steve,
On Sat, Dec 19, 2009 at 10:25:47PM +0800, Steve Rago wrote:
>
> On Sat, 2009-12-19 at 20:20 +0800, Wu Fengguang wrote:
> >
> > On Thu, Dec 17, 2009 at 04:17:57PM +0800, Peter Zijlstra wrote:
> > > On Wed, 2009-12-16 at 21:03 -0500, Steve Rago wrote:
> > > > Eager Writeback for NFS Clients
> > > > -------------------------------
> > > > Prevent applications that write large sequential streams of data (like backup, for example)
> > > > from entering into a memory pressure state, which degrades performance by falling back to
> > > > synchronous operations (both synchronous writes and additional commits).
> >
> > What exactly is the "memory pressure state" condition? What's the
> > code to do the "synchronous writes and additional commits" and maybe
> > how they are triggered?
>
> Memory pressure occurs when most of the client pages have been dirtied
> by an application (think backup server writing multi-gigabyte files that
> exceed the size of main memory). The system works harder to be able to
> free dirty pages so that they can be reused. For a local file system,
> this means writing the pages to disk. For NFS, however, the writes
> leave the pages in an "unstable" state until the server responds to a
> commit request. Generally speaking, commit processing is far more
> expensive than write processing on the server; both are done with the
> inode locked, but since the commit takes so long, all writes are
> blocked, which stalls the pipeline.
Let me try reiterate the problem with code, please correct me if I'm
wrong.
1) normal fs sets I_DIRTY_DATASYNC when extending i_size, however NFS
will set the flag for any pages written -- why this trick? To
guarantee the call of nfs_commit_inode()? Which unfortunately turns
almost every server side NFS write into sync writes..
writeback_single_inode:
do_writepages
nfs_writepages
nfs_writepage ----[short time later]---> nfs_writeback_release*
nfs_mark_request_commit
__mark_inode_dirty(I_DIRTY_DATASYNC);
if (I_DIRTY_SYNC || I_DIRTY_DATASYNC) <---- so this will be true for most time
write_inode
nfs_write_inode
nfs_commit_inode
2) NFS commit stops pipeline because it sleep&wait inside i_mutex,
which blocks all other NFSDs trying to write/writeback the inode.
nfsd_sync:
take i_mutex
filemap_fdatawrite
filemap_fdatawait
drop i_mutex
If filemap_fdatawait() can be moved out of i_mutex (or just remove
the lock), we solve the root problem:
nfsd_sync:
[take i_mutex]
filemap_fdatawrite => can also be blocked, but less a problem
[drop i_mutex]
filemap_fdatawait
Maybe it's a dumb question, but what's the purpose of i_mutex here?
For correctness or to prevent livelock? I can imagine some livelock
problem here (current implementation can easily wait for extra
pages), however not too hard to fix.
The proposed patch essentially takes two actions in nfs_file_write()
- to start writeback when the per-file nr_dirty goes high
without committing
- to throttle dirtying when the per-file nr_writeback goes high
I guess this effectively prevents pdflush from kicking in with
its bad committing behavior
In general it's reasonable to keep NFS per-file nr_dirty low, however
questionable to do per-file nr_writeback throttling. This does not
work well with the global limits - eg. when there are many dirty
files, the summed-up nr_writeback will still grow out of control.
And it's more likely to impact user visible responsiveness than
a global limit. But my opinion can be biased -- me have a patch to
do global NFS nr_writeback limit ;)
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
