Re: Towards unlocking mmap(2) & munmap(2)
On Fri, Oct 28, 2022 at 11:08:55AM +0200, Martin Pieuchot wrote: > On 20/10/22(Thu) 16:17, Martin Pieuchot wrote: > > On 11/09/22(Sun) 12:26, Martin Pieuchot wrote: > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > mmap(2) for anons and munmap(2). > > > > > > Please test it with WITNESS enabled and report back. > > > > New version of the diff that includes a lock/unlock dance in > > uvm_map_teardown(). While grabbing this lock should not be strictly > > necessary because no other reference to the map should exist when the > > reaper is holding it, it helps make progress with asserts. Grabbing > > the lock is easy and it can also save us a lot of time if there is any > > reference counting bugs (like we've discovered w/ vnode and swapping). > > Here's an updated version that adds a lock/unlock dance in > uvm_map_deallocate() to satisfy the assert in uvm_unmap_remove(). > Thanks to tb@ from pointing this out. > > I received many positive feedback and test reports, I'm now asking for > oks. In principal, this is what I did a few months back, with less asserts and less "Must be called with map [un]locked" comment additions for functions that gained assertions. I went through the tree to manually verify that each function you added an assert to is called with[out] the map lock held: this looks good. GENERIC.MP+uvm (your diff) with mmap(2) as and munmap(2) unlocked has been working fine on my daily x230 driver with browsing, compiling, etc. i386 kernels build fine using GENERIC.MP+uvm and i386 regress runs fine using GENERIC.MP+uvm+WITNESS. sparc64 kernels build fine using GENERIC.MP+uvm and sparc64 regress runs fine using GENERIC.MP+uvm (WITNESS doesn't boot). Please keep an eye on syzkaller after this gets committed. Last time I committed something like this syzcaller was broken. This was reported off-list. OK kn
Re: Towards unlocking mmap(2) & munmap(2)
On Sun, Oct 30, 2022 at 01:58:40PM +0100, Martin Pieuchot wrote: > On 30/10/22(Sun) 12:45, Klemens Nanni wrote: > > On Sun, Oct 30, 2022 at 12:40:02PM +, Klemens Nanni wrote: > > > regress on i386/GENERIC.MP+WITNESS with this diff shows > > > > Another one; This machine has three read-only NFS mounts, but none of > > them are used during builds or regress. > > It's the same. See archives of bugs@ for discussion about this lock > order reversal and a potential fix from visa@. Great, thanks!
Re: Towards unlocking mmap(2) & munmap(2)
On 30/10/22(Sun) 12:45, Klemens Nanni wrote: > On Sun, Oct 30, 2022 at 12:40:02PM +, Klemens Nanni wrote: > > regress on i386/GENERIC.MP+WITNESS with this diff shows > > Another one; This machine has three read-only NFS mounts, but none of > them are used during builds or regress. It's the same. See archives of bugs@ for discussion about this lock order reversal and a potential fix from visa@. > > This one is most certainly from the NFS regress tests themselves: > 127.0.0.1:/mnt/regress-nfs-server 3548 2088 1284 > 62%/mnt/regress-nfs-client > > witness: lock order reversal: > 1st 0xd6381eb8 vmmaplk (&map->lock) > 2nd 0xf5c98d24 nfsnode (&np->n_lock) > lock order data w2 -> w1 missing > lock order "&map->lock"(rwlock) -> "&np->n_lock"(rrwlock) first seen at: > #0 rw_enter+0x57 > #1 rrw_enter+0x3d > #2 nfs_lock+0x27 > #3 VOP_LOCK+0x50 > #4 vn_lock+0x91 > #5 vn_rdwr+0x64 > #6 vndstrategy+0x2bd > #7 physio+0x18f > #8 vndwrite+0x1a > #9 spec_write+0x74 > #10 VOP_WRITE+0x3f > #11 vn_write+0xde > #12 dofilewritev+0xbb > #13 sys_pwrite+0x55 > #14 syscall+0x2ec > #15 Xsyscall_untramp+0xa9 >
Re: Towards unlocking mmap(2) & munmap(2)
On 30/10/22(Sun) 12:40, Klemens Nanni wrote: > On Fri, Oct 28, 2022 at 11:08:55AM +0200, Martin Pieuchot wrote: > > On 20/10/22(Thu) 16:17, Martin Pieuchot wrote: > > > On 11/09/22(Sun) 12:26, Martin Pieuchot wrote: > > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > > mmap(2) for anons and munmap(2). > > > > > > > > Please test it with WITNESS enabled and report back. > > > > > > New version of the diff that includes a lock/unlock dance in > > > uvm_map_teardown(). While grabbing this lock should not be strictly > > > necessary because no other reference to the map should exist when the > > > reaper is holding it, it helps make progress with asserts. Grabbing > > > the lock is easy and it can also save us a lot of time if there is any > > > reference counting bugs (like we've discovered w/ vnode and swapping). > > > > Here's an updated version that adds a lock/unlock dance in > > uvm_map_deallocate() to satisfy the assert in uvm_unmap_remove(). > > Thanks to tb@ from pointing this out. > > > > I received many positive feedback and test reports, I'm now asking for > > oks. > > regress on i386/GENERIC.MP+WITNESS with this diff shows This isn't related to this diff.
Re: Towards unlocking mmap(2) & munmap(2)
On Sun, Oct 30, 2022 at 12:40:02PM +, Klemens Nanni wrote: > regress on i386/GENERIC.MP+WITNESS with this diff shows Another one; This machine has three read-only NFS mounts, but none of them are used during builds or regress. This one is most certainly from the NFS regress tests themselves: 127.0.0.1:/mnt/regress-nfs-server 3548 2088 1284 62%/mnt/regress-nfs-client witness: lock order reversal: 1st 0xd6381eb8 vmmaplk (&map->lock) 2nd 0xf5c98d24 nfsnode (&np->n_lock) lock order data w2 -> w1 missing lock order "&map->lock"(rwlock) -> "&np->n_lock"(rrwlock) first seen at: #0 rw_enter+0x57 #1 rrw_enter+0x3d #2 nfs_lock+0x27 #3 VOP_LOCK+0x50 #4 vn_lock+0x91 #5 vn_rdwr+0x64 #6 vndstrategy+0x2bd #7 physio+0x18f #8 vndwrite+0x1a #9 spec_write+0x74 #10 VOP_WRITE+0x3f #11 vn_write+0xde #12 dofilewritev+0xbb #13 sys_pwrite+0x55 #14 syscall+0x2ec #15 Xsyscall_untramp+0xa9
Re: Towards unlocking mmap(2) & munmap(2)
On Fri, Oct 28, 2022 at 11:08:55AM +0200, Martin Pieuchot wrote: > On 20/10/22(Thu) 16:17, Martin Pieuchot wrote: > > On 11/09/22(Sun) 12:26, Martin Pieuchot wrote: > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > mmap(2) for anons and munmap(2). > > > > > > Please test it with WITNESS enabled and report back. > > > > New version of the diff that includes a lock/unlock dance in > > uvm_map_teardown(). While grabbing this lock should not be strictly > > necessary because no other reference to the map should exist when the > > reaper is holding it, it helps make progress with asserts. Grabbing > > the lock is easy and it can also save us a lot of time if there is any > > reference counting bugs (like we've discovered w/ vnode and swapping). > > Here's an updated version that adds a lock/unlock dance in > uvm_map_deallocate() to satisfy the assert in uvm_unmap_remove(). > Thanks to tb@ from pointing this out. > > I received many positive feedback and test reports, I'm now asking for > oks. regress on i386/GENERIC.MP+WITNESS with this diff shows witness: lock order reversal: 1st 0xd6381aa8 vmmaplk (&map->lock) 2nd 0xd76a9790 inode (&ip->i_lock) lock order "&ip->i_lock"(rrwlock) -> "&map->lock"(rwlock) first seen at: #0 rw_enter_read+0x32 #1 vm_map_lock_read_ln+0x15 #2 uvmfault_lookup+0x60 #3 uvm_fault_check+0x14 #4 uvm_fault+0xe4 #5 kpageflttrap+0xe5 #6 trap+0x260 #7 calltrap+0xc #8 copyout+0x42 #9 uiomove+0x135 #10 ffs_read+0x27b #11 VOP_READ+0x3c #12 vn_rdwr+0x85 #13 vmcmd_map_readvn+0x7e #14 exec_process_vmcmds+0x5e #15 sys_execve+0x69e #16 start_init+0x241 #17 proc_trampoline+0x12 lock order "&map->lock"(rwlock) -> "&ip->i_lock"(rrwlock) first seen at: #0 rw_enter+0x57 #1 rrw_enter+0x3d #2 ufs_lock+0x27 #3 VOP_LOCK+0x50 #4 vn_lock+0x91 #5 vn_rdwr+0x64 #6 vndstrategy+0x2bd #7 physio+0x18f #8 vndwrite+0x1a #9 spec_write+0x74 #10 VOP_WRITE+0x3f #11 vn_write+0xde #12 dofilewritev+0xbb #13 sys_pwrite+0x55 #14 syscall+0x2ec #15 Xsyscall_untramp+0xa9
Re: Towards unlocking mmap(2) & munmap(2)
On Fri, Oct 28, 2022 at 11:08:55AM +0200, Martin Pieuchot wrote: > On 20/10/22(Thu) 16:17, Martin Pieuchot wrote: > > On 11/09/22(Sun) 12:26, Martin Pieuchot wrote: > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > mmap(2) for anons and munmap(2). > > > > > > Please test it with WITNESS enabled and report back. > > > > New version of the diff that includes a lock/unlock dance in > > uvm_map_teardown(). While grabbing this lock should not be strictly > > necessary because no other reference to the map should exist when the > > reaper is holding it, it helps make progress with asserts. Grabbing > > the lock is easy and it can also save us a lot of time if there is any > > reference counting bugs (like we've discovered w/ vnode and swapping). > > Here's an updated version that adds a lock/unlock dance in > uvm_map_deallocate() to satisfy the assert in uvm_unmap_remove(). > Thanks to tb@ from pointing this out. > > I received many positive feedback and test reports, I'm now asking for > oks. ok tb
Re: Towards unlocking mmap(2) & munmap(2)
On 20/10/22(Thu) 16:17, Martin Pieuchot wrote:
> On 11/09/22(Sun) 12:26, Martin Pieuchot wrote:
> > Diff below adds a minimalist set of assertions to ensure proper locks
> > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of
> > mmap(2) for anons and munmap(2).
> >
> > Please test it with WITNESS enabled and report back.
>
> New version of the diff that includes a lock/unlock dance in
> uvm_map_teardown(). While grabbing this lock should not be strictly
> necessary because no other reference to the map should exist when the
> reaper is holding it, it helps make progress with asserts. Grabbing
> the lock is easy and it can also save us a lot of time if there is any
> reference counting bugs (like we've discovered w/ vnode and swapping).
Here's an updated version that adds a lock/unlock dance in
uvm_map_deallocate() to satisfy the assert in uvm_unmap_remove().
Thanks to tb@ from pointing this out.
I received many positive feedback and test reports, I'm now asking for
oks.
Index: uvm/uvm_addr.c
===
RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
retrieving revision 1.31
diff -u -p -r1.31 uvm_addr.c
--- uvm/uvm_addr.c 21 Feb 2022 10:26:20 - 1.31
+++ uvm/uvm_addr.c 28 Oct 2022 08:41:30 -
@@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, stru
!(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr))
return ENOMEM;
+ vm_map_assert_anylock(map);
+
error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr,
entry_out, addr_out, sz, align, offset, prot, hint);
Index: uvm/uvm_fault.c
===
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.132
diff -u -p -r1.132 uvm_fault.c
--- uvm/uvm_fault.c 31 Aug 2022 01:27:04 - 1.132
+++ uvm/uvm_fault.c 28 Oct 2022 08:41:30 -
@@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, va
struct vm_page *pg;
KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+ vm_map_assert_anylock(map);
/*
* we assume that the area we are unwiring has actually been wired
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.301
diff -u -p -r1.301 uvm_map.c
--- uvm/uvm_map.c 24 Oct 2022 15:11:56 - 1.301
+++ uvm/uvm_map.c 28 Oct 2022 08:46:28 -
@@ -491,6 +491,8 @@ uvmspace_dused(struct vm_map *map, vaddr
vaddr_t stack_begin, stack_end; /* Position of stack. */
KASSERT(map->flags & VM_MAP_ISVMSPACE);
+ vm_map_assert_anylock(map);
+
vm = (struct vmspace *)map;
stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
@@ -570,6 +572,8 @@ uvm_map_isavail(struct vm_map *map, stru
if (addr + sz < addr)
return 0;
+ vm_map_assert_anylock(map);
+
/*
* Kernel memory above uvm_maxkaddr is considered unavailable.
*/
@@ -1457,6 +1461,8 @@ uvm_map_mkentry(struct vm_map *map, stru
entry->guard = 0;
entry->fspace = 0;
+ vm_map_assert_wrlock(map);
+
/* Reset free space in first. */
free = uvm_map_uaddr_e(map, first);
uvm_mapent_free_remove(map, free, first);
@@ -1584,6 +1590,8 @@ boolean_t
uvm_map_lookup_entry(struct vm_map *map, vaddr_t address,
struct vm_map_entry **entry)
{
+ vm_map_assert_anylock(map);
+
*entry = uvm_map_entrybyaddr(&map->addr, address);
return *entry != NULL && !UVM_ET_ISHOLE(*entry) &&
(*entry)->start <= address && (*entry)->end > address;
@@ -1704,6 +1712,8 @@ uvm_map_is_stack_remappable(struct vm_ma
vaddr_t end = addr + sz;
struct vm_map_entry *first, *iter, *prev = NULL;
+ vm_map_assert_anylock(map);
+
if (!uvm_map_lookup_entry(map, addr, &first)) {
printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n",
addr, end, map);
@@ -1868,6 +1878,8 @@ uvm_mapent_mkfree(struct vm_map *map, st
vaddr_t addr; /* Start of freed range. */
vaddr_t end; /* End of freed range. */
+ UVM_MAP_REQ_WRITE(map);
+
prev = *prev_ptr;
if (prev == entry)
*prev_ptr = prev = NULL;
@@ -1996,10 +2008,7 @@ uvm_unmap_remove(struct vm_map *map, vad
if (start >= end)
return 0;
- if ((map->flags & VM_MAP_INTRSAFE) == 0)
- splassert(IPL_NONE);
- else
- splassert(IPL_VM);
+ vm_map_assert_wrlock(map);
/* Find first affected entry. */
entry = uvm_map_entrybyaddr(&map->addr, start);
@@ -2531,6 +2540,8 @@ uvm_map_teardown(struct vm_map *map)
KASSERT((map->flags & VM_MAP_IN
Re: Towards unlocking mmap(2) & munmap(2)
On 11/09/22(Sun) 12:26, Martin Pieuchot wrote:
> Diff below adds a minimalist set of assertions to ensure proper locks
> are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of
> mmap(2) for anons and munmap(2).
>
> Please test it with WITNESS enabled and report back.
New version of the diff that includes a lock/unlock dance in
uvm_map_teardown(). While grabbing this lock should not be strictly
necessary because no other reference to the map should exist when the
reaper is holding it, it helps make progress with asserts. Grabbing
the lock is easy and it can also save us a lot of time if there is any
reference counting bugs (like we've discovered w/ vnode and swapping).
Please test and report back.
Index: uvm/uvm_addr.c
===
RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
retrieving revision 1.31
diff -u -p -r1.31 uvm_addr.c
--- uvm/uvm_addr.c 21 Feb 2022 10:26:20 - 1.31
+++ uvm/uvm_addr.c 20 Oct 2022 14:09:30 -
@@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, stru
!(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr))
return ENOMEM;
+ vm_map_assert_anylock(map);
+
error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr,
entry_out, addr_out, sz, align, offset, prot, hint);
Index: uvm/uvm_fault.c
===
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.132
diff -u -p -r1.132 uvm_fault.c
--- uvm/uvm_fault.c 31 Aug 2022 01:27:04 - 1.132
+++ uvm/uvm_fault.c 20 Oct 2022 14:09:30 -
@@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, va
struct vm_page *pg;
KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+ vm_map_assert_anylock(map);
/*
* we assume that the area we are unwiring has actually been wired
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.298
diff -u -p -r1.298 uvm_map.c
--- uvm/uvm_map.c 16 Oct 2022 16:16:37 - 1.298
+++ uvm/uvm_map.c 20 Oct 2022 14:09:31 -
@@ -491,6 +491,8 @@ uvmspace_dused(struct vm_map *map, vaddr
vaddr_t stack_begin, stack_end; /* Position of stack. */
KASSERT(map->flags & VM_MAP_ISVMSPACE);
+ vm_map_assert_anylock(map);
+
vm = (struct vmspace *)map;
stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
@@ -570,6 +572,8 @@ uvm_map_isavail(struct vm_map *map, stru
if (addr + sz < addr)
return 0;
+ vm_map_assert_anylock(map);
+
/*
* Kernel memory above uvm_maxkaddr is considered unavailable.
*/
@@ -1457,6 +1461,8 @@ uvm_map_mkentry(struct vm_map *map, stru
entry->guard = 0;
entry->fspace = 0;
+ vm_map_assert_wrlock(map);
+
/* Reset free space in first. */
free = uvm_map_uaddr_e(map, first);
uvm_mapent_free_remove(map, free, first);
@@ -1584,6 +1590,8 @@ boolean_t
uvm_map_lookup_entry(struct vm_map *map, vaddr_t address,
struct vm_map_entry **entry)
{
+ vm_map_assert_anylock(map);
+
*entry = uvm_map_entrybyaddr(&map->addr, address);
return *entry != NULL && !UVM_ET_ISHOLE(*entry) &&
(*entry)->start <= address && (*entry)->end > address;
@@ -1704,6 +1712,8 @@ uvm_map_is_stack_remappable(struct vm_ma
vaddr_t end = addr + sz;
struct vm_map_entry *first, *iter, *prev = NULL;
+ vm_map_assert_anylock(map);
+
if (!uvm_map_lookup_entry(map, addr, &first)) {
printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n",
addr, end, map);
@@ -1868,6 +1878,8 @@ uvm_mapent_mkfree(struct vm_map *map, st
vaddr_t addr; /* Start of freed range. */
vaddr_t end; /* End of freed range. */
+ UVM_MAP_REQ_WRITE(map);
+
prev = *prev_ptr;
if (prev == entry)
*prev_ptr = prev = NULL;
@@ -1996,10 +2008,7 @@ uvm_unmap_remove(struct vm_map *map, vad
if (start >= end)
return 0;
- if ((map->flags & VM_MAP_INTRSAFE) == 0)
- splassert(IPL_NONE);
- else
- splassert(IPL_VM);
+ vm_map_assert_wrlock(map);
/* Find first affected entry. */
entry = uvm_map_entrybyaddr(&map->addr, start);
@@ -2526,6 +2535,8 @@ uvm_map_teardown(struct vm_map *map)
KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+ vm_map_lock(map);
+
/* Remove address selectors. */
uvm_addr_destroy(map->uaddr_exe);
map->uaddr_exe = NULL;
@@ -2567,6 +2578,8 @@ uvm_map_teardown(struct vm_map *map)
entry = TAILQ_NEXT(entry, dfree.deadq);
}
Re: Towards unlocking mmap(2) & munmap(2)
On 14/09/22(Wed) 15:47, Klemens Nanni wrote: > On 14.09.22 18:55, Mike Larkin wrote: > > On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote: > > > Diff below adds a minimalist set of assertions to ensure proper locks > > > are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of > > > mmap(2) for anons and munmap(2). > > > > > > Please test it with WITNESS enabled and report back. > > > > > > > Do you want this tested in conjunction with the aiodoned diff or by itself? > > This diff looks like a subset of the previous uvm lock assertion diff > that came out of the previous "unlock mmap(2) for anonymous mappings" > thread[0]. > > https://marc.info/?l=openbsd-tech&m=164423248318212&w=2 > > It didn't land eventually, I **think** syzcaller was a blocker which we > only realised once it was committed and picked up by syzcaller. > > Now it's been some time and more UVM changes landed, but the majority > (if not all) lock assertions and comments from the above linked diff > should still hold true. > > mpi, I can dust off and resend that diff, If you want. > Nothing for release, but perhaps it helps testing your current efforts. Please hold on, this diff is known to trigger a KASSERT() with witness. I'll send an update version soon. Thank you for disregarding this diff for the moment.
Re: Towards unlocking mmap(2) & munmap(2)
On 14.09.22 18:55, Mike Larkin wrote: On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote: Diff below adds a minimalist set of assertions to ensure proper locks are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of mmap(2) for anons and munmap(2). Please test it with WITNESS enabled and report back. Do you want this tested in conjunction with the aiodoned diff or by itself? This diff looks like a subset of the previous uvm lock assertion diff that came out of the previous "unlock mmap(2) for anonymous mappings" thread[0]. https://marc.info/?l=openbsd-tech&m=164423248318212&w=2 It didn't land eventually, I **think** syzcaller was a blocker which we only realised once it was committed and picked up by syzcaller. Now it's been some time and more UVM changes landed, but the majority (if not all) lock assertions and comments from the above linked diff should still hold true. mpi, I can dust off and resend that diff, If you want. Nothing for release, but perhaps it helps testing your current efforts.
Re: Towards unlocking mmap(2) & munmap(2)
On Sun, Sep 11, 2022 at 12:26:31PM +0200, Martin Pieuchot wrote:
> Diff below adds a minimalist set of assertions to ensure proper locks
> are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of
> mmap(2) for anons and munmap(2).
>
> Please test it with WITNESS enabled and report back.
>
Do you want this tested in conjunction with the aiodoned diff or by itself?
> Index: uvm/uvm_addr.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 uvm_addr.c
> --- uvm/uvm_addr.c21 Feb 2022 10:26:20 - 1.31
> +++ uvm/uvm_addr.c11 Sep 2022 09:08:10 -
> @@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, stru
> !(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr))
> return ENOMEM;
>
> + vm_map_assert_anylock(map);
> +
> error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr,
> entry_out, addr_out, sz, align, offset, prot, hint);
>
> Index: uvm/uvm_fault.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> retrieving revision 1.132
> diff -u -p -r1.132 uvm_fault.c
> --- uvm/uvm_fault.c 31 Aug 2022 01:27:04 - 1.132
> +++ uvm/uvm_fault.c 11 Sep 2022 08:57:35 -
> @@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, va
> struct vm_page *pg;
>
> KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
> + vm_map_assert_anylock(map);
>
> /*
>* we assume that the area we are unwiring has actually been wired
> Index: uvm/uvm_map.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_map.c,v
> retrieving revision 1.294
> diff -u -p -r1.294 uvm_map.c
> --- uvm/uvm_map.c 15 Aug 2022 15:53:45 - 1.294
> +++ uvm/uvm_map.c 11 Sep 2022 09:37:44 -
> @@ -162,6 +162,8 @@ int
> uvm_map_inentry_recheck(u_long, v
>struct p_inentry *);
> boolean_t uvm_map_inentry_fix(struct proc *, struct p_inentry *,
>vaddr_t, int (*)(vm_map_entry_t), u_long);
> +boolean_t uvm_map_is_stack_remappable(struct vm_map *,
> + vaddr_t, vsize_t);
> /*
> * Tree management functions.
> */
> @@ -491,6 +493,8 @@ uvmspace_dused(struct vm_map *map, vaddr
> vaddr_t stack_begin, stack_end; /* Position of stack. */
>
> KASSERT(map->flags & VM_MAP_ISVMSPACE);
> + vm_map_assert_anylock(map);
> +
> vm = (struct vmspace *)map;
> stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
> stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
> @@ -570,6 +574,8 @@ uvm_map_isavail(struct vm_map *map, stru
> if (addr + sz < addr)
> return 0;
>
> + vm_map_assert_anylock(map);
> +
> /*
>* Kernel memory above uvm_maxkaddr is considered unavailable.
>*/
> @@ -1446,6 +1452,8 @@ uvm_map_mkentry(struct vm_map *map, stru
> entry->guard = 0;
> entry->fspace = 0;
>
> + vm_map_assert_wrlock(map);
> +
> /* Reset free space in first. */
> free = uvm_map_uaddr_e(map, first);
> uvm_mapent_free_remove(map, free, first);
> @@ -1573,6 +1581,8 @@ boolean_t
> uvm_map_lookup_entry(struct vm_map *map, vaddr_t address,
> struct vm_map_entry **entry)
> {
> + vm_map_assert_anylock(map);
> +
> *entry = uvm_map_entrybyaddr(&map->addr, address);
> return *entry != NULL && !UVM_ET_ISHOLE(*entry) &&
> (*entry)->start <= address && (*entry)->end > address;
> @@ -1692,6 +1702,8 @@ uvm_map_is_stack_remappable(struct vm_ma
> vaddr_t end = addr + sz;
> struct vm_map_entry *first, *iter, *prev = NULL;
>
> + vm_map_assert_anylock(map);
> +
> if (!uvm_map_lookup_entry(map, addr, &first)) {
> printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n",
> addr, end, map);
> @@ -1843,6 +1855,8 @@ uvm_mapent_mkfree(struct vm_map *map, st
> vaddr_t addr; /* Start of freed range. */
> vaddr_t end; /* End of freed range. */
>
> + UVM_MAP_REQ_WRITE(map);
> +
> prev = *prev_ptr;
> if (prev == entry)
> *prev_ptr = prev = NULL;
> @@ -1971,10 +1985,7 @@ uvm_unmap_remove(struct vm_map *map, vad
> if (start >= end)
> return;
>
> - if ((map->flags & VM_MAP_INTRSAFE) == 0)
> - splassert(IPL_NONE);
> - else
> - splassert(IPL_VM);
> + vm_map_assert_wrlock(map);
>
> /* Find first affected entry. */
> entry = uvm_map_entrybyaddr(&map->addr, start);
> @@ -4027,6 +4038,8 @@ uvm_map_checkprot(struct vm_map *map, va
> {
> struct vm_map_entry *entry;
>
> + vm_map_assert_anylock(map);
> +
> if (start < map->min_offset || end > map->max_offset || sta
Towards unlocking mmap(2) & munmap(2)
Diff below adds a minimalist set of assertions to ensure proper locks
are held in uvm_mapanon() and uvm_unmap_remove() which are the guts of
mmap(2) for anons and munmap(2).
Please test it with WITNESS enabled and report back.
Index: uvm/uvm_addr.c
===
RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
retrieving revision 1.31
diff -u -p -r1.31 uvm_addr.c
--- uvm/uvm_addr.c 21 Feb 2022 10:26:20 - 1.31
+++ uvm/uvm_addr.c 11 Sep 2022 09:08:10 -
@@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, stru
!(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr))
return ENOMEM;
+ vm_map_assert_anylock(map);
+
error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr,
entry_out, addr_out, sz, align, offset, prot, hint);
Index: uvm/uvm_fault.c
===
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
retrieving revision 1.132
diff -u -p -r1.132 uvm_fault.c
--- uvm/uvm_fault.c 31 Aug 2022 01:27:04 - 1.132
+++ uvm/uvm_fault.c 11 Sep 2022 08:57:35 -
@@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, va
struct vm_page *pg;
KASSERT((map->flags & VM_MAP_INTRSAFE) == 0);
+ vm_map_assert_anylock(map);
/*
* we assume that the area we are unwiring has actually been wired
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.294
diff -u -p -r1.294 uvm_map.c
--- uvm/uvm_map.c 15 Aug 2022 15:53:45 - 1.294
+++ uvm/uvm_map.c 11 Sep 2022 09:37:44 -
@@ -162,6 +162,8 @@ int uvm_map_inentry_recheck(u_long, v
struct p_inentry *);
boolean_t uvm_map_inentry_fix(struct proc *, struct p_inentry *,
vaddr_t, int (*)(vm_map_entry_t), u_long);
+boolean_t uvm_map_is_stack_remappable(struct vm_map *,
+vaddr_t, vsize_t);
/*
* Tree management functions.
*/
@@ -491,6 +493,8 @@ uvmspace_dused(struct vm_map *map, vaddr
vaddr_t stack_begin, stack_end; /* Position of stack. */
KASSERT(map->flags & VM_MAP_ISVMSPACE);
+ vm_map_assert_anylock(map);
+
vm = (struct vmspace *)map;
stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr);
@@ -570,6 +574,8 @@ uvm_map_isavail(struct vm_map *map, stru
if (addr + sz < addr)
return 0;
+ vm_map_assert_anylock(map);
+
/*
* Kernel memory above uvm_maxkaddr is considered unavailable.
*/
@@ -1446,6 +1452,8 @@ uvm_map_mkentry(struct vm_map *map, stru
entry->guard = 0;
entry->fspace = 0;
+ vm_map_assert_wrlock(map);
+
/* Reset free space in first. */
free = uvm_map_uaddr_e(map, first);
uvm_mapent_free_remove(map, free, first);
@@ -1573,6 +1581,8 @@ boolean_t
uvm_map_lookup_entry(struct vm_map *map, vaddr_t address,
struct vm_map_entry **entry)
{
+ vm_map_assert_anylock(map);
+
*entry = uvm_map_entrybyaddr(&map->addr, address);
return *entry != NULL && !UVM_ET_ISHOLE(*entry) &&
(*entry)->start <= address && (*entry)->end > address;
@@ -1692,6 +1702,8 @@ uvm_map_is_stack_remappable(struct vm_ma
vaddr_t end = addr + sz;
struct vm_map_entry *first, *iter, *prev = NULL;
+ vm_map_assert_anylock(map);
+
if (!uvm_map_lookup_entry(map, addr, &first)) {
printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n",
addr, end, map);
@@ -1843,6 +1855,8 @@ uvm_mapent_mkfree(struct vm_map *map, st
vaddr_t addr; /* Start of freed range. */
vaddr_t end; /* End of freed range. */
+ UVM_MAP_REQ_WRITE(map);
+
prev = *prev_ptr;
if (prev == entry)
*prev_ptr = prev = NULL;
@@ -1971,10 +1985,7 @@ uvm_unmap_remove(struct vm_map *map, vad
if (start >= end)
return;
- if ((map->flags & VM_MAP_INTRSAFE) == 0)
- splassert(IPL_NONE);
- else
- splassert(IPL_VM);
+ vm_map_assert_wrlock(map);
/* Find first affected entry. */
entry = uvm_map_entrybyaddr(&map->addr, start);
@@ -4027,6 +4038,8 @@ uvm_map_checkprot(struct vm_map *map, va
{
struct vm_map_entry *entry;
+ vm_map_assert_anylock(map);
+
if (start < map->min_offset || end > map->max_offset || start > end)
return FALSE;
if (start == end)
@@ -4886,6 +4899,7 @@ uvm_map_freelist_update(struct vm_map *m
vaddr_t b_start, vaddr_t b_end, vaddr_t s_start, vaddr_t s_end, int flags)
{
KDASSERT(b_end >= b_star
