Date: Mon, 16 Oct 2006 09:20:26 -0400 From: "Chuck Lever" <chucklever@gmail.com> To: "Bruce Evans" <bde@zeta.org.au> 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: <76bd70e30610160620x67e5d3a5j938c26744d0b9759@mail.gmail.com> In-Reply-To: <20061016164122.S63585@delplex.bde.org> 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> <20061016164122.S63585@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 10/16/06, Bruce Evans <bde@zeta.org.au> wrote: > On Sun, 15 Oct 2006, Chuck Lever wrote: > > On 10/15/06, Bruce Evans <bde@zeta.org.au> wrote: > 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. OK. I can review patches if you think that would help, but I can't contribute code at the moment because of IP issues at my current employer. Hopefully that will change soon. > > 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. What I'm getting at here is that clearing the cache and verifying the cache are different things. Yes, both still require at least one RPC on the wire. But clearing then replacing the attribute information is actually fairly client-CPU-expensive compared to verifying, especially since it is done so often. Verification is simple: Compare the ctime in the cached attributes against the ctime in the returned attributes. If they match then your cached attributes are still valid. If you are being careful, then you will also ensure that the cached file ID matches the returned file ID (ie that the file object itself hasn't been replaced). Another thing to consider is that a LOOKUP is usually more expensive for servers than a GETATTR. If your client has already cached lookup results for the file to be opened, you can get away with a GETATTR on the parent directory to verify that it has not changed, and that will almost always be faster than doing a full LOOKUP. -- "We who cut mere stones must always be envisioning cathedrals" -- Quarry worker's creed
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?76bd70e30610160620x67e5d3a5j938c26744d0b9759>