Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: sys/uvm: unwind failed amap copies
To:
Kirill A. Korinsky <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Sat, 08 Aug 2026 15:55:04 +0200

Download raw body.

Thread
> Date: Mon, 03 Aug 2026 01:23:08 +0200
> From: Kirill A. Korinsky <kirill@korins.ky>

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?

> +			amap_wipeout(amap);
>  			return;
>  		}
>  
> 
> 
> -- 
> wbr, Kirill
> 
>