> On 1. Jul 2021, at 21:04, David Holland <dholland-current%netbsd.org@localhost> 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 - hannken%eis.cs.tu-bs.de@localhost - TU Braunschweig
Attachment:
signature.asc
Description: Message signed with OpenPGP