splice: optionally limit worker queues

[gallatin]

Summary

This change introduces a tunable (kern.ipc.splice.num_wq) which can be used to limit the number
of splice worker queues. This can be desirable if you do not want splice threads to be able to
consume all available CPU on a server.

The default (-1) keeps the current behavior of running one worker for each core in the system.
An administrator can set it to 0 (either via tunable, or before the first splice call via sysctl) to
effectively disable splice, or some number smaller than the number of cores to limit
splice thread use.

[imp]

sys/kern/uipc_socket.c

346

This doesn't look like a tuneable. It's set to something else in both code paths.

[gallatin]

• fixed typo in sysctl declaration

[gallatin]

sys/kern/uipc_socket.c

346

Whoops, cut & paste bug. Good catch!

[markj]

FreeBSD/sys/kern/uipc_socket.c

1688 ↗

(On Diff #173093)

This won't work properly in the case where the CPU ID space has holes. That's what the loop was trying to handle before.

It might be easiest to rework the mapping between workers and CPU IDs: make splice_wq[] an array of mp_ncpus elements, and just use a counter instead of the CPU ID in the loop in splice_init() which starts each worker.

[gallatin]

FreeBSD/sys/kern/uipc_socket.c

1688 ↗

(On Diff #173093)

Yeah, I had that in the first iteration, but backed it when I changed to starting all threads. I'll restore it.
Since we don't bind the worker to a CPU, my feeling is that this should all just be based on the number of cores..

[gallatin]

Re-worked thread setup to use mp_ncpus rather than mp_maxid, as suggested by @markj

[markj]

FreeBSD/sys/kern/uipc_socket.c

354 ↗

(On Diff #173129)

If someone sets new = -1 via sysctl, during runtime, this will break. new == -1 is only handled in splice_init().

482 ↗

(On Diff #173129)

I'd rather have a separate tunable for this, but ok.

[gallatin]

FreeBSD/sys/kern/uipc_socket.c

354 ↗

(On Diff #173129)

Pasting in convo from slack, as I was not able to get into differential last night:

[8:25 PM]gallatin I'm in an airport, and for some reason differential is not letting me in.   From the email, I think you mean this nit;

uipc_socket.c:354
+ } else {
+ splice_num_wq = new;
+ }

If someone sets new = -1 via sysctl, during runtime, this will break. new == -1 is only handled in splice_init().

I'm not sure if I'm missing something... I'm taking the init lock and checking that we have not been init'ed before allowing -1.  That's what the check for (new <= 0 && splice_init_state != 0) is intended to prevent:

if (!cold)
        sx_xlock(&splice_init_lock);
if (new < -1 || new > mp_ncpus ||
    (new <= 0 && splice_init_state != 0)) {
        error = EINVAL;
} else {
        splice_num_wq = new;
}
if (!cold)
        sx_xunlock(&splice_init_lock);

[8:27 PM]Mark Johnston Ahh, my bad, I parsed that check incorrectly
[8:27 PM]gallatin NP, thanks for taking the time!
[8:27 PM]Mark Johnston LGTM then