On Mon, 25 Nov 2013 23:41:02 +0000 "Myklebust, Trond" <Trond.Myklebust@xxxxxxxxxx> wrote: > On Tue, 2013-11-26 at 10:23 +1100, NeilBrown wrote: > > On Mon, 25 Nov 2013 09:59:39 -0500 Chuck Lever <chuck.lever@xxxxxxxxxx> wrote: > > > > > Hi Neil- > > > > > > On Nov 24, 2013, at 11:59 PM, NeilBrown <neilb@xxxxxxx> wrote: > > > > > > > > > > > Hi Trond, > > > > I just noticed commit acdc53b2146c7ee67feb1f02f7bc3020126514b8 from 2010 > > > > reverts the effect commit 28c494c5c8d425e15b7b82571e4df6d6bc34594d from Chunk > > > > in 2007. > > > > > > I'm wondering if a subsequent commit changed filemap_write_and_wait(). > > > > I hadn't thought of that possibility. I've just had a look at the > > differences between acdc53b2146c7ee6 and now and cannot find anything that > > could be related. > > To clarify a little: my understanding is that the current 2-pass code in > write_cache_pages() is supposed to prevent livelock. Instead of chasing > PAGECACHE_TAG_DIRTY tags (which are constantly being set if an > application is actively writing), we call tag_pages_for_writeback() once > in order to convert the current set of PAGECACHE_TAG_DIRTY tags into > PAGECACHE_TAG_TOWRITE tags, and then we have a second pass write those > pages back (and wait for completion). > IOW: the inode->i_mutex should be unnecessary here... Thanks for that pointer. You are right, write_cache_pages shouldn't loop indefinitely any more. And I also just noticed commit 72cb77f4a5ace37b12dcb47a0e8637a2c28ad881 and NFS_INO_FLUSHING which is an extra reason that the i_mutex isn't needed. Don't know how I missed that last night. Less haste, more speed I guess. So it looks like I jumped the gun and there must be some other explanation. Sorry 'bout that. Thanks, NeilBrown > > > Now that said, we recently added in the call to nfs_inode_dio_wait(). If > applications are using O_DIRECT, then _that_ could livelock. There is > nothing currently preventing the applications from continuing to bump > the inode->i_dio_count while we're waiting. Christoph has proposed some > locking changes that should fix that problem. I'm still evaluating his > patchset... > > Cheers > Trond > > Cheers > Trond
Attachment:
signature.asc
Description: PGP signature
