On 07 Apr 2014, at 18:38, Taylor R Campbell <campbell+netbsd-tech-k...@mumble.net> wrote:
> Date: Mon, 7 Apr 2014 18:32:02 +0200 > From: "J. Hannken-Illjes" <hann...@eis.cs.tu-bs.de> > > On 07 Apr 2014, at 18:02, Taylor R Campbell > <campbell+netbsd-tech-k...@mumble.net> wrote: > >> In that case, could you set the VI_CHANGING bit in vp, rather than >> node->vn_vnode = NULL in the vcache node, to indicate that the vnode >> is being initialized in vcache_intern? > > This will not work for layered file systems. They will share the > v_interlock from another vnode (in VFS_LOAD_NODE) so we better don't use > the interlock before loading the fs node. > > You can pass the shared interlock to vcache_intern, like I illustrated > in the code I attached. Sorry, I missed the new argument to vcache_intern. But there is still a window between getnewvnode() and setting VI_CHANGING where another thread may succeed with vget() so for this to work either getnewvnode() returns with interlock held or it has to set VI_CHANGING and all callers of getnewvnode have to be changed. >> Perhaps we ought at least to change VFS_VGET so it returns the vnode >> unlocked, and once we do that, use it consistently to look up vnodes >> by inode number rather than calling vcache_intern in multiple >> different places. > > Maybe in the far future. Already looked at it and it is a very big job > if we want to do it in one step. > > We could do this for just ufs: > > 1. Add ufs_vget_unlocked to make the sole call to vcache_intern for > ufs. > > 2. Make ufs_vget call ufs_vget_unlocked and then lock the result. > > 3. Replace calls to VFS_VGET by ufs_vget_unlocked, rather than by > vcache_intern. > > Then, when we fix VFS_VGET, we replace ufs_vget by ufs_vget_unlocked. Ok -- tomorrow ... -- J. Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)