On 31/01/22(Mon) 10:24, Klemens Nanni wrote:
> Running with my uvm assert diff showed that uvm_fault_unwire_locked()
> was called without any locks held.
>
> This happened when I rebooted my machine:
>
> uvm_fault_unwire_locked()
> uvm_unmap_kill_entry_withlock()
> uvm_unmap_kill_entry()
> uvm_map_teardown()
> uvmspace_free()
>
> This code does not corrupt anything because
> uvm_unmap_kill_entry_withlock() is grabbing the kernel lock aorund its
> uvm_fault_unwire_locked() call.
>
> But regardless of the kernel lock dances in this code path, the uvm map
> ought to be protected by its own lock. uvm_fault_unwire() does that.
>
> uvm_fault_unwire_locked()'s comment says the map must at least be read
> locked, which is what all other code paths to that function do.
>
> This makes my latest assert diff happy in the reboot case (it did not
> always hit that assert).
I'm happy your asserts found a first bug.
I"m afraid calling this function below could result in a grabbing the
lock twice. Can we be sure this doesn't happen?
uvm_unmap_kill_entry() is called in many different contexts and this is
currently a mess. I don't know what NetBSD did in this area but it is
worth looking at and see if there isn't a good idea to untangle this.
> 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 31 Jan 2022 09:28:04 -0000
> @@ -2132,7 +2143,7 @@ uvm_unmap_kill_entry_withlock(struct vm_
> if (VM_MAPENT_ISWIRED(entry)) {
> KERNEL_LOCK();
> entry->wired_count = 0;
> - uvm_fault_unwire_locked(map, entry->start, entry->end);
> + uvm_fault_unwire(map, entry->start, entry->end);
> KERNEL_UNLOCK();
> }
>
>
