Why advisory locking in fdrop_locked() not vn_close()?

[Robert Watson]

fdrop_locked() is responsible for dropping a reference on a locked file
descriptor, then optionally garbage collecting if it was the last
reference.  On the whole, it's really just a wrapper for fo_close(), which
invokes the close method on the file operation vector.  With one
exception: if the file descriptor points at a vnode, then it calls
VOP_ADVLOCK() on the vnode to release locks associated with the file
descriptor.  I'm looking at pushing down Giant into fo_close() so that
close() called on pipes and sockets doesn't grab Giant, but it's the
obvious obstacle.  In my local work branch, I simply switch on the f_type
to determine if I should grab Giant, but that's fairly undesirable.  I'd
like to push the VOP_ADVLOCK() call down into vn_close() and purge all VFS
knowledge from fdrop_locked(), and then push Giant acquisition down into
the fo_close() implementations (as needed).

This would require pushing a file descriptor reference into fo_close(),
which is not something the file descriptor API currently does for that
method.  It does, however, do this for several of the other file
descriptor methods, so I'm not sure this change would represent a layering
violation.  Given that the locking code is at the VFS layer but that locks
correspond to file descriptor level objects, it seems like some amount of
layering mess is obligated anyway.  I was wondering if anyone familiar
with this code knows if there was a specific reason VOP_ADVLOCK()  was
placed at the file descriptor level rather than the fo_close()
(vn_close()) level, other than the fact that the 'struct file *' simply
wasn't passed down in existing code?

Robert N M Watson             FreeBSD Core Team, TrustedBSD Projects
robert at fledge.watson.org      Senior Research Scientist, McAfee Research