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.

I am terrified of that.  I worry that the uvm space could contain changes
which have not been reflected back into an object, say a mmap'd file.
The child exits, the parent assumes the objects are coherent, but they are
not.

All the process-exiting changes feel a micro-optimization which lands well
outside expectations, and I strongly suspect it is going to encounter
futher problems.

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

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.

It probably cannot use the guts of uvm_map_teardown(), because I think
on shared-U+K architectures it can damage the kernel mappings.  It cannot
call pmap_destroy().  It just purges mappings.

As claudio says, the reaper can remain, it performs the action of cleaning
the process lists like today, and does the final uvmspace_free().  Later on,
we can try to optimize the reaper, but such a change must not change the
deadproc / knote / wakeup handling.