Re: sys/uvm: unwind failed amap copies

On Sat, 08 Aug 2026 15:55:04 +0200,
Mark Kettenis <[email protected]> wrote:
> 
> > Date: Mon, 03 Aug 2026 01:23:08 +0200
> > From: Kirill A. Korinsky <[email protected]>
> 
> Hi Kirill,
> 
> > 
> > tech@,
> > 
> > I continue my hunt on hangs and crashes at my very small octeon.
> > 
> > Here the second finding from that a few days long sprint.
> > 
> > A PR_NOWAIT chunk allocation in amap_copy() may fail after earlier
> > chunks already contain copied anons; amap_free() requires an empty amap,
> > so direct cleanup trips its diagnostic assertion or leaves copied anon
> > references orphaned which may end who knows how.
> > 
> > Here, I use amap_wipeout() instead to reverse the partial copy, release
> > the shared lock, and free the temporary amap.
> > 
> > Thought?
> > 
> > Index: sys/uvm/uvm_amap.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/uvm/uvm_amap.c,v
> > diff -u -p -r1.99 uvm_amap.c
> > --- sys/uvm/uvm_amap.c	18 Jun 2026 13:14:26 -0000	1.99
> > +++ sys/uvm/uvm_amap.c	2 Aug 2026 23:08:35 -0000
> > @@ -640,10 +640,8 @@ amap_copy(struct vm_map *map, struct vm_
> >  
> >  		chunk = amap_chunk_get(amap, lcv, 1, PR_NOWAIT);
> >  		if (chunk == NULL) {
> > -			amap_unlock(srcamap);
> > -			/* Destroy the new amap. */
> > -			amap->am_ref--;
> > -			amap_free(amap);
> > +			amap->am_ref = 0;
> 
> Why do you set am_ref to zero here instead of just decreasing it?
> 

Well, we shouldn't be here with am_ref not 1 as far as I understand the
code, and that am_ref-- was a bit missleading.

But after think more, I see that it is bad idea to replace it to 0.

We shouldn't be here with am_ref non 1 never means that we actually won't be
here, and if we here with am_ref not 1, am_ref-- makes am_ref not 0 and it
will crash inside amap_wipeout() at KASSERT(amap->am_ref == 0).

So, here cleaner diff.

Ok?

Index: sys/uvm/uvm_amap.c
===================================================================
RCS file: /home/cvs/src/sys/uvm/uvm_amap.c,v
diff -u -p -r1.99 uvm_amap.c
--- sys/uvm/uvm_amap.c	18 Jun 2026 13:14:26 -0000	1.99
+++ sys/uvm/uvm_amap.c	8 Aug 2026 17:05:19 -0000
@@ -640,10 +640,9 @@ amap_copy(struct vm_map *map, struct vm_
 
 		chunk = amap_chunk_get(amap, lcv, 1, PR_NOWAIT);
 		if (chunk == NULL) {
-			amap_unlock(srcamap);
-			/* Destroy the new amap. */
+			/* amap_wipeout() releases the shared lock. */
 			amap->am_ref--;
-			amap_free(amap);
+			amap_wipeout(amap);
 			return;
 		}
 


-- 
wbr, Kirill