Re: [PATCH v4 03/20] Documentation: Mention control groups

[Valentin Gagarin <valentin@gagarin.work>]

On Tue, 21 Jul 2026 21:59:08 -0400, Demi Marie Obenour <demiobenour@gmail.com> wrote:
> diff --git a/Documentation/doc/development/control-groups.adoc b/Documentation/doc/development/control-groups.adoc
> new file mode 100644
> index 0000000..6ce33f2
> --- /dev/null
> +++ b/Documentation/doc/development/control-groups.adoc
> @@ -0,0 +1,88 @@
> += Control groups in Spectrum
> +
> +// SPDX-FileCopyrightText: 2026 Demi Marie Obenour <demiobenour@gmail.com>
> +// SPDX-License-Identifier: GFDL-1.3-no-invariants-or-later OR CC-BY-SA-4.0
> +
> +Linux control groups (cgroups) can be used for several purposes:
> +
> +1. They allow waiting for a group of processes to exit.
> +2. They allow terminating a group of processes.
> +3. They allow limiting a group of processes' access to resources.

Not repeating prefixes helps readability.  And a link to upstream
documentation is always good.

    https://www.kernel.org/doc/html/latest/admin-guide/cgroup-v2.html[Linux control groups] (cgroups) can be used for several purposes:
    
    1. Waiting for a group of processes to exit.
    2. Terminating a group of processes.
    3. Limiting a group of processes' access to resources.

> [ ... skip 10 lines ... ]
> +2. The per-VM services for each VM are under `/vm-services.slice/vm-${VM}.slice`,
> +   where `${VM}` is replaced by the VM's ID.
> +3. Each per-VM service is under `/vm-services.slice/vm-${VM}.slice/${SERVICE_NAME}`,
> +   where `${VM}` is replaced by the VM's ID and `${SERVICE_NAME}` is replaced by
> +   the name of the service.
> +4. The VMM runs under `/vm-services.slice/vm-${VM}.slice/vmm`.

I don't think we introduce VMM anywhere else in the documentation, so
let's write it out.

    4. The virtual machine manager (VMM) runs under `/vm-services.slice/vm-${VM}.slice/vmm`.

> +
> +If a cgroup contains child cgroups, it likely contains a `$inner.service`
> +cgroup.  This is where programs that would otherwise run in the cgroup itself
> +are placed.  Generally, these programs are instances of `s6-svscan` and/or
> +`s6-supervise`.

Since we're not discussing s6 anywhere, please link to what these things
mean.  Also I think it's fine to imply logical OR when using "or" with
our audience, and reduce the visual noise (the general recommendation is
to avoid slashes in prose).

    are placed.  Generally, these programs are instances of
    https://skarnet.org/software/s6/s6-svscan.html[`s6-svscan`] or
    https://skarnet.org/software/s6/s6-supervise.html[`s6-supervise`].

> +
> +== Using Control Groups
> +
> +When adding a new s6 service, one should carefully consider whether it
> +should be placed in a control group.  Most services should be placed in
> +a control group, with only a few exceptions:

The first sentence reads as if one now has to make serious decisions,
but the second one amounts to "mostly no".  This was confusing to read,
and took attention away from the relevant parts, which seem to revolve
around "how to do that".  I recommend removing the first sentence.

    Most services should be placed in
    a control group, with only a few exceptions:

> +
> +1. Services, such as `getty`, that spawn background processes.
> +2. Loggers.
> +3. Trivial services that don't do anything.
> +
> +Generally, it's best to set the control group up as the first thing
> +the service does.  To do that, use `cgroup-setup --leaf -- $1 COMMAND_LINE`,
> +where `$1` should be the service name and `COMMAND_LINE` is the program
> +to run in a cgroup.

It took me a while to figure out that this is a new tool that comes
with Spectrum.  Not sure how to handle that best, but here's an attempt:

    Generally, it's best to set the control group up as the first thing
    the service does, with Spectrum's https://spectrum-os.org/git/spectrum/tree/tools/cgroup-setup[`cgroup-setup`].

    Call `cgroup-setup --leaf -- $1 COMMAND_LINE`,
    where `$1` should be the service name and `COMMAND_LINE` is the program
    to run in a cgroup.

> +
> +If you use execline for your run script, this is as simple as:

For example, using https://skarnet.org/software/execline/[execline] for your run script:

> +
> +.run
> +....

But I'd prefer "manually" to make that unambiguous.

> +#!/bin/execlineb -WS1

That renders as a separate paragraph only containing "run".  Let's
remove that, it doesn't make sense visuallyr

> +
> +cgroup-setup --leaf -- $1
> +# rest of script comes here
> +....
> +
> +If the service exits, it's usually best to terminate any programs left
> +behind with SIGKILL and remove the control group.  In Spectrum, this is

To keep the convention of displaying code items in monospace.  Also
shouldn't it be "when a service exits", since it's usually a matter of
time and holds for any service?

    When a service exits, it's usually best to terminate any programs left
    behind with `SIGKILL` and remove the control group.  In Spectrum, this is

This new (sub-)section could use a heading, such as "Purging control
groups".  A separate section also removes the implication that we're
still talking about "the" same service as before.

> +called "purging" the cgroup.  To purge the cgroup when a service exits,
> +make the `finish` script invoke `/usr/bin/cgroup-s6-finish`.  The first
> +two command line arguments must be the first two arguments passed to the
> +`finish` script.  The third argument must be the path to the cgroup to

To purge the cgroup when a service exits,
    make the `finish` script in the https://skarnet.org/software/s6/servicedir.html[s6 service directory]
    invoke Spectrum's `/usr/bin/cgroup-s6-finish`.

This also needs an example.  It took me a couple of times to visualise
that it's just `cgroup-s6-finish $1 $2 $3`.

And is it necessary to say `/usr/bin/...`?  Ideally we'd already
indicate here that `cgroup-s6-finish` is a symlink to `cgroup-setup`.

> +be purged relative to the cgroup the program itself is in.  This is
> +usually, but not always, the third argument to the `finish` script.

This needs more information.  When is it the third argument, when is it
not?  I'd be helpless at this point.

> +
> +When invoked as `cgroup-s6-finish`, `cgroup-setup` checks if
> +the service exited due to a signal that caused it to dump core.  If it
> +did, `cgroup-s6-finish` exits with status 125, ensuring that
> +`s6-supervise` will *not* restart it.  This is intentional: if a service

+crashes due to a fatal signal, this is possibly a sign of memory

> +crashes due to a fatal signal, this is possibly a sign of memory
> +corruption.  Restarting the service in this case can turn an unreliable

This should probably go into reference documentation for that program,
but let's keep it here until we have a place to list it and a way to do
that automatically.

> +memory corruption exploit into a reliable one.  Rust panics do not cause
> +core dumps, so the service will be restarted afterwards.

I don't understand what that last sentence means for me as a user.  This
seems detached from the previous explanations.

> +
> +One can also use `cgroup-purge` to purge a cgroup explicitly.  This is
> +used to stop the VMM and all per-VM services when a VM is shut down.

Do you mean that command is intended to be used manually?  I'm not sure
here, because "explicitly" doesn't intuitively convey that to me.  If
you want to keep the terminology, an example with a bit of introduction
would help, such as:

> +
> +== Future plans
> +
> +Control groups are designed around a single writer process controlling each
> +of them.  Many Linux distros use systemd for this, but Spectrum doesn't use
> +systemd.  The only persistent per-service process is s6-supervise, but that
> +doesn't have control group support.

s/distros/distributions/ and maybe highlight command names with
monospace.

    Control groups are designed around a single writer process controlling each
    of them.  Many Linux distributions use `systemd` for this, but Spectrum doesn't use
    `systemd`.  The only persistent per-service process is `s6-supervise`, but that
    doesn't have control group support.


Generally I strongly recommend one sentence per line for documentation,
a recommendation that apparently goes back to early Unix[1].  That makes
review a lot easier.  As you can observe, otherwise one has to cut
random unrelated pieces from the diff to discuss one word or phrase from
a sentence.  We don't have to introduce it now, but should start this as
a convention.

[1]: https://rhodesmill.org/brandon/2012/one-sentence-per-line/

Reviewed-by: Valentin Gagarin <valentin@gagarin.work>

-- 
Valentin Gagarin <valentin@gagarin.work>