In D58264#1340702, @markj wrote:

In D58264#1339790, @kib wrote:

In D58264#1339528, @markj wrote:

In D58264#1339300, @kib wrote:

The proposal is:

• if child was forked, the parent is guaranteed that waitpid() returns the exit status after the child exit.

Makes sense.

• If child was pdforked, waitpid does not see the child.

This changes existing semantics, right? Why is that ok?

IMO it is illogical. I think that waitpid() was enabled as a desperate measure due to lack of pdwait() and trouble implementing it.
But ok, I think I see how to still allow waitpid(). The proposed semantic is that waitpid() is allowed once, but is not required for reaping, and does not cause reaping. The child is reaped when the procdesc is pdwait-ed or closed still. This is done with 'weak' references.

• if there are procdescs on the child (does not matter, was the child forked or pdforked), we guarantee that exactly one pdwait() on some of the procdescs returns the exit status.

I'm dumb: why can't they all return the exit status? p_xexit is not cleared after it is read.

May be it is me who is dumb, but I do not see then when to reap? The problem is that it is impossible to distinguish between filedesc and file. For instance, I might set a flag on the file referencing procdesc that pdwait() was done on it. But this means that dup-ed file descriptor cannot pdwait().

This behavior would be very confusing, I do not know how to specify it without specifying the implementation.

Again, I sense I am misunderstanding something basic, but: only let the (real) parent reap. Whether it used fork() or pdfork() does not matter, only the parent can decide whether to reap. Any procdesc holder can use pdwait() to collect the instantaneous status, or to block until the parent reaps. Once the parent reaps, procdesc_reap() turns all procdescs into tombstones.

Is this a dup?

In D58402#1340563, @markj wrote:

In D58402#1340417, @markj wrote:

In D58402#1340410, @markj wrote:

I tried with 3 pages and got a huge number of test failures, and the VM became unresponsive. I don't know why. Maybe GET_STACK_USAGE() is returning false in some paths where it did not before, and it's causing problems. I will try to debug further.

I still don't know what's going on, but if I boot a VM with kstack_pages=3 and run tests in /usr/tests/libexec/tftpd, they all fail.

Sorry, this was user error (I accidentally overrode some other tunables when I set kern.kstack_pages, specifically net.inet.ip.fw.default_to_accept=1).

I tried running the test suite again with kstack_pages=3 and it seems ok. I can try the openzfs test suite too, but it takes a long time to run.

I tried on my crash box which is netbooted.

My reading of the graph you posted is that we never exceed 2 pages. So reducing the stack to 3 pages should be fine for this workload, while reducing to two is probably not going to work. Am I correct?

Initialize error.

Fix locking in procdecs_close() for zombies case.

Rebase on top of D58407

Add weak zombie references that allow waitpid() but do not block reaping.

In D58264#1339528, @markj wrote:

In D58264#1339300, @kib wrote:

The proposal is:

• if child was forked, the parent is guaranteed that waitpid() returns the exit status after the child exit.

Makes sense.

• If child was pdforked, waitpid does not see the child.

This changes existing semantics, right? Why is that ok?

In D58397#1339713, @markj wrote:

In D58397#1339692, @arrowd wrote:

This would prevent a process from tracking itself using EVFILT_PROC. Not sure how useful it is, but maybe it makes sense to add an exception for ident == getpid().

Yes, there is some precedent for that. For instance, raise() is supposed to work in capability mode.

Return error.

I am thinking about reducing the amd64 default kstack size to 4->3, or even 2.

I do not object strong, but knote is 160 bytes.

In D58393#1339635, @markj wrote:

Why?

In D58314#1339359, @feld wrote:

In D58314#1339356, @kib wrote:

This obviously changes the semantic of the pwait(1) tool. Before, it returned control after the process exited. Now, it additionally wait until a reaper makes the wait*() call.

But what does "exited" really mean in this context?

This obviously changes the semantic of the pwait(1) tool. Before, it returned control after the process exited. Now, it additionally wait until a reaper makes the wait*() call.

In D58264#1339299, @markj wrote:

In D58264#1339297, @kib wrote:

proc_to_reap(orphan) can reap.

I promised to look at the tests if there is an agreement on the idea of this change. I know that there is some breakage.

Some summary of 1) current semantics 2) proposed semantics would be useful. The review description is not very clear.

As I understand it, 2) is:

• only the parent can reap the child, by calling wait() or pdwait(),
• any procdescs which refer to the child will prevent the child PID from being recycled,
• anybody may use pdwait() to fetch the status of the child, but only the parent can consume the status.

proc_to_reap(orphan) can reap.

In D58264#1339293, @markj wrote:

Did you run the ptrace tests? Several fail, and then I see a panic

panic: reaped an orphan (pid 0)
cpuid = 13
time = 1784674501
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0xa5/frame 0xfffffe00f1a525b0
kdb_backtrace() at kdb_backtrace+0xc6/frame 0xfffffe00f1a52710
vpanic() at vpanic+0x214/frame 0xfffffe00f1a528b0
panic() at panic+0xb5/frame 0xfffffe00f1a52980
kern_wait6() at kern_wait6+0x57c/frame 0xfffffe00f1a52a10
sys_wait4() at sys_wait4+0x18e/frame 0xfffffe00f1a52d10
amd64_syscall() at amd64_syscall+0x3d8/frame 0xfffffe00f1a52f30
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe00f1a52f30
--- syscall (7, FreeBSD ELF64, wait4), rip = 0x3ce8fd7e46ea, rsp = 0x3ce8f93b2d78, rbp = 0x3ce8f93b2db0 ---

Rename PZOMBIEREF_ORPHAN to PZOMBIEREF_REALPARENT.
Move an assert into proc_realparent().
Correct the condition to reparent on the last procdesc close.
Start modifying the man pages.

Fix one more locking comment.

Change p_zombieref lock annotation to be just proctree_lock.

Will be consumed by D58264

Add a control variable for proc_can_ptrace() calls.