On 03/05/25(Sat) 07:16, Theo de Raadt wrote:
> Claudio Jeker <claudio@openbsd.org> wrote:
> 
> > On Sat, May 03, 2025 at 02:08:35AM -0400, Ted Unangst wrote:
> > > On 2025-05-03, Claudio Jeker wrote:
> > > 
> > > > I mentioned this to mpi@, his diff does not free threads until the process
> > > > exits and runs out of proc on any workload that uses many threads.
> > > > This just does not work, his diff is over agressive by removing the reaper
> > > > even for that one case..
> > > 
> > > I think that chunk can be pulled up a little in exit1, so that when one
> > > thread exits, it will delete any dead siblings. Need a P_ZOMBIE, or a
> > > pr_deadproc list. This seems like an old idea? Did I forgot why we
> > > rejected it?
> > 
> > I do not understand why we can't keep the reaper just for this. The work
> > needed for releasing struct proc and its uarea is minimal. Why delay it
> > until the next thread exits?
> 
> I do not understand any of the diff.  It seems to be trying to return a
> child to the parent, before resources are cleared.

It doesn't.  Order of operations didn't change.  The KERNEL_LOCK() is
there and asserted.

> [...] 
> > There is no need to remove the reaper but we need to move the heavy uvm
> > teardown out of it.

I disagree.  I need to remove the reaper.  That *is* the problem.

> I've called this uvmspace_purge().  It would be a bit like
> 
>      uvm_unmap_remove(p->p_vmspace, 0, VM_MAXUSER_ADDRESS,
>          &dead_entries, FALSE, TRUE, TRUE)
> 
> Except there are clearly optimizations.  The function can sleep.  It is
> called quite early in process context.
> [...] 

There's no need to add a new function.  Calling uvm_exit() from exit1()
is enough and work but that's not buying us much.

Thanks for all of you that reported issues with this current diff.  I'll
try to reproduce and send a new one.

If you're interest helping, please let me know.  I'm still interested on
feedbacks about the pieces of this diff that are independent from the
exit1() changes.