> On 1. Jul 2021, at 21:04, David Holland <[email protected]> wrote: > > On Thu, Jul 01, 2021 at 07:54:33PM +0200, J. Hannken-Illjes wrote: >> lookup_fastforward -> lookup_parsepath -> VOP_PARSEPATH -> ... -> >> fstrans_start > > Bleh. I had a feeling we were going to end up regretting that > fastforward code. :-| > >> According to vnode_if.src VOP_PARSEPATH(dvp...) should take a locked vnode >> but here this lock is missing. So either >> >> - make sure the vnode is locked so fstrans_start will no loner block. >> >> or >> >> - add FSTRANS=NO to vop_parsepath, file kern/vnode_if.src and allow unlocked >> vnodes: >> >> vop_parsepath { >> + FSTRANS=NO >> IN struct vnode *dvp; >> >> David? > > I thought the vnode was locked readonly in the fastforward path. Did I > misread? Or is that not good enough?
Nope, the fastforward path takes namecache locks only. > Anyway, I think it's probably ok to change vop_parsepath to not > require locked vnodes at all. The only parsepath operation that does > anything other than string ops is rumpfs's, and it takes etfs_lock to > look in some tables that etfs_lock covers. Unless that's going to > interact badly with fstrans without the vnode lock covering it (seems > unlikely, but IDK) there shouldn't be a problem. This is ok, the vnode is referenced and comparing it to rootvnode is ok. > However, except in the fastforward code the vnode will be locked. So I > think it should be "= = =" in vnode_if.src. If you also need to add > FSTRANS=NO, that should be fine too. Setting "= = =" is ok, but it is only a comment. You also need "FSTRANS=NO" to prevent VOP_PARSEPATH() to take fstrans and deadlock. -- J. Hannken-Illjes - [email protected] - TU Braunschweig
signature.asc
Description: Message signed with OpenPGP
