On 06/08/21(Fri) 08:08, Theo Buehler wrote:
> > The diff below fixes this by setting the "source" amap lock to the newly
> > allocated one. This is not strictly necessary on OpenBSD since the amap
> > is only inserted on the global list at the end of amap copy, but this
> > satisfies the locking requirement of amap_wipeout() and is IMHO the
> > simplest solution. This has also the advantage of reducing the
> > differences with NetBSD.
>
> This diff causes a reproducible hang with mount_mfs(8) on my laptop. I
> use an mfs noperm partition as described in
> https://www.openbsd.org/faq/faq5.html#Release
>
> $ tail -1 /etc/fstab
> swap /dest mfs rw,nosuid,noperm,-P/var/dest,-s1.5G,noauto 0 0
>
> If I run 'top -S -s.1' on an otherwise idle system and do 'mount /dest',
> I see two mount_mfs(8) processes spin in fltamap and the pagedaemon is
> spinning in pgdaemon before the system locks up completely (this takes
> something between 1 and 20 seconds to happen).
I couldn't reproduce this hang here. Do you also see it with the
smaller fix below?
Index: uvm/uvm_amap.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_amap.c,v
retrieving revision 1.89
diff -u -p -r1.89 uvm_amap.c
--- uvm/uvm_amap.c 26 Mar 2021 13:40:05 -0000 1.89
+++ uvm/uvm_amap.c 30 Aug 2021 08:31:21 -0000
@@ -618,6 +618,13 @@ amap_copy(struct vm_map *map, struct vm_
return;
srcamap = entry->aref.ar_amap;
+ /*
+ * Make the new amap share the source amap's lock, and then lock
+ * both.
+ */
+ amap->am_lock = srcamap->am_lock;
+ rw_obj_hold(amap->am_lock);
+
amap_lock(srcamap);
/*
@@ -655,7 +662,7 @@ amap_copy(struct vm_map *map, struct vm_
chunk = amap_chunk_get(amap, lcv, 1, PR_NOWAIT);
if (chunk == NULL) {
- amap_unlock(srcamap);
+ /* amap_wipeout() releases the lock. */
amap->am_ref = 0;
amap_wipeout(amap);
return;
@@ -695,10 +702,10 @@ amap_copy(struct vm_map *map, struct vm_
* If we referenced any anons, then share the source amap's lock.
* Otherwise, we have nothing in common, so allocate a new one.
*/
- KASSERT(amap->am_lock == NULL);
- if (amap->am_nused != 0) {
- amap->am_lock = srcamap->am_lock;
- rw_obj_hold(amap->am_lock);
+ KASSERT(amap->am_lock == srcamap->am_lock);
+ if (amap->am_nused == 0) {
+ rw_obj_free(amap->am_lock);
+ amap->am_lock = NULL;
}
amap_unlock(srcamap);
