Last time, I started off with some locking fixups for the OpenAFS client on FreeBSD, but left off in the middle. We had fixed the cosmetic errors coming from afs_FlushVCache, but not the underlying problem. We recall that the cosmetic issues arose due to incorrect locking around accesses to the v_usecount field of the vnode; it turns out that there are only a small number of places where this happens. Most of them are uninteresting, just a quick check and not much else. But in osi_VM_FlushVCache (which is FreeBSD-specific code), we check the use count once, and then again later on in the function, and then we call vgone(). vgone() is a heavyweight function call, marking a vnode as being free for reuse. As such, it requires some pretty heavy locking around calls to it — in particular, it requires an exclusive lock on the vnode. (vgone also acquires the vnode interlock internally.) However, this is not quite sufficient, as once vgone places the vnode on the free list, it is susceptible to being destroyed (the FreeBSD VFS layer runs a cleaner periodically). But we still have the vnode locked! We need to unlock it after vgone returns, and need some mechanism to ensure that it doesn’t go away in the meantime. This is done by placing a “hold” on the vnode, or increasing its “hold count”. This is a closely related idea to the use count (and in fact the usual way to increment the use count also increments the hold count), but needs to be tracked separately for implementation details like this. So, then, we put a hold on the vnode before sending it away (keeping the interlock held for efficiency), then do the unlock, and then drop the hold. This procedure is sufficiently internal to the VFS layer so as to not be documented in a man page; I learned it because a FreeBSD VFS expert directed me to the vlrureclaim function in sys/kern/vfs_subr.c. It is not quite the same, as it is iterating over a list of vnodes, but it does go and free up vnodes that are not currently being used, so the checks and locks it takes are a good example for my use case.
The extra bonus of going and implementing a proper set of instructions around vgone is that it allowed the removal of some duplicated work! In addition to osi_VM_FlushVCache, the osi_TryEvictVCache function was doing some checks and then calling vgone. Well, more properly, it was calling vgonel, which is not supposed to be an exported symbol but just happened to work due to an implementation detail of the kernel linker! It turns out that the checks it was doing are exactly the same ones done in osi_VM_FlushVCache, so the latter can be implemented in terms of the former, removing a goodly chunk of code. (It actually wants to be implemented in terms of afs_FlushVCache, which does some additional bookkeeping on the number of vcaches in use, but I didn’t realize this until after the code was committed; I ran into it while tracking down another issue.) This change allowed my (multi-threaded) testing to go far enough to expose a rare race condition elsewhere in the codebase, which we’ll cover next time.