lkml.org 
[lkml]   [1996]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 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


    \
     
     \ /
      Last update: 2005-03-22 13:38    [W:0.019 / U:246.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site