Messages in this thread |  | | Date | Mon, 15 Jul 1996 08:37:28 +0300 (EET DST) | From | Linus Torvalds <> | Subject | Re: NFS caching patch. |
| |
Ok, this is _exactly_ why I didn't check the mtime in the first place, and just to explain WHY this patch is bad bad bad, I'll walk you through it.
On Sun, 14 Jul 1996, A.N.Kuznetsov wrote: > diff -ur linux/fs/nfs/file.c linux-obj/fs/nfs/file.c > --- linux/fs/nfs/file.c Sun Jul 14 21:03:44 1996 > +++ linux-obj/fs/nfs/file.c Sun Jul 14 21:11:36 1996 > @@ -147,8 +147,23 @@ > if (pos > inode->i_size) > inode->i_size = pos; > /* Avoid possible Solaris 2.5 nfsd bug */ > - if (inode->i_ino == fattr.fileid) > + if (inode->i_ino == fattr.fileid) { > + > + /* > + * It is hack, that cannot be avoided for NFSv2, > + * because it has no "weak cache consistency" mechanism. > + * We are deemed to believe, that the file > + * was not changed since previous getattr (or > + * another operation returning gratuitous attributes). > + * Another solution would be to call getattr before > + * each write :-) --ANK > + * (see also nfs_notify_change() in inode.c) > + */ > + inode->i_mtime = fattr.mtime.seconds; > + NFS_MTIME_USECS(inode) = fattr.mtime.useconds; > + > nfs_refresh_inode(inode, &fattr); > + } > return written; > } >
The above "hack" is the obvious solution, but it's WRONG.
Think of when we write to the file, and have it cached. When the write succeeds, we now update the "mtime" of the local cached data from the resulting file attributes, and _really_ really bad things happen if somebody else has happened to write to the file just before we wrote to it.
In short, the above will update "mtime" to a new time _without_ guaranteeing that no changes have taken place in the file between the two dates. Your logic is that "WE changed the file, thus we don't need to invalidate", but the case may be that "WE AND SOMEBODY ELSE changed the file".
Now, the above patch will result in us not necessarily flushing the cache _ever_ even if it's full of data that is stale. That's a lot worse than the current behaviour, where we don't flush the cache immediately, but we do flush it eventually when it times out. The current behaviour is at least correct (according to NFS specs you can cache read data for some time), the patch introduces a bug (and your comment about calling "getattr()" before each write is equally broken - you _cannot_ make the race window go away, and it's fundamental to the whole NFSv2 design).
Linus
|  |