Re: linux 2.4.18-5 nfs/rpc problem

[Trond Myklebust <trond.myklebust@fys.uio.no>]

>>>>> " " == David Jeffery <david_jeffery@adaptec.com> writes:

     > Eduard, I have also seen this problem with both Red Hat and
     > stock kernels.  I've got some patches That I'm testing to try
     > and fix the problem for me, but it takes a while for my setup
     > to reproduce the problem.  How easy is it for you to trigger?

     > I've attached two patches for 2.4.22 that I'm currently
     > testing.  The first (nfsiput.patch) is to fix the deadlocked
     > processes you see by moving the iput() call to happen after the
     > waited on request is unlocked.

I take it you're assuming that the problem is a deadlock due to
clear_inode() hanging on our page lock? That does indeed appear to be
a possible source of contention.

Instead of your 2 patches, though, how about just ensuring that
anybody calling nfs_wait_on_requests() already has a reference to the
inode?

There is indeed one case where this is not being done: in
page_launder() you only have a lock on the page. This means that you
may end up calling nfs_wait_on_request() in order to flush out a page
only to discover that it called iput() from beneath you and is now
waiting in clear_inode() on your page lock.

Cheers,
  Trond

--- linux-2.4.23-pre5/fs/nfs/write.c.orig	2003-07-09 14:11:04.000000000 -0400
+++ linux-2.4.23-pre5/fs/nfs/write.c	2003-10-10 14:54:16.000000000 -0400
@@ -225,8 +225,19 @@
 	struct inode *inode = page->mapping->host;
 	unsigned long end_index;
 	unsigned offset = PAGE_CACHE_SIZE;
+	int inode_referenced = 0;
 	int err;
 
+	/*
+	 * Note: We need to ensure that we have a reference to the inode
+	 *       if we are to do asynchronous writes. If not, waiting
+	 *       in nfs_wait_on_request() may deadlock with clear_inode().
+	 *
+	 *       If igrab() fails here, then it is in any case safe to
+	 *       call nfs_wb_page(), since there will be no pending writes.
+	 */
+	if (igrab(inode) != 0)
+		inode_referenced = 1;
 	end_index = inode->i_size >> PAGE_CACHE_SHIFT;
 
 	/* Ensure we've flushed out any previous writes */
@@ -244,7 +255,8 @@
 		goto out;
 do_it:
 	lock_kernel();
-	if (NFS_SERVER(inode)->wsize >= PAGE_CACHE_SIZE && !IS_SYNC(inode)) {
+	if (NFS_SERVER(inode)->wsize >= PAGE_CACHE_SIZE && !IS_SYNC(inode) &&
+			inode_referenced) {
 		err = nfs_writepage_async(NULL, inode, page, 0, offset);
 		if (err >= 0)
 			err = 0;
@@ -256,7 +268,9 @@
 	unlock_kernel();
 out:
 	UnlockPage(page);
-	return err; 
+	if (inode_referenced)
+		iput(inode);
+	return err;
 }
 
 /*


-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
SourceForge.net hosts over 70,000 Open Source Projects.
See the people who have HELPED US provide better services:
Click here: http://sourceforge.net/supporters.php
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs