On Wed, Oct 27, 2010 at 11:57:31PM -0400, Benjamin Kaduk wrote:
> On Wed, 27 Oct 2010, Kostik Belousov wrote:
>
> >On Wed, Oct 27, 2010 at 10:59:56AM -0400, Benjamin Kaduk wrote:
> >>[1] The old (racy) function is osi_TryEvictVCache, here:
> >>http://git.openafs.org/?p=openafs.git;a=blob;f=src/afs/FBSD/osi_vcache.c;h=c2060c74f0155a610d2ea94f3c7f508e8ca4373a;hb=HEAD
> >The function looks very strange for much more serious reasons. Why do you
> >try to manage the vnode revocation in the filesystem module at all ?
>
> I am still becoming familiar with the AFS code, but I think this is
> largely due to a difference in the vfs structure that AFS has been using
> and the FreeBSD standard. E.g. vop_inactive/vop_reclaim do not actually
> free filesystem-specific resources, instead keeping a free list of
> "vcache" entries. So, the original authors of this AFS code were
> approaching the problem in a somewhat different way.
> Therefore, are somewhat-orthogonal pools of vcaches and vnodes (with some
> intersection). If the vcaches are all in use in use, there is a routine
> which tries to "shake some loose"; if it can free up vcaches, their
> associated vnodes also need to be cleaned up in some fashion. It may be
> that no additional code is actually needed to do this, though -- I am not
> sure.
I think the best would be to move the code to vop_reclaim(), possibly calling
vgone() from vop_inactivate().
>
> >VFS will (assumedly in a right way) revoke and destroy non-referenced
> >vnodes when needed.
> >
>
> You are, in essence, suggesting that the body of the conditional should
> be just "return 1;"? (In my incomplete testing the vref() call was always
> made.)
See above.
>
> >Anyway, you need to hold vnode lock before checking for the vnode
> >refcounter.
> >See the vfs_subr.c:vlrureclaim() for the correct dance with
> >vholdl()/vn_lock()
> >sequence that does the revocation I mentioned in the race-free way.
>
> Unless I am mistaken, the path you refer to still uses vgonel(), which is
> not available to me. Is it safe to drop and reaquire the interlock
> between the vgone and vdrop?
vgonel() use is an optimization to avoid dropping/reacquiring the vnode
interlock. Correct code can call vdrop() after vgone() call, without
doing anything with interlock. Please note that vnode lock should be
dropped between vgone and vdrop.
pgpdFocOuh12h.pgp
Description: PGP signature
