Date: Mon, 16 Oct 2006 18:05:34 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Chuck Lever <chucklever@gmail.com> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, mjacob@freebsd.org, cvs-all@freebsd.org, Kris Kennaway <kris@obsecurity.org> Subject: Re: cvs commit: src/sys/nfsclient nfs_vnops.c Message-ID: <20061016164122.S63585@delplex.bde.org> In-Reply-To: <76bd70e30610150837w61689cf6ya2499d100a15c3e8@mail.gmail.com> References: <200610140725.k9E7PC37008454@repoman.freebsd.org> <20061014231502.GA38708@rink.nu> <20061015105809.M59123@delplex.bde.org> <20061015051044.GA42764@xor.obsecurity.org> <20061014222221.H97880@ns1.feral.com> <20061014222437.N4701@ns1.feral.com> <20061015153454.G59979@delplex.bde.org> <76bd70e30610150837w61689cf6ya2499d100a15c3e8@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Chuck, On Sun, 15 Oct 2006, Chuck Lever wrote: > On 10/15/06, Bruce Evans <bde@zeta.org.au> wrote: >> My previous mail more or less explained why rc.conf began setting it to >> 2 in 1998: It didn't exist before then, so it was initially set to a >> conservative default of 2. Only the mount options for the _attribute_ >> cache existed before then. rc.conf and fstab never had any special >> support for these, so I think rc.conf shouldn't have any special support >> for the _access_ cache timeout (it now defaults to setting it to its >> kernel default value). > > I agree with this... other clients I'm familiar with don't tune the > access cache timeout separately from the attribute cache. Now that I understand these caches a bit better, I think independent timeouts for them are just a bug. At least for the FreeBSD implmentation of the v3 case, every cache miss for one of the caches causes a refresh of both caches. Thus the effective timeout is the minimum of the 2 timeouts except in cases that are hard to describe and harder to control. I'll try removing the special support for the access cache timeout in rc.conf first. >> I just got around to looking at some nfs RFCs. nfs4 has a lot to say >> about the problem of too many RPCs. nfs3 has less to say. > > What you're discussing here is known as "close-to-open cache > consistency" and is not discussed thoroughly in the NFS RFCs. A > better source for learning about expected behavior is Callaghan's "NFS > Illustrated." I should have said ``google hits for something like "open/close consistency nfsN RFC"'' have a lot more to say about the problem when N = 4 than when N = 3. >> I think I now know how to fix the second largest source of extra RPCs >> properly: >> >> We used clear the _attribute_ cache (and the access cache as a side >> effect?) at the _end_ of nfs_open(), but we are supposed to clear it >> at the _start_ of every open(). We can't do the latter properly because >> a fresh set of attributes are needed or at least preferred when >> nfs_lookup() is called before nfs_open(). We only ever clear the attribute cache directly, but in the v3 case we refresh both caches together so in many cases clearing only the attribute cache has the same effect on consistency as clearing both. > The fact that open is not an atomic operation in the VFS layer is > problematic. You have to consider how the VFS will recover in the > case where the client believes a file is there because it has just > been accessing it, but it was just removed by another client. It might be as atomic as possible due to exclusive access. > One reason for the "GETATTR on open" is to make sure the file being > opened is the same file as what may be cached. I think FreeBSD does > this by completely clearing the attribute cache on each open? Kind of > not efficient computationally, since that implies you are never using > your cache. Yes, now on every open, but still sometimes after nfs_lookup() has used unnecessarily stale attributes for its access check (see another reply). Only the attribute cache entry for one file is cleared on open() of course, and the cache still gets used for other operations until the next timeout or close(). What mohans@ fixed was usually clearing the attribute cache only at the end of nfs_open(). That allowed the cache to be refreshed soon by another operation and then the new entry was usually used by the next open(). He also changed nfs_close() to clear the attribute cache unconditionally. That corresponds to always clearing it at the end of nfs_open(). I think this only affects the next nfs_lookup(). It has the same problem as clearing at the end of nfs_open() (another operation may refresh the cache), but might have a smaller race window. It doesn't help nfs_lookup() at all for the case of 2 open()s with no close in between, while the old version does. I should have removed the clearing in nfs_close() instead of the clearing in nfs_open() to get a not so quick but less incorrect quick fix. Then nfs_lookup() would use stale attributes for its access check, but it gets this wrong anyway; its VOP_GETATTR() call would usually use stale attributes, but for open() nfs_open() would always force a refresh. >> Clearing the attribute cache >> at the end of nfs_open() seems to be just a hack which gives a good >> chance of the clearing living until the next open(). It doesn't always >> work. Clearing the cache in nfs_close() is further from always working. >> It fails if the file is re-open()ed before the first open() instance >> is close()d, and wasn't done. >> >> Now we clear the attribute cache at the start of nfs_open() and clear >> it in nfs_close(). For simple open-close sequences, this gives 2 >> clearings and 2 refreshes. First, nfs_lookup() refreshes; then >> nfs_open() clears and refreshes; finally, nfs_close() clears. > > Again, I'm not sure why it is necessary to clear the attribute cache, > when simply verifying that the file object remains the same is all > that is necessary. An RPC is required to tell if the file changed. FreeBSD just always uses a Getattr RPC (NFSV3ACCESS_ALL) for this in the v3 case. Getting all the attributes at once is supposed to be an optimization. > ... >> - force a clear in nfs_open() only if we aren't sure that nfs_lookup() >> didn't already do it. This is for safety in cases like core dumps >> where namei() isn't called by open(2) and we forget to tell namei() >> that the lookup is for open(). Then refresh normally. Implement this >> using a generation count or another flag? > > I think you also need to clear in the case where the file being opened > is different that what is in the client's cache (the case where > another client has replaced the file). How would we know what other clients are doing without another RPC? In nfs_open(), we normally have caches that have usually both just been refreshed by nfs_lookup(), and my proposed fix is to ensure that nfs_lookup() always does this refresh (if it is for open()). I think nfs_lookup() also needs to force a refresh (if it is for open()) for its own purposes. I don't know how nfs_lookup() handles cases where it was the file's directory that moved without the file's attributes changing. > You haven't spelled out how the access cache should behave in these > cases. Usually the access cache is treated identically to the > attribute cache. It should track the attrtibute cache in FreeBSD too. I think this happens except for the bug in nfs_lookup(). Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061016164122.S63585>