On Sat, Dec 24, 2005 at 03:28:06PM +0900, YAMAMOTO Takashi wrote: > > One thing I noted in this change is that vget() will now call > > VOP_INACTIVE() on vnodes that were never activated. Will this cause issues > > with other file systems? > > what kind of issues? Well, I'm not 100% sure. Thus the question. The old code went out of its way to not call VOP_INACTIVE() on the errored vnode. Also, it made sure that we did the right thing in face of multiple processes each in vget() and failing. > afaik, vn_lock fails only when the vnode is being reclaimed. > in that case, calling vrele doesn't hurt. Yes. My concern, however, was to make sure that we don't end up disturbing the vnode in its new life (for the case where we reclaimed the vnode for getnewvnode()). Like call VOP_INACTIVE() on a live vnode. I've been staring at this for a bit and trying to think of race conditions that would do this. I think that you're right, and we're ok. Since we have spinlocks around reference count updating and we never play with TAILQ macros unless we were in the transition between zero and one (either 0->1 or 1->0), I think we're fine. I'm still confused as I don't understand the comment. Since we hold references across these operations, we will not mess up queue membership nor the reference counting. So vrele() will never be a problem; either we really are the last reference release on the new life (when VOP_INACTIVE() would be correct) or we aren't. Also, if the other user just vclean()ed the vnode, the VOP_INACTIVE() call will be handled by deadfs, which will be fine. I guess the comment dated to an earlier implementation. Take care, Bill
Attachment:
pgp6_FsVzl7dh.pgp
Description: PGP signature