On Mon, Dec 30, 2013 at 11:35:48AM +0100, J. Hannken-Illjes wrote: > The layered file systems hashlists currently have to work on locked > vnodes as the vnode operations returning vnodes return them locked. > > This leads to some very dirty hacks -- see layer_node_find() from > sys/miscfs/genfs/layer_subr.c for example. > > The best solution is to change these vnode operations to return > unlocked (and referenced) vnodes. > > The attached diff implements this change for operations create, mknod, > mkdir and symlink. It passes the atf test suite and some file system > stress tests I use here. > > Comments or objections anyone?
My first thought is that I really don't think it's a good idea to change important semantic properties (particularly locking) of calls from what's effectively a public API without changing the signatures too so old code won't compile. Also, since what you're doing is basically unlocking before return in every implementation, and then relocking after return at every call site, it's possible to do one op at a time; it seems like that would be a good idea in case unexpected problems crop up. Unlocking and relocking should not itself be a problem since AFAIK there isn't any reason for those locks to be held in the first place; however, I think I'd better go review all the call sites just in case. Plus I think it would be advisable to make this change first, so that in case we do find a site where it matters we can lock the returned object before anyone else can see it: # -#% mkdir dvp L U U +#% mkdir dvp L L L #% mkdir vpp - L - #% mkdir vpp - L - # This is not, unfortunately, entirely trivial; I've been putting it off to combine with namei/componentname-related changes. -- David A. Holland dholl...@netbsd.org