From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: Re: pool: Check that we can sleep early
To: Christian Ludwig <cludwig@genua.de>
Cc: tech@openbsd.org
Date: Thu, 31 Jul 2025 16:50:01 +0200

On Thu, Jul 31, 2025 at 04:01:12PM +0200, Christian Ludwig wrote:
> Hi,
> 
> bluhm@ has hit a bug earlier ([1]) that panics pretty late, after a
> context switch to a different thread. The resulting backtrace is rather
> useless. The problem was a missing mtx_leave() before calling
> pool_get(..., PR_WAITOK) in a completely different code path.
> 
> This diff adds a check in pool_get() that we actually are in a sleepable
> context when PR_WAITOK is given. There is an equivalent check present in
> malloc() already.
> 
> Tests and feedback welcome.
 
Yes please. OK claudio@
 
>  - Christian
> 
> [1] https://marc.info/?l=openbsd-bugs&m=175139531419854
> 
> 
> diff --git a/sys/kern/subr_pool.c b/sys/kern/subr_pool.c
> index d76e8b27cd57..7fb57d569d1c 100644
> --- a/sys/kern/subr_pool.c
> +++ b/sys/kern/subr_pool.c
> @@ -584,8 +584,13 @@ pool_get(struct pool *pp, int flags)
>  	}
>  	pl_leave(pp, &pp->pr_lock);
>  
> -	if ((slowdown || pool_debug == 2) && ISSET(flags, PR_WAITOK))
> -		yield();
> +	if (ISSET(flags, PR_WAITOK)) {
> +#ifdef DIAGNOSTIC
> +		assertwaitok();
> +#endif
> +		if (slowdown || pool_debug == 2)
> +			yield();
> +	}
>  
>  	if (v == NULL) {
>  		struct pool_get_memory mem = { .v = NULL };



-- 
:wq Claudio