lkml.org 
[lkml]   [1998]   [Dec]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] 2.1.131, NFS locking/cache coherency fixes
Linus,

Recently I provided a patch to fix some problems with NFS cache
coherency and locking, which stopped mail from being lost amongst other
things. In 2.1.131 you included a much smaller solution, and the main
problem is now fixed. (I've tested it).

Your fix got me wondering if it was correct w.r.t. race conditions (as I
had written much more paranoid code). This led me to through the twisty
maze of NFS client code, net/sunrpc and eventually to the unbalanced
lock_kernel() in rpciod(). A moment of enlightenment later. I feel I
have advanced one more step down into the great cellar of kernel
interlocking strategies. I see that the races I saw before were
illusions.

But there are still problems with 2.1.131. Unless you're going to
correct me again. See the enclosed patch. The changes are (in order):

nfs/file.c

1. When a process exits, don't leave a stale lock on the file if we
happen to get a signal -- we're exiting, we're not going to handle
the signal, no one else will clean up the lock.

This cleans up in case of accidents only; it doesn't affect
applications behaving normally.

2. Important. Revalidate cached data immediately, while we still
have the kernel lock. Otherwise async replies can mean we don't
flush the data when we should.

nfs/inode.c

3. Optimisation. Eliminate a second redundant attribute request
round trip after each cache invalidation.

4. Maintainability fix. No difference in effect at the moment,
but play safe. I think it's clearer what's intended this way.

Alan, if Linus isn't around yet could you apply this in your tree?
I think this makes the difference between safe NFS locking & unsafe NFS
locking (specifically point 2).

Enjoy,
-- Jamie

--- linux/./fs/nfs/file.c.nfslock Sun Dec 6 21:22:19 1998
+++ linux/./fs/nfs/file.c Sat Dec 12 04:19:16 1998
@@ -245,7 +245,7 @@
* with locks..
*/
status = nfs_wb_all(inode);
- if (status < 0)
+ if (status < 0 && !(current->flags & PF_EXITING))
return status;

if ((status = nlmclnt_proc(inode, cmd, fl)) < 0)
@@ -254,7 +254,13 @@
/*
* Make sure we re-validate anything we've got cached.
* This makes locking act as a cache coherency point.
+ *
+ * For coherency, must revalidate while we still have the
+ * kernel lock, as a later async reply can call
+ * nfs_refresh_inode() with info from before the lock.
*/
NFS_CACHEINV(inode);
+ _nfs_revalidate_inode(NFS_SERVER(inode), filp->f_dentry);
+
return 0;
}
--- linux/./fs/nfs/inode.c.nfslock Sun Dec 6 21:22:22 1998
+++ linux/./fs/nfs/inode.c Fri Dec 11 10:36:47 1998
@@ -706,6 +706,7 @@
NFS_ATTRTIMEO(inode) = NFS_MAXATTRTIMEO(inode);
}
NFS_OLDMTIME(inode) = fattr.mtime.seconds;
+ NFS_READTIME(inode) = jiffies;
dfprintk(PAGECACHE, "NFS: %s/%s revalidation complete\n",
dentry->d_parent->d_name.name, dentry->d_name.name);
out:
@@ -790,13 +791,13 @@
inode->i_atime = fattr->atime.seconds;
inode->i_mtime = fattr->mtime.seconds;
inode->i_ctime = fattr->ctime.seconds;
+ error = 0;
+ if (invalid)
+ goto out_invalid;
/*
* Update the read time so we don't revalidate too often.
*/
NFS_READTIME(inode) = jiffies;
- error = 0;
- if (invalid)
- goto out_invalid;
out:
return error;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:46    [W:1.689 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site