On 04/02/22(Fri) 03:39, Klemens Nanni wrote: > [...] > ... with the lock grabbed in uvm_map_teardown() that is, otherwise > the first call path can lock against itself (regress/misc/posixtestsuite > is a reproduce for this): > > vm_map_lock_read_ln+0x38 > uvm_fault_unwire+0x58 > uvm_unmap_kill_entry_withlock+0x68 > uvm_unmap_remove+0x2d4 > sys_munmap+0x11c > > which is obvious in hindsight. > > So grabbing the lock in uvm_map_teardown() instead avoids that while > still ensuring a locked map in the path missing a lock.
This should be fine since this function should only be called when the last reference of a map is dropped. In other words the locking here is necessary to satisfy the assertions. I wonder if lock shouldn't be taken & released around uvm_map_teardown() which makes it easier to see that this is called after the last refcount decrement. > Index: uvm_map.c > =================================================================== > RCS file: /cvs/src/sys/uvm/uvm_map.c,v > retrieving revision 1.282 > diff -u -p -r1.282 uvm_map.c > --- uvm_map.c 21 Dec 2021 22:21:32 -0000 1.282 > +++ uvm_map.c 4 Feb 2022 02:51:00 -0000 > @@ -2734,6 +2751,7 @@ uvm_map_teardown(struct vm_map *map) > KERNEL_ASSERT_UNLOCKED(); > > KASSERT((map->flags & VM_MAP_INTRSAFE) == 0); > + vm_map_lock(map); > > /* Remove address selectors. */ > uvm_addr_destroy(map->uaddr_exe); >
