fork: do not exclude pid 1's children from creating reapsubtrees

[val_packett.cool]

Summary

To make the procctl(PROC_REAP_*) API usable in pid 1 the same way as
in a subreaper, we need to not do this.

Sponsored by: https://www.patreon.com/valpackett

Test Plan

As far as I can tell this shoooouldn't change anything for people not trying to build an init system that uses this API, i.e. probably everyone except me right now?? But I don't know…

[kevans]

Looks like mjg added this second half of the conditional and kib the first, so let's tag them explicitly in case they have an opinion.

[mjg]

quoting the commit:

While here a bug was discovered: all processes would inherit reap id from
the first process spawned by init. This had a side effect of keeping the
ID used and when allocation rolls over to the beginning it keeps being
skipped.

[crest_freebsd_rlwinm.de]

What is gained by having PID 1 acquire sub-reaper status given that it's the reaper? What are the expected semantics for proctl(PROC_REAP_ACQUIRE) in PID 1? Would it be enough to wrap the procctl(…) call with if ( getpid() != 1 ) { … } or is there more to this?

[kib]

In D39996#910664, @mjg wrote:

quoting the commit:

While here a bug was discovered: all processes would inherit reap id from
the first process spawned by init. This had a side effect of keeping the
ID used and when allocation rolls over to the beginning it keeps being
skipped.

IMO it is micro-overoptimization, not a fix for a bug. It is fine as far as init(8) does not use reaper API.
It if starts using it, then the check needs to go away.

There is nothing wrong with the fact that all processes spawned from sh running /etc/rc get the same reaper subtree id, rather it is how the things should be done. init(8) might spawn other processes, mostly getty for historicaly reasons, and then console and terminal sessions get other reapsubtree ids.

In D39996#910877, @crest_freebsd_rlwinm.de wrote:

What is gained by having PID 1 acquire sub-reaper status given that it's the reaper? What are the expected semantics for proctl(PROC_REAP_ACQUIRE) in PID 1? Would it be enough to wrap the procctl(…) call with if ( getpid() != 1 ) { … } or is there more to this?

I do not understand this question. init does not get sub-reaper status neither in stock sources, nor after the proposed patch.

[val_packett.cool]

In D39996#911105, @kib wrote:

IMO it is micro-overoptimization, not a fix for a bug. It is fine as far as init(8) does not use reaper API.
It if starts using it, then the check needs to go away.

(well, *it* – users can already replace init(8) with any alternative…)

There is nothing wrong with the fact that all processes spawned from sh running /etc/rc get the same reaper subtree id, rather it is how the things should be done. init(8) might spawn other processes, mostly getty for historicaly reasons, and then console and terminal sessions get other reapsubtree ids.

Ahh, here I was trying to "reproduce the bug" without using /etc/rc and failing :D Now it makes sense what that commit was referring to. Agreed.

In D39996#910877, @crest_freebsd_rlwinm.de wrote:

What is gained by having PID 1 acquire sub-reaper status given that it's the reaper? What are the expected semantics for proctl(PROC_REAP_ACQUIRE) in PID 1? Would it be enough to wrap the procctl(…) call with if ( getpid() != 1 ) { … } or is there more to this?

I do not understand this question. init does not get sub-reaper status neither in stock sources, nor after the proposed patch.

IIUC there isn't a distinct "sub-reaper status", PROC_REAP_ACQUIRE just sets the same flag that pid 1 already gets from birth. In any case yeah, this is not about ACQUIRE at all.

[val_packett.cool]

ping!