Don't hardcode maximum PID.

[pjd]

Summary

• Obtain maximum PID from kern.pid_max sysctl.
• Use kern.pid_max and hw.ncpu to calculate column widths in top(1).
• Always expect kern.pid_max to exists, instead of using hardcoded PID maximum value.

With those changes the kernel can be compiled with a higher PID_MAX value, eg. 999999.

[kib]

If you manually upload diffs, please generate them with something like git diff -U999999 <whatever> so that the full content is available.

I think most of the changes are fine except I do not quite like the top modifications. Imagine that you just booted the machine, then all pids are short (4 digits). After some time, the display flips because you get the first 5-digid pid. Then again, depending on the display filtering criteria, pid column width would oscillate.

[kib]

For NO_PID, also look at usr.sbin/bsnmpd/modules/snmp_hostres/hostres_swrun_tbl.c and contrib/sendmail/src/sendmail.h (later is probably fine).

[pjd]

In D43077#982024, @kib wrote:

If you manually upload diffs, please generate them with something like git diff -U999999 <whatever> so that the full content is available.

Will do.

I think most of the changes are fine except I do not quite like the top modifications. Imagine that you just booted the machine, then all pids are short (4 digits). After some time, the display flips because you get the first 5-digid pid. Then again, depending on the display filtering criteria, pid column width would oscillate.

No, the PID column width won't change, because we look at the maximum PID, which is 99999, so it will be 5 digits all the time.

I should have include the motivation for this change. The idea here is to be able to compile the kernel with a higher PID_MAX, eg. 999999 and have userland adjust properly without recompiling.

I have a system with a high number of processes (~15000) and have a lot of lock contention on allproc_lock, etc. I was hoping I might be able to reduce contention if I increase PID name space by an order of magnitude, so it will be easier to find a free PID. I don't think it helped, but along the way I discovered some userland tools that assume PID can't be higher than 99999.

[pjd]

• Remove hardcoded PID_MAX from sched_{g,s}etaffinity().
• Remove hardcoded NO_PID from bsnmpd.

[pjd]

Add missing include.

[pjd]

In D43077#982025, @kib wrote:

For NO_PID, also look at usr.sbin/bsnmpd/modules/snmp_hostres/hostres_swrun_tbl.c and contrib/sendmail/src/sendmail.h (later is probably fine).

I've added changes to bsnmpd and sched_{get,set}affinity(). sendmail defines NO_PID as 0, so this is unrelated to my changes.

[zlei]

bin/pkill/pkill.c

837 ↗

(On Diff #131487)

Currently MIB kern.pid_max is writable tunable. In case it got decreased , i.e.

# sysctl kern.pid_max=1000
kern.pid_max:99999 -> 1000

Then pkill may no longer be able to kill previously spawned progress, say pid 2000.

The MIB kern.pid_max should only affect the process of forking new progress IIUC.

884 ↗

(On Diff #131487)

Then pkill may no longer be able to kill previously spawned progress, say pid 2000.

I'm not quite sure but it seems the right way should ignore the MAX_PID or pidmax().

[kib]

bin/pkill/pkill.c

884 ↗

(On Diff #131487)

Problem is that pids > MAX_PID are tids.

[zlei]

bin/pkill/pkill.c

884 ↗

(On Diff #131487)

Problem is that pids > MAX_PID are tids.

You are right.
The maximum possible pid (the kernel support) is hardcoded to MAX_PID.

See sys/sys/proc.h:

/*
 * We use process IDs <= pid_max <= PID_MAX; PID_MAX + 1 must also fit
 * in a pid_t, as it is used to represent "no process group".
 */
#define PID_MAX         99999
#define NO_PID          100000
#define THREAD0_TID     NO_PID
extern pid_t pid_max;

And,

# sysctl kern.pid_max=100000
kern.pid_max: 99999
sysctl: kern.pid_max=100000: Invalid argument

So

I'm not quite sure but it seems the right way should ignore the MAX_PID or pidmax().

is wrong.

Then we actually want a readonly MIB for the compile macro PID_MAX.

SYSCTL_INT(kern, OID_AUTO, compile_pid_max, CTLFLAG_RD,
    SYSCTL_NULL_INT_PTR, PID_MAX, "Compile maximum allowed pid");

and then userland read it instead of kern.pid_max.

[markj]

usr.sbin/bsnmpd/modules/snmp_hostres/hostres_swrun_tbl.c

434 ↗

(On Diff #131487)

Fix the whitespace while you're here?

559 ↗

(On Diff #131487)

At least write an error message here? Looks like other code in this file (also in the caller) uses syslog(3) for such things.

[kib]

bin/pkill/pkill.c

884 ↗

(On Diff #131487)

This may be in fact the right solution, and it is quite good for the problem at hands.

[pjd]

bin/pkill/pkill.c

837 ↗

(On Diff #131487)

Good catch. I wonder if there is a real value in having this writable, instead of just read-only tunable.
@kib I think you have introduced kern.pid_max. What was the use case for it? IIRC one use case was to be able to run FreeBSD 1.x jails under newer kernel where FreeBSD 1.x had a lower PID_MAX?
Still, adding read-only sysctl might be the best choice here.

884 ↗

(On Diff #131487)

Not checking if the PID in pidfile is greater than MAX_PID in pkill is IMHO best solution or am I missing something? Even if we have value greater than MAX_PID in the pidfile, kill(2) only operates on PIDs and not on TIDs.
Even if we introduce kern.compile_pid_max or similar I still think it would be best to just remove this condition.

[pjd]

lib/libc/gen/sched_setaffinity.c

47 ↗

(On Diff #131487)

As I understand this check is here to allow to use newer libc with an older kernel.
If we decide to add new sysctl we are going to need a new P_OSREL_ macro and check it here, right?

[pjd]

Remove MAX_PID check from pkill.

[kib]

I believe that zlei@ note should be handled globally for this patch. The width of the pid field for ps/top/anything else should be determined by the number of decimal digits in the value of the new sysctl.

lib/libc/gen/sched_setaffinity.c

47 ↗

(On Diff #131487)

Not necessarily, simply doing the new sysctl and falling back to hard-coded value is enough.

[pjd]

Introduce kern.pid_max_limit.

[zlei]

sys/kern/kern_mib.c

757

For the kernel part, I'd support if you would like to commit them separately.
Those can be also MFCed easily.

[pjd]

sys/kern/kern_mib.c

757

I can do that, that's not a problem.

[pjd]

sys/kern/kern_mib.c

757

@zlei If you are ok with the change if it will be done in two commits, can you accept PR?

[kib]

sys/kern/kern_mib.c

757

Please go ahead with the kernel change. Consider it 'reviewed by kib' if needed.

The libc change should be put into the next review.

[zlei]

sys/kern/kern_mib.c

757

@zlei If you are ok with the change if it will be done in two commits, can you accept PR?

@pjd The change to kernel part looks good to me. Just go ahead.