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

[Kostik Belousov]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20101028/17476bdf/attachment.pgp