On Mon, 2011-10-10 at 16:33 -0400, Jeff Layton wrote: > The current test in nfs_commit_unstable_pages does not handle the edge > condition properly. When ncommit == 0, then that likely means that the > kernel doesn't need to do anything more for the inode. The current test > though will return true in that case, and the inode will end up being > marked dirty. > > Fix this by not re-marking the inode dirty when ncommit == 0. > > Mike noticed this problem initially in RHEL5 (2.6.18-based kernel) which > has a backported version of 420e3646. The inode cache there was > growing very large, but calling sync() would essentially "fix" the > problem. > > What I'm not clear on is how big a problem this is in mainline kernels > as the writeback code there is very different. I think this basically > means that the inode will remain dirty until there's a WB_SYNC_ALL > flush. Either way, it seems incorrect to mark the inode dirty in this > case. > > Also, another question I have here -- shouldn't we be holding the i_lock > when doing this check? > > Reported-by: Mike McLean <mikem@xxxxxxxxxx> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> > --- > fs/nfs/write.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > index 17e20e4..6395e98 100644 > --- a/fs/nfs/write.c > +++ b/fs/nfs/write.c > @@ -1557,7 +1557,7 @@ static int nfs_commit_unstable_pages(struct inode *inode, struct writeback_contr > /* Don't commit yet if this is a non-blocking flush and there > * are a lot of outstanding writes for this mapping. > */ > - if (nfsi->ncommit <= (nfsi->npages >> 1)) > + if (nfsi->ncommit && nfsi->ncommit <= (nfsi->npages >> 1)) > goto out_mark_dirty; > > /* don't wait for the COMMIT response */ Umm... The above implementation will actually cause you to call nfs_commit_inode() when nfsi->ncommit is zero, which again causes you to go through all the hoopla with nfs_commit_set_lock() etc., for a case when you know you could just exit... Cheers Trond -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
