[PATCH] Fix 'implicit declaration' warning and update vgone(9)

[Benjamin Kaduk]

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.

> 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.)

> 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?


Many thanks,

Ben