From: Jeremie Courreges-Anglas <jca@wxcvbn.org>
Subject: Re: pool: Check that we can sleep early
To: Christian Ludwig <cludwig@genua.de>
Cc: Jonathan Gray <jsg@jsg.id.au>, tech@openbsd.org
Date: Fri, 1 Aug 2025 11:24:22 +0200

On Thu, Jul 31, 2025 at 06:40:00PM +0200, Christian Ludwig wrote:
> On Fri, Aug 01, 2025 at 12:20:33AM +1000 Jonathan Gray wrote:
> > 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.
> > 
> > Why after the pool_do_get() call?
> 
> Ah, good catch!
> 
> > For a long time my tree has had the following without issue
> > 
> > Index: subr_pool.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/subr_pool.c,v
> > diff -u -p -r1.241 subr_pool.c
> > --- subr_pool.c	21 Jul 2025 20:36:41 -0000	1.241
> > +++ subr_pool.c	22 Jul 2025 02:52:13 -0000
> > @@ -562,6 +562,9 @@ pool_get(struct pool *pp, int flags)
> >  	void *v = NULL;
> >  	int slowdown = 0;
> >  
> > +	if (flags & PR_WAITOK)
> > +		assertwaitok();
> > +
> >  	KASSERT(flags & (PR_WAITOK | PR_NOWAIT));
> >  	if (pp->pr_flags & PR_RWLOCK)
> >  		KASSERT(flags & PR_WAITOK);
> > 
> 
> That is better for sure.

I'm fine with the code location as long as it happens before
pool_do_get().  I've been running with this on my builders and laptop,
and I'm not hitting immediate crashes.  Maybe we should just go ahead?

ok jca@

-- 
jca