From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id AC0C877BA; Wed, 26 Aug 2026 14:07:12 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 9544E77C3; Wed, 26 Aug 2026 14:07:10 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fout-a1-smtp.messagingengine.com (fout-a1-smtp.messagingengine.com [103.168.172.144]) by atuin.qyliss.net (Postfix) with ESMTPS id 6EA1E77C2 for ; Wed, 26 Aug 2026 14:07:08 +0000 (UTC) Received: from phl-compute-05.internal (phl-compute-05.internal [10.202.2.45]) by mailfout.phl.internal (Postfix) with ESMTP id 26020EC009E; Wed, 26 Aug 2026 10:07:06 -0400 (EDT) Received: from phl-frontend-04 ([10.202.2.163]) by phl-compute-05.internal (MEProxy); Wed, 26 Aug 2026 10:07:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1787753226; x=1787839626; bh=EGT7VdezD3 yqmVjK5cH9RdkH9u6wbXFPusnqdokVa3A=; b=GcdyoJRiaI1EKwdpbl1RthYH8W il6mO3QL4gQiRGcKi1sO6ujdK0Or2f/RDEf6QnUJiLUjzq1t9FaqS3IrvZVk4gHp FQTAS0+W3eUpG7sCjsmBWZ2+REWF1zO28ZTb1fYEfoUNm+TdUn5KOJyjUC+2t8Sb WarYJnZZnyDde4ov2mbrHMFOe0gejRPb/OCKIZQbavE3AMpNypud5Zsm+A4sRCON VXTXQMUFfPRWUS9tleNwM76VnShOy9RfVeFndEcEFta/Ir6NW+FHrAd8r6u/KEde oFNEg9SBF3cr5SA6Zt81n3PQ/IhoCmqp8up+wo5se/0sh+vt1bTc98PZzODQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1787753226; x=1787839626; bh=EGT7VdezD3yqmVjK5cH9RdkH9u6wbXFPusn qdokVa3A=; b=cuqil64QU/GWCQKgVYIlAulh0zhx/LOmz1Gre72R0ZTDYIzIb1K Oaa07XUhGFRXpIbkGygQqGN7dbZC/FVfmXtfhA8uwg6DFGbPPJewqmv39W+pgs3B PMr7Ilz1UFGZkEDsPgvNPX0FzJek0UgmJF28Vk5dlilYqPHD/oPh7qbw+UEk6oSt 4H/h512YUiLUIacGJ8lPL3xq0Q/Dem9/Jqch6F8QnCAoxUSc0/Dtocmnn8fD4wdP CKkb0aGCkK7UyCqXOdxpqkMZNSozdLXlNO299dyjEU64sGAA5chpJbOrYpmVW22i +nrEX8YGk3uyJdRpm5jrHoZzqqiyGmzzRQA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTFhdmMDTPeBMrYlmgKKCYz7jnMsjtRjr2U/sjonyBccB3Qb4XDlqQUuH4PqYbqsx9 EMtSxmbuOgOyduQsuuX0/ROVb9/8Zn0kAE9Yvk2rPbJkfVelcZ0akAZdNVgWndNpm49g/N vr0dteOIZ9hDkot7p0QY4aXQGpn6GtlR0r9dyIfufEBfHG/VLTbxp6dNbskvlIx6Ul6I/d d+X7VBqSKpH7ANlokpducQFxhLGGIDeoDqJYNMByt75dlfzGjLMn2rBWZ2+/oMAiSBZi3W PcYj8eJVVGY0KPaoJ2g5UatWrIUe1ZikbYy5CAVyUd1NNUs4PhyX2J3ZOB5MaiXC8s8CVG ThflHFWn4delBfeZz5NGfNahvmrTq5CKu/IxCiuK4aqIpPxeN3WYKSZbU4tUUDOrkFEfd4 dPyyQJU214IgbT/AvW9jeDw+9rTuF0EpaZXkNMPRJT1P1Xf4CuSs8/GvO7GA13tT/RSZHU QKd1SwUtNDgrmbGmVmHLCmy/J5p/kMD5g1VGErhayWmhb0jh3EK+tHNadh+KxkHsWVgzcI 8p4CYm9xlRGfETM31ZnJrlTuGqnQ4GQckkYvIwwpUVhr+HchfRLvZxTmY945sSjVWY8ViU X2o3gCrzWkYuT+0wCo+UKiivT6frFOY3BQjD4Y1mOSSNcgOM/PLO05oiQj6w X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 26 Aug 2026 10:07:05 -0400 (EDT) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 8C140C81D792; Wed, 26 Aug 2026 16:07:02 +0200 (CEST) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v7 02/19] tools: Add control group manager In-Reply-To: <20260821-cgroups-v7-2-7f1870dedefc@gmail.com> References: <20260821-cgroups-v7-0-7f1870dedefc@gmail.com> <20260821-cgroups-v7-2-7f1870dedefc@gmail.com> Date: Wed, 26 Aug 2026 16:07:01 +0200 Message-ID: <87y0dt7z7e.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: HN2SR6PH4BAER2ZCZFJMMNNNVBIDPIQB X-Message-ID-Hash: HN2SR6PH4BAER2ZCZFJMMNNNVBIDPIQB X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.10 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > This program has two modes: > > 1. Create a control group if it doesn't exist, optionally wait for other > programs in it to exit, and exec another program in it. > > 2. Purge a control group: kill all programs in it, then delete it. > > Locking is used to ensure that concurrent invocations are safe. > > Signed-off-by: Demi Marie Obenour The structure of the program is looking really good now. All remaining comments are minor, except for making sure we're doing the right thing with enabling controllers. Pay attention to naming =E2=80=94 good names are really important for making it clear to readers what a program does. > diff --git a/tools/cgroup-setup/src/cgroup.rs b/tools/cgroup-setup/src/cg= roup.rs > new file mode 100644 > index 0000000000000000000000000000000000000000..463d3fc87ee34ccfbf399a337= e2f49d9031728ee > --- /dev/null > +++ b/tools/cgroup-setup/src/cgroup.rs > @@ -0,0 +1,289 @@ > +// SPDX-FileCopyrightText: 2026 Demi Marie Obenour > +// SPDX-License-Identifier: EUPL-1.2+ > + > +use std::ffi::OsStr; > +use std::fs::File; > +use std::io::{Read as _, Seek as _, Write as _}; > +use std::os::unix::prelude::*; > + > +use std::path::{Component, Path, PathBuf}; > + > +use rustix::fs::{AtFlags, CWD, Dir, FlockOperation}; > +use rustix::path; > +use rustix::{ > + fs::{Mode, OFlags, ResolveFlags}, > + io::Errno, > +}; > + > +pub enum OpenFlags { > + Read, > + Write, > + Directory, > +} > + > +#[derive(Debug)] > +pub(crate) struct Cgroup { > + fd: Vec, > +} > + > +impl AsFd for Cgroup { > + fn as_fd(&self) -> BorrowedFd<'_> { > + self.fd.last().unwrap().as_fd() > + } > +} > + > +fn assert_single_component(component: &Path) { > + match component.as_os_str().as_bytes() { > + b"" | b"." | b".." =3D> panic!("bad component"), > + c if c.contains(&b'\0') =3D> panic!("NUL in component"), > + c if c.contains(&b'/') =3D> panic!("/ in component"), > + _ =3D> {} > + } > +} > + > +// Wrapper around openat2() with better defaults. > +pub fn openat2_simple( > + fd: impl AsFd, > + path: impl path::Arg, > + flags: OpenFlags, > +) -> Result { > + rustix::fs::openat2( > + fd.as_fd(), > + path, > + OFlags::CLOEXEC > + | match flags { > + OpenFlags::Read =3D> OFlags::RDONLY | OFlags::NOCTTY, > + OpenFlags::Write =3D> OFlags::WRONLY | OFlags::NOCTTY, > + OpenFlags::Directory =3D> OFlags::RDONLY | OFlags::DIREC= TORY, > + }, > + Mode::empty(), > + ResolveFlags::NO_SYMLINKS | ResolveFlags::NO_MAGICLINKS | Resolv= eFlags::NO_XDEV, > + ) > +} > + > +pub const DEFAULT_LEAF: &str =3D "$inner.service"; > + > +pub fn check_path(path: &Path) -> Result<(), String> { > + let bytes =3D path.as_os_str().as_bytes(); > + // Path::components() skips ., so use string manipulation instead. > + for component in bytes[path.is_absolute() as usize..].split(|&b| b = =3D=3D b'/') { > + if matches!(component, b"" | b"." | b"..") { > + return Err(format!("cgroup path {path:?} isn't canonical")); > + } > + } > + Ok(()) > +} > + > +// Remove all subdirectories of the given directory recursively, but not= the > +// directory itself. The directory file descriptor is closed. [nit] That's clear from the type signature, so probably doesn't need to be explicitly documented. > +// > +// This isn't the most efficient possible algorithm, but simplicity is m= ore > +// important than performance in this case. Also, it keeps open more fi= le > +// descriptors than strictly necessary, but Spectrum runs with a very hi= gh limit > +// for the number of open file descriptors, and it uses shallow control = group > +// hierarchies. > +// > +// This uses a recursive algorithm, but so does std::fs::remove_dir_all(= ). Trying > +// to be more robust than the standard library is not worthwhile. In pa= rticular, > +// the standard library function must be safe on systems where untrusted= users (or > +// even network endpoints!) can create deeply nested directory trees, wh= ereas in > +// Spectrum cgroups are only writeable by root. > +fn remove_recursively(mut dirfd: Dir, remaining_depth: usize) -> Result<= (), Errno> { > + if remaining_depth < 1 { > + panic!("control groups too deeply nested"); > + } > + while let Some(element) =3D dirfd.next() { "element" is a bit of an odd name for a directory entry, no? > + let parent_fd =3D dirfd.fd().unwrap(); Could be lifted out of the loop, right? > + let element =3D element.expect("Iterating through a cgroup direc= tory failed?"); > + let path =3D element.file_name(); "name" would probably be clearer than "path", since we know it's a single component (and it's consistent with the file_name method). > + if element.file_type() !=3D rustix::fs::FileType::Directory || p= ath =3D=3D c"." || path =3D=3D c".." { > + continue; > + } > + let fd =3D openat2_simple(parent_fd, path, OpenFlags::Directory)= ?; > + remove_recursively(Dir::new(fd).unwrap(), remaining_depth - 1)?; > + rustix::fs::unlinkat(parent_fd, path, AtFlags::REMOVEDIR)?; > + } > + Ok(()) > +} > + > +fn assert_simple_path(current_cgroup: &Path) { > + let current_cgroup =3D current_cgroup.as_os_str().as_bytes(); > + if !matches!(current_cgroup, b"" | b".") { > + for component in current_cgroup.split(|&b| b =3D=3D b'/') { > + assert_single_component(Path::new(OsStr::from_bytes(componen= t))); > + } > + } > +} Very non-obvious what this does =E2=80=94 of course you'll get a lot of "si= ngle component"s if you split a path on /. What you actually want to do is just check for no null bytes or .. components, right? Given assert_single_component is doing a more specific check than just single components, it should be renamed accordingly. (Although I'd struggle to think of a name, because I still find what it's checking, and where we check it, to be a bit arbitrary, especially when it's a path that's come from the kernel=E2=80=A6) With check_path as well we have a confusing collection of subtly different, inconsistently named path checking functions. These should be unified if possible, named systematically if not, and in either case it should be clear from the name what the function is for. > +// Convert the cgroup path to one relative to /sys/fs/cgroup. > +// > +// If the path starts with /, the leading / is removed and the result is= returned > +// without further processing. Otherwise, the current cgroup is read fr= om > +// /proc/thread-self/cgroup. If its last component is $inner.service, t= hat is > +// removed. Finally, the current cgroup is prepended to the provided cg= roup path, > +// with a single / as separator. The result of this operation is return= ed. > +fn prepend_current_cgroup_if_needed(path: &Path) -> Result { > + if let Ok(suffix) =3D path.strip_prefix("/") { > + return Ok(suffix.to_owned()); > + } > + assert_simple_path(path); > + // /proc/thread-self is the same as /proc/self, except for the curre= nt thread > + // instead of the initial thread. In this case, the two are identic= al, but > + // using /proc/thread-self is better practice as it is correct in mo= re cases. > + // Reading /proc/thread-self/cgroup should never fail unless the sys= tem is > + // seriously broken. > + let current_cgroup =3D > + std::fs::read("/proc/thread-self/cgroup").expect("cannot read /p= roc/thread-self/cgroup"); > + // Using this on a system without cgroups v2 mounted is user error > + // and not supported. > + let current_cgroup =3D current_cgroup > + .strip_prefix(b"0::/") > + .and_then(|e| e.strip_suffix(b"\n")) > + .ok_or_else(|| { > + "/proc/thread-self/cgroup doesn't start with 0::/ or doesn't= end with a newline.\n\ > + Either cgroups aren't in use at all, or you are using cgroup= s v1." > + .to_owned() > + })?; > + let mut current_cgroup =3D PathBuf::from(OsStr::from_bytes(current_c= group)); > + assert_simple_path(¤t_cgroup); > + // Strip the implied $inner.service suffix. > + // This is used to satisfy the "no internal processes" rule. > + if current_cgroup.ends_with(Path::new(DEFAULT_LEAF)) { > + assert!(current_cgroup.pop()); > + } > + // "." refers to the current cgroup. > + if path !=3D Path::new(".") { > + current_cgroup.push(path); > + } > + Ok(current_cgroup) > +} > + > +pub(crate) fn write_value(fd: &dyn AsFd, name: &Path, value: &[u8]) -> R= esult<(), String> { > + let fd =3D openat2_simple(fd, name, OpenFlags::Write) > + .map_err(|e| format!("Cannot open {name:?}: {e}"))?; > + File::from(fd).write_all(value).map_err(|e| { > + format!( > + "Cannot write {:?} to {name:?}: {e}", > + OsStr::from_bytes(value) > + ) > + }) > +} > + > +impl Cgroup { > + pub fn new(path: &Path) -> Result { > + let cgroup_root =3D rustix::fs::openat2( > + CWD, > + Path::new("/sys/fs/cgroup"), > + OFlags::CLOEXEC | OFlags::DIRECTORY | OFlags::RDONLY, > + Mode::empty(), > + ResolveFlags::NO_SYMLINKS | ResolveFlags::NO_MAGICLINKS, > + ) > + .map_err(|e| format!("Cannot open /sys/fs/cgroup: {e}"))?; > + // It's simpler to always have the root cgroup at the bottom of = the stack, > + // even though no lock needs to be taken on it. Otherwise, one = would need > + // to special-case the cgroup root. One could remove the first = element if > + // there is more than one element in the vector, but that's not = worth it. > + // cgroup-setup doesn't operate in an environment where FDs are = a limited > + // resource. > + let mut cgroup =3D Self { > + fd: vec![cgroup_root], > + }; > + > + let path =3D prepend_current_cgroup_if_needed(path)?; > + for component in path.components() { > + let Component::Normal(component) =3D component else { > + unreachable!() > + }; > + let sub_fd =3D openat2_simple(&cgroup, component, OpenFlags:= :Directory) > + .map_err(|e| format!("Cannot open sub-cgroup {component:= ?}: {e}"))?; > + // Take a shared lock on the cgroup. > + rustix::fs::flock(&sub_fd, FlockOperation::LockShared) > + .map_err(|e| format!("Cannot lock sub-cgroup {component:= ?}: {e}"))?; > + cgroup.fd.push(sub_fd); > + } > + Ok(cgroup) > + } > + > + pub fn wait_for_empty(fd: &dyn AsFd) -> std::io::Result<()> { > + let wait_file =3D openat2_simple(fd, c"cgroup.events", OpenFlags= ::Read)?; > + let mut wait_fd =3D File::from(wait_file); > + let mut v =3D vec![]; > + loop { > + v.clear(); > + wait_fd > + .seek(std::io::SeekFrom::Start(0)) > + .expect("Seek on control group file should succeed"); > + wait_fd > + .read_to_end(&mut v) > + .expect("reading from control group should work"); > + // Check that the cgroup isn't already empty. If it was, > + // the kernel would not send an event and poll() would wait > + // forever. > + if v.split(|&c| c =3D=3D b'\n').any(|line| line =3D=3D b"pop= ulated 0") { > + break; > + } > + let mut fds =3D libc::pollfd { > + fd: wait_fd.as_raw_fd(), > + events: libc::POLLPRI | libc::POLLERR, > + revents: 0, > + }; > + // SAFETY: FFI call, valid arguments, fds contains 1 element > + if unsafe { libc::poll(&raw mut fds, 1, -1) } !=3D 1 { > + panic!("poll failed"); > + } > + } > + Ok(()) > + } > + > + pub fn purge_child(&mut self, path: &Path) -> Result<(), String> { > + assert_single_component(path); > + // See if we can just delete the child directly. > + match rustix::fs::unlinkat(&self, path, AtFlags::REMOVEDIR) { > + // If the cgroup was successfully deleted, or if it > + // has already been deleted, we are done. > + Ok(()) | Err(Errno::NOENT) =3D> return Ok(()), > + // If this cgroup is in use, keep going. > + Err(Errno::BUSY) =3D> {} > + Err(e) =3D> return Err(format!("Cannot purge {path:?}: {e}")= ), > + } > + > + let sub_fd =3D match openat2_simple(&self, path, OpenFlags::Dire= ctory) { > + Ok(sub_fd) =3D> sub_fd, > + Err(Errno::NOENT) =3D> return Ok(()), > + Err(e) =3D> { > + return Err(format!("Cannot open sub-cgroup {path:?}: {e}= ",)); > + } > + }; > + > + // Take an exclusive lock on the cgroup that is about to be remo= ved. This > + // avoids concurrent executions of this program operating on del= eted > + // sub-cgroups. Dir::new() doesn't expose a reference to its in= ternal FD > + // so it must be delayed until later. Yes it does? It's Dir::fd. You used it elsewhere already. It's fine to delay Dir::new but this comment is not correct. > + rustix::fs::flock(&sub_fd, FlockOperation::LockExclusive) > + .map_err(|e| format!("Cannot lock sub-cgroup: {e}"))?; > + > + // Kill all processes in the child cgroup. > + write_value(&sub_fd, Path::new("cgroup.kill"), b"1")?; > + > + // Wait for the child cgroup to become empty. > + Self::wait_for_empty(&sub_fd) > + .map_err(|e| format!("Cannot wait for cgroup to become empty= : {e}")) > + .inspect_err(|_| { > + self.fd.pop().unwrap(); Why do we need to do this? What's the matching push? Why should failing to wait for a child cgroup to be empty mean we unlock its parent? > + })?; > + > + // Remove the child cgroup and its contents recursively. > + remove_recursively(Dir::new(sub_fd).unwrap(), 1000) > + .map_err(|e| format!("Cannot remove: {e}"))?; > + > + // Delete the cgroup. If it's been re-created in the meantime a= nd is > + // currently in use, this is not an error. Another process dele= ting the > + // cgroup is also not an error. Both of these can happen becaus= e of the > + // time period between remove_child_directories() closing the fi= le > + // descriptor (releasing its lock) and the above call to flock(). > + match rustix::fs::unlinkat(&self, path, AtFlags::REMOVEDIR) { > + Ok(()) | Err(Errno::BUSY) | Err(Errno::NOENT) =3D> Ok(()), > + Err(e) =3D> Err(format!("Cannot delete: {e}")), > + } > + } > +} > diff --git a/tools/cgroup-setup/src/main.rs b/tools/cgroup-setup/src/main= .rs > new file mode 100644 > index 0000000000000000000000000000000000000000..c757a4ad37c812ef5ce5249dc= 1bc3104ec246eef > --- /dev/null > +++ b/tools/cgroup-setup/src/main.rs > @@ -0,0 +1,168 @@ > +// SPDX-FileCopyrightText: 2026 Demi Marie Obenour > +// SPDX-License-Identifier: EUPL-1.2+ > + > +mod cgroup; > + > +use cgroup::{Cgroup, OpenFlags, openat2_simple, write_value}; > +use rustix::{ > + fs::{FlockOperation, Mode, XattrFlags}, > + io::Errno, > +}; > +use std::{ > + env::ArgsOs, > + fs::File, > + io::Read as _, > + os::unix::prelude::*, > + path::{Path, PathBuf}, > +}; > + > +fn enable_subtree_control(fd: &dyn AsFd) -> Result<(), String> { > + rustix::fs::fsetxattr(fd, c"user.delegate", b"1", XattrFlags::empty(= )) > + .map_err(|e| format!("Cannot enable cgroup delegation: {e}"))?; > + let mut buf =3D Vec::new(); > + File::from( > + openat2_simple(fd, c"cgroup.controllers", OpenFlags::Read) > + .map_err(|e| format!("Cannot open cgroup.controllers: {e}"))= ?, > + ) > + .read_to_end(&mut buf) > + .map_err(|e| format!("Cannot read cgroup.controllers: {e}"))?; > + let mut subtree =3D vec![]; > + if buf.is_empty() { > + return Ok(()); > + } > + for controller in buf.split(|&b| b =3D=3D b' ') { > + if !subtree.is_empty() { > + subtree.push(b' '); > + } > + subtree.push(b'+'); > + subtree.extend_from_slice(controller); > + } > + if !subtree.is_empty() { > + write_value(&fd, Path::new("cgroup.subtree_control"), &subtree)?; > + } > + Ok(()) > +} My memory of our conversation on a call last week is that we found it undesirable to enable every controller, since that causes behaviour surprising action-at-a-distance behaviour changes. Rather specific requested controllers should be enabled when necessary, right? > + > +fn spawn_in_cgroup( > + mut args: std::iter::Peekable, > + cgroup: Option<&dyn AsFd>, > +) -> Result<(), String> { We're not spawning anything if all we're doing is an exec. It should be called exec_in_cgroup. > + let Some(program_name) =3D args.next() else { > + return Ok(()); > + }; > + if let Some(cgroup) =3D cgroup { > + let pid =3D std::process::id().to_string(); > + write_value( > + cgroup, > + Path::new("$inner.service/cgroup.procs"), > + pid.as_bytes(), > + ) > + .map_err(|e| format!("Cannot move process to child cgroup: {e}")= )?; > + } > + let e =3D std::process::Command::new(&program_name).args(args).exec(= ); > + Err(format!("Cannot spawn child {program_name:?}: {e}")) Cannot *exec*. > +} > + > +// Check that the path is canonical, > +// then split it into basename and filename. > +fn split_path(path: &Path) -> Result<(&Path, &Path), String> { > + cgroup::check_path(path)?; > + Ok((path.parent().unwrap(), Path::new(path.file_name().unwrap()))) > +} > + > +fn cgroup_setup(args: ArgsOs) -> Result<(), String> { > + let mut wait =3D true; > + let mut args =3D args.peekable(); > + while let Some(arg) =3D args.peek() { > + if !arg.as_bytes().starts_with(b"-") { > + break; > + } > + let arg =3D args.next().unwrap(); I'd find let _ =3D args.next() slightly clearer, because then it's clear we're not interested in the value, since we already have it. > + let Some(option) =3D arg.as_bytes().strip_prefix(b"--") else { > + return Err("takes no short options".to_owned()); > + }; > + match option { > + b"" =3D> break, > + b"no-wait" =3D> wait =3D false, > + _ =3D> return Err(format!("unknown long option {arg:?}")), > + } > + } > + let Some(cgroup_path) =3D args.next().map(PathBuf::from) else { > + return Err("have no positional arguments, expected at least 1".t= o_owned()); > + }; > + > + let (parent_cgroup_path, child_cgroup_path) =3D split_path(&cgroup_p= ath)?; > + let cgroup =3D Cgroup::new(parent_cgroup_path)?; > + match rustix::fs::mkdirat(&cgroup, child_cgroup_path, Mode::from_raw= _mode(0o755)) { > + Ok(()) | Err(Errno::EXIST) =3D> {} > + Err(e) =3D> { > + return Err(format!( > + "Cannot create child cgroup {child_cgroup_path:?}: {e}" > + )); > + } > + } > + > + let child =3D openat2_simple(&cgroup, child_cgroup_path, OpenFlags::= Directory) > + .map_err(|e| format!("Cannot open child cgroup: {e}"))?; > + > + // While waiting, hold an exclusive lock on the child. > + // This avoids two processes both waiting for the same cgroup to bec= ome > + // empty, then spawning processes in the same cgroup. > + rustix::fs::flock(&child, FlockOperation::LockExclusive) > + .map_err(|e| format!("Cannot take an exclusive lock on child cgr= oup: {e}"))?; > + if wait { > + Cgroup::wait_for_empty(&child) > + .map_err(|e| format!("Cannot wait for {parent_cgroup_path:?}= to be empty: {e}"))?; > + } > + > + // Spectrum's programs (such as this one) expect cgroup.subtree_cont= rol > + // to be set by the program that created the cgroup. systemd-aware > + // programs, like systemd-udevd, expect user.delegate=3D1 to be set. > + enable_subtree_control(&child)?; > + > + // If the child process will need to manage cgroups itself, it will = need > + // to set up a sub-cgroup due to the "no internal processes" rule. = It's > + // simplest to just do it automatically. If the cgroup already exis= ts, > + // that isn't an error. > + match rustix::fs::mkdirat(&child, cgroup::DEFAULT_LEAF, Mode::from_r= aw_mode(0o755)) { > + Ok(()) | Err(Errno::EXIST) =3D> {} > + Err(e) =3D> return Err(format!("Cannot create $inner.service cgr= oup: {e}")), > + } > + > + spawn_in_cgroup(args, Some(&child)) > +} > + > +fn cgroup_purge(mut args: ArgsOs) -> Result<(), String> { > + if args.len() !=3D 1 { > + return Err("usage: cgroup-purge CGROUP_TO_PURGE".to_owned()); > + } > + let arg =3D args.next().unwrap(); > + let (parent, child) =3D split_path(Path::new(&arg))?; > + Cgroup::new(parent)?.purge_child(child) > +} > + > +fn run(prog_name: &Path, args: ArgsOs) -> Result<(), String> { > + match prog_name.file_name().map(|f| f.as_bytes()) { > + Some(b"cgroup-setup") =3D> cgroup_setup(args), > + Some(b"cgroup-purge") =3D> cgroup_purge(args), > + _ =3D> Err(format!( > + "must be invoked as \"cgroup-setup\" or \ > + \"cgroup-purge\", got {prog_name:?}", > + )), > + } > +} > + > +fn main() { > + let mut args =3D std::env::args_os(); > + let Some(prog_name) =3D args.next() else { > + eprintln!("No command line arguments (argv[0] is NULL)"); > + std::process::exit(1); > + }; > + match run(Path::new(&prog_name), args) { > + Ok(()) =3D> {} > + Err(e) =3D> { > + eprintln!("{prog_name:?}: {}", e); > + std::process::exit(1); > + } > + } > +} --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCao7zBQAKCRCZddwkt31p Fa6zAP4sk8UIVEMJ4XYH/YME3cMu5lw3g99xFRbT9kFO76nK8wD/SmGgjgmhpc1k wn2h84ri4jncF8IE96XI7WRQAgzWGgQ= =7TRO -----END PGP SIGNATURE----- --=-=-=--