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 BE9F6ADAB; Thu, 30 Jul 2026 14:53:42 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 700F8AD57; Thu, 30 Jul 2026 14:53:39 +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.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,RCVD_IN_MSPIKE_H2, SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fhigh-b3-smtp.messagingengine.com (fhigh-b3-smtp.messagingengine.com [202.12.124.154]) by atuin.qyliss.net (Postfix) with ESMTPS id 76420AD56 for ; Thu, 30 Jul 2026 14:53:37 +0000 (UTC) Received: from phl-compute-09.internal (phl-compute-09.internal [10.202.2.49]) by mailfhigh.stl.internal (Postfix) with ESMTP id 55AB87A009C; Thu, 30 Jul 2026 10:53:35 -0400 (EDT) Received: from phl-frontend-03 ([10.202.2.162]) by phl-compute-09.internal (MEProxy); Thu, 30 Jul 2026 10:53:35 -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=fm1; t=1785423215; x=1785509615; bh=QmgdBavhaf 3Jov1nzww5W/6TP0URo2EKNQK+6lxQkgo=; b=IrqglPjCSmy7jKa7OFkLqVlJA0 DOTjWKyHw1BVgqrokss2ecroHD+NPKakWD2W26M+bSMbb/sChfDBdrL4Ss4ddJfB 7oNKzItZfqvXAcpiDkwJKl+iDKSIBAq4WiQzaVq8xQTviwYYM4HMGkyUD/N2h8sC e+CZtxFPGk4mQGAp+owHKaUYwaHEN4vYIwBYuBq51qYYVLFPUie3MlqHsag9Pt9j EJwAUOfu1LnNmsOgp+RoMjZZ70h9yWuHTc/9O5QmmwC1+GYaQlNyy1AMlXmcr1mX HwmKAEbqgv88y1smcR+FL3lFmp/Fa9oEQLYcYQuLftAxLANfGoEyRFhbZuGA== 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=fm2; t= 1785423215; x=1785509615; bh=QmgdBavhaf3Jov1nzww5W/6TP0URo2EKNQK +6lxQkgo=; b=oFWtOkM67TUORvOavFBMmP5fZ0yqt1TzMWEQBTouBoUSAXg/iAk JiE/A2EDuDuSI6qfuxKifTr7s47BxeLYGwGr5UlsPHa80C/bvb0rkNrz4xFwG1Sn ofICSftCtOTzg5hD3pItFAua7+4aQguDsM94mAYEXQ9GIA7IbKOW1XKAfLJh5+/G g59KCLw1VrNolcrkUX7rxbqFqCHmWZP6M26ShFVA4F5fAHB05yv68P3oqYUzrKvv BOvP0m6eGrXw7rdK7mGUXPA+9Fvaqy4sdRRLSQlOsPj/apm+Z0KdFNKG91dJ9jqb 333tvZbDmDZeifwKV8iUODp8Ic9qRLYMlog== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: dmFkZTGZQ/QY48xsRa24MND7DtueAtjZLKrWaSoRBWWzesGx7LMcIS0GaZH89L+cVKEO68 iqk1cfEthgO1yqlSFIaQzpgUWhXanA1JMMr7g8NXY/KIYKjBhEKyx5OCMQ/G3G1bMO4vh4 Qyjfg6BFyZAkxlyMvfWzEFwH3TQa61Aa16nc/EvwRRb0979CbEHSSXIryF0TN1/5+mquhB 7FHnhFe5iqeotBxGZjkJIUwZEhK7/ZDmJUupXcpW3PotzbaoR/JO/FVz1/65R9ac/jzsNq myV4X205uhsgSgUuagHfVHpBGA6IsGKZ7lM9R7MAsxxcOcwI/kkxaavT26wkJ3DE1kpln1 H8p32eEN4CJmHZb4S3nphcevMkYS8ESyK0XFGeBOnXxoSMyYZsSp1Gb3aF9GmGwlCIaAnY x/tmR6YDrC9Qu0GZKmUA4eRMHXkcLEmdgHLTqXO1v+y72VD10rSWD+uBDikL1cipQCpDB5 iwM0p+irNtw+HDm/8Q8QZQ7nt/ss5N4Iko4aFxuHGDYedpfGoJm8MMxD++qjcCHhjWFdTe chQ0wfsWFm6O39GTKrITjo3DCjRLmjwucbBdwlTvaK5L82NxatS83yUTeQakldeW+nT+tn 0mpIAOHA1gCq7XYdAPJydYDm5dXNbJLQbH2GCessyLUBP0wA4nmjFlduEFpw X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 30 Jul 2026 10:53:34 -0400 (EDT) Received: by mbp.qyliss.net (Postfix, from userid 1000) id 6842A8AFA61D; Thu, 30 Jul 2026 16:53:32 +0200 (CEST) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v4 02/20] tools: Add control group manager In-Reply-To: <295f55ea-f5bd-49e0-8eb6-3a6aab9e981f@gmail.com> References: <20260721-cgroups-v4-0-46b2e5fff7b6@gmail.com> <20260721-cgroups-v4-2-46b2e5fff7b6@gmail.com> <87a4rjrp2t.fsf@alyssa.is> <87cxw8ljkh.fsf@alyssa.is> <295f55ea-f5bd-49e0-8eb6-3a6aab9e981f@gmail.com> Date: Thu, 30 Jul 2026 16:53:31 +0200 Message-ID: <87h5lgv89g.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: Y6FGADXBGNGO7CW2UZGYE6FFHQHGCZMV X-Message-ID-Hash: Y6FGADXBGNGO7CW2UZGYE6FFHQHGCZMV 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 Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > On 7/27/26 08:10, Alyssa Ross wrote: >> Demi Marie Obenour writes: >>=20 >>> On 7/22/26 12:01, Alyssa Ross wrote: >>>> Demi Marie Obenour writes: >>=20 >>>>> +fn remove_recursively(fd: OwnedFd, remaining_depth: usize) -> Result= <(), Errno> { >>>>> + if remaining_depth < 1 { >>>>> + panic!("control groups too deeply nested"); >>>>> + } >>>>> + let mut d =3D rustix::fs::Dir::new(fd).expect("cannot start iter= ating"); >>>>> + while let Some(element) =3D d.next() { >>>>> + let element =3D element.expect("Iterating through a cgroup d= irectory failed?"); >>>>> + if element.file_type() !=3D rustix::fs::FileType::Directory { >>>>> + continue; >>>>> + } >>>>> + >>>>> + let remaining_depth =3D remaining_depth - 1; >>>>> + let d: &rustix::fs::Dir =3D &d; >>>>> + let dirfd =3D d.fd().unwrap(); >>>>> + let path =3D element.file_name(); >>>>> + remove_all(remaining_depth, dirfd, path)?; >>=20 >> I'd inline these variables into the function call the extent possible. >> Otherwise I have to follow a lot of shuffling around. > > Will fix in v5. > >>>>> + } >>>>> + drop(d); >>>>> + Ok(()) >>>>> +} >>>>> + >>>>> +fn remove_all( >>>>> + remaining_depth: usize, >>>>> + dirfd: BorrowedFd<'_>, >>>>> + path: &std::ffi::CStr, >>>>> +) -> Result<(), Errno> { >>>>> + if path =3D=3D c"." || path =3D=3D c".." { >>>>> + return Ok(()); >>>>> + } >>=20 >> It's a bit weird that calling remove_all on . or .. does not fail. >> Maybe would be clearer to move this check to the call site? > > Will fix in v5, making the code simpler. > >>>>> + if rustix::fs::unlinkat(dirfd, path, AtFlags::REMOVEDIR).is_ok()= { >>>>> + return Ok(()); >>>>> + } >>=20 >> We could drop this, right? A few extra syscalls, but less to wrap my >> head around. > > Correct. > >>>>> + let fd =3D rustix::fs::openat2( >>>>> + dirfd, >>>>> + path, >>>>> + OFlags::CLOEXEC | OFlags::NOFOLLOW | OFlags::RDONLY | OFlags= ::DIRECTORY, >>>>> + Mode::empty(), >>>>> + ResolveFlags::NO_SYMLINKS | ResolveFlags::BENEATH | ResolveF= lags::NO_XDEV, >>>>> + )?; >>>>> + remove_recursively(fd, remaining_depth)?; >>>>> + rustix::fs::unlinkat(dirfd, path, AtFlags::REMOVEDIR)?; >>>>> + Ok(()) >>>>> +} >>>> >>>> Could we save a lot of code by calling std::fs::remove_dir_all with a >>>> /proc/self/fd path? It's already documented to ignore symlinks. >>> >>> I tried, but that tries to delete files too, and that isn't allowed. >>=20 >> Ah, alright. That could use an explanatory comment. >>=20 >> How can we be confident we have enough stack for this? Is there a way >> it could be done non-recursively, with state on the heap? > > In Spectrum, the depth is currently limited to a small constant. Right, but that's really papering over the problem, and our stack is also limited to a small constant size. We could avoid the problem entirely by instead running paths to operate on through a VecDeque rather than recursing, and then we wouldn't need any arbitrary limit. >>>> Last time I suggested a clearer way of doing this, but it has instead >>>> got even less clear. >>>> >>>> (I'm not sure why we'd care if there's a newline specifically, as >>>> opposed to any other control character.) >>> >>> If cgroups v1 is in use, the file can contain multiple lines, one for >>> each cgroup the program is in. I also am not sure if starting with >>> "0::/" is an invariant in that case. >>> >>> Using this program with cgroups v1 mounted is user error and will >>> never happen on Spectrum, but if this tool is used outside of Spectrum, >>> it could happen. >>=20 >> I see. It seems like with cgroups v1, it _could_ start with 0::/, but >> probably wouldn't. I think it may not be possible to tell from this >> file whether cgroups v1 is in use. >>=20 >> So I suppose it depends what you want to happen if cgroups v1 is in use. >> If it looks enough like cgroups v2, do you continue, or do you >> explicitly check for cgroups v1? If the latter (sounds more sensible to >> me), you need to explicitly check for cgroups v1 somehow I think. Can >> cgroups v1 and v2 be in use at the same time? If so, checking might be >> complicated, but if not, you can just check what type of filesystem is >> mounted at /sys/fs/cgroup, or see if it has a >> /sys/fs/cgroup/cgroup.controllers file. > > In the case of Spectrum, I think it's okay to just panic if cgroups v2 > isn't mounted or isn't working properly. It's a bug in either > cgroup-setup or the kernel, almost certainly the former. Yeah I agree it's completely fine for new stuff to requrie cgroups v2. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCamtlawAKCRBbRZGEIw/w ohFPAQCTd3tOdZ7FRV/+iMFvOAmC0BcX9x3ZvOZpkNa3K7VwPAEA+qQv/k+YJhfz JStcgazs++lSYZHn60DXCCSjqqjvaQw= =7Uz4 -----END PGP SIGNATURE----- --=-=-=--