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 9CEA88850; Wed, 05 Aug 2026 01:36:13 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id A766B883F; Wed, 05 Aug 2026 01:36:11 +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_PASS,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE autolearn=unavailable autolearn_force=no version=4.0.1 Received: from mail-yw1-x112d.google.com (mail-yw1-x112d.google.com [IPv6:2607:f8b0:4864:20::112d]) by atuin.qyliss.net (Postfix) with ESMTPS id 52BB9883D for ; Wed, 05 Aug 2026 01:36:07 +0000 (UTC) Received: by mail-yw1-x112d.google.com with SMTP id 00721157ae682-8201447e8cdso7271487b3.3 for ; Tue, 04 Aug 2026 18:36:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1785893764; x=1786498564; darn=spectrum-os.org; h=content-type:autocrypt:in-reply-to:content-language:references:cc :to:subject:from:user-agent:mime-version:date:message-id:from:to:cc :subject:date:message-id:reply-to:content-type; bh=OMLHfbJWW1Gaj/CYv60shf/ndjkFH+d1q+cg1zOy9aE=; b=PbBrpYE+F5wlUIRsNVu05TK2oKH7a4cm0r1YApT6N06+KbcVgAkxs5r6WDCTX7lcVv tfQblz4kmiaJe7FJP21qcHS3glPxtQbmiyi4rQu1xK4qfKgkBNorskNOGKkqgFrRYG2t ksVc4Se9r5dDaK5GbZwxwAh7TtWChfyuyAfkgGHnMjLMOkW3NfqLLQgMrB6JR6+zal23 pzRdpxq2Li+EJTk2MLsd+hcLAECSpi3C5CVGgC/ENmo1mLIPPHZFif6Q825VmCKLArJa T4eUKMEyJR3YlZC5xj2kamaLf6ZRM6XIAoziBav6ysJ9EJ+F47uGRzhboGKwKiN5ZCEz drxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785893764; x=1786498564; h=content-type:autocrypt:in-reply-to:content-language:references:cc :to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=OMLHfbJWW1Gaj/CYv60shf/ndjkFH+d1q+cg1zOy9aE=; b=SF/3sVwGiA4Rhzvs2B20TnYfyMDYPf+EfyTQKGQmlUQPxSfSOUxUW4jQva+QjqIKAK QDeRq3pxRdTSNk55wOPL3f/clQdHHK7OcisT1SRjE3BxKt4ojsdTqKtWPg8joS+mj37F Kbr0fluG1fvDFW7msapmUhCcX8NwwUE9XX1IBDO6Gb4IYdWmUmACwyjmGpKWg7Qv3N6M evPZ2qPeWWlqEiEiIibAHcH4UyAv6+SjhQqDGtaNX0VD4ZyB2hWXSE2h7EjIuHqONMCt ymg/dXE6JVNN/6v+6JvIwSAEAY9aPbRV47yb7rO7BV7F282rstUQV+t//0zXEGM2eMlo hVHA== X-Gm-Message-State: AOJu0YxzXtX0QfXzVd7xUa1VrPsd4PPBmSauotjaDBHLiogR6yTPEslf MNpum8Yaw3pONCtyTrqKy3yUCUqe38IXeXFKJI5s4N4olUTrLXiliJEt X-Gm-Gg: AR+sD13FDpesTxrEtHrVy0ycw8pimaQ1/zTXSqt4odOmamSI0C1S+8qRl4pG0kB9Wou LyHODCpRkhcvRSL2KTryXnfwb5lvtKOPsw6hVjYi1HfQbkL2nbSIdgpdcr6UvLxk99KZvXc92sG SzpEKR5tS1iOZfWmi2iWwEthYVbE6ThAgAxEBXpgF6cIrmZfDXuz4KGXDZbOQT0xFHnEmNhpjey DO4PSGoITnQSjFwY0xYdwtdpWasfnAhLE0t2NooBZSwfyfrBrr6TtU1esw2k0WbXK2UGQ34KikG a0kV7EvPWbWaBhxojPPAN9KNycVUNLdFumwuCc7COlzZ7fmFlajrw3m0pUGwKu6g85XuFZMnZXI CYhRCUxFwmuxeWoUc1CWFiUJpVqDZ7M3meY8okcAAB48D9cdEJMJzlepOmoKXaC3XQmIxTJ/8eS TTDZBJGyqjmOrIHbe063w2tZhp95SVPd9IE7T/2cjCJSgz9SRqD/oIfS/DKH/qVX8= X-Received: by 2002:a05:690c:6c81:b0:80b:bfee:c426 with SMTP id 00721157ae682-8201f3acb3bmr18146147b3.14.1785893764052; Tue, 04 Aug 2026 18:36:04 -0700 (PDT) Received: from [10.138.10.6] ([185.98.168.14]) by smtp.gmail.com with ESMTPSA id 00721157ae682-82013c49127sm13806057b3.32.2026.08.04.18.36.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 04 Aug 2026 18:36:02 -0700 (PDT) Message-ID: <2ce7b61d-faad-49b1-9f15-019140e2dca1@gmail.com> Date: Tue, 4 Aug 2026 21:36:00 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Demi Marie Obenour Subject: Re: [PATCH v5 02/19] tools: Add control group manager To: Alyssa Ross References: <20260731-cgroups-v5-0-b325bac9d34f@gmail.com> <20260731-cgroups-v5-2-b325bac9d34f@gmail.com> Content-Language: en-US In-Reply-To: Autocrypt: addr=demiobenour@gmail.com; keydata= xsFNBFp+A0oBEADffj6anl9/BHhUSxGTICeVl2tob7hPDdhHNgPR4C8xlYt5q49yB+l2nipd aq+4Gk6FZfqC825TKl7eRpUjMriwle4r3R0ydSIGcy4M6eb0IcxmuPYfbWpr/si88QKgyGSV Z7GeNW1UnzTdhYHuFlk8dBSmB1fzhEYEk0RcJqg4AKoq6/3/UorR+FaSuVwT7rqzGrTlscnT DlPWgRzrQ3jssesI7sZLm82E3pJSgaUoCdCOlL7MMPCJwI8JpPlBedRpe9tfVyfu3euTPLPx wcV3L/cfWPGSL4PofBtB8NUU6QwYiQ9Hzx4xOyn67zW73/G0Q2vPPRst8LBDqlxLjbtx/WLR 6h3nBc3eyuZ+q62HS1pJ5EvUT1vjyJ1ySrqtUXWQ4XlZyoEFUfpJxJoN0A9HCxmHGVckzTRl 5FMWo8TCniHynNXsBtDQbabt7aNEOaAJdE7to0AH3T/Bvwzcp0ZJtBk0EM6YeMLtotUut7h2 Bkg1b//r6bTBswMBXVJ5H44Qf0+eKeUg7whSC9qpYOzzrm7+0r9F5u3qF8ZTx55TJc2g656C 9a1P1MYVysLvkLvS4H+crmxA/i08Tc1h+x9RRvqba4lSzZ6/Tmt60DPM5Sc4R0nSm9BBff0N m0bSNRS8InXdO1Aq3362QKX2NOwcL5YaStwODNyZUqF7izjK4QARAQABzTxEZW1pIE1hcmll IE9iZW5vdXIgKGxvdmVyIG9mIGNvZGluZykgPGRlbWlvYmVub3VyQGdtYWlsLmNvbT7CwXgE EwECACIFAlp+A0oCGwMGCwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJELKItV//nCLBhr8Q AK/xrb4wyi71xII2hkFBpT59ObLN+32FQT7R3lbZRjVFjc6yMUjOb1H/hJVxx+yo5gsSj5LS 9AwggioUSrcUKldfA/PKKai2mzTlUDxTcF3vKx6iMXKA6AqwAw4B57ZEJoMM6egm57TV19kz PMc879NV2nc6+elaKl+/kbVeD3qvBuEwsTe2Do3HAAdrfUG/j9erwIk6gha/Hp9yZlCnPTX+ VK+xifQqt8RtMqS5R/S8z0msJMI/ajNU03kFjOpqrYziv6OZLJ5cuKb3bZU5aoaRQRDzkFIR 6aqtFLTohTo20QywXwRa39uFaOT/0YMpNyel0kdOszFOykTEGI2u+kja35g9TkH90kkBTG+a EWttIht0Hy6YFmwjcAxisSakBuHnHuMSOiyRQLu43ej2+mDWgItLZ48Mu0C3IG1seeQDjEYP tqvyZ6bGkf2Vj+L6wLoLLIhRZxQOedqArIk/Sb2SzQYuxN44IDRt+3ZcDqsPppoKcxSyd1Ny 2tpvjYJXlfKmOYLhTWs8nwlAlSHX/c/jz/ywwf7eSvGknToo1Y0VpRtoxMaKW1nvH0OeCSVJ itfRP7YbiRVc2aNqWPCSgtqHAuVraBRbAFLKh9d2rKFB3BmynTUpc1BQLJP8+D5oNyb8Ts4x Xd3iV/uD8JLGJfYZIR7oGWFLP4uZ3tkneDfYzsFNBFp+A0oBEAC9ynZI9LU+uJkMeEJeJyQ/ 8VFkCJQPQZEsIGzOTlPnwvVna0AS86n2Z+rK7R/usYs5iJCZ55/JISWd8xD57ue0eB47bcJv VqGlObI2DEG8TwaW0O0duRhDgzMEL4t1KdRAepIESBEA/iPpI4gfUbVEIEQuqdqQyO4GAe+M kD0Hy5JH/0qgFmbaSegNTdQg5iqYjRZ3ttiswalql1/iSyv1WYeC1OAs+2BLOAT2NEggSiVO txEfgewsQtCWi8H1SoirakIfo45Hz0tk/Ad9ZWh2PvOGt97Ka85o4TLJxgJJqGEnqcFUZnJJ riwoaRIS8N2C8/nEM53jb1sH0gYddMU3QxY7dYNLIUrRKQeNkF30dK7V6JRH7pleRlf+wQcN fRAIUrNlatj9TxwivQrKnC9aIFFHEy/0mAgtrQShcMRmMgVlRoOA5B8RTulRLCmkafvwuhs6 dCxN0GNAORIVVFxjx9Vn7OqYPgwiofZ6SbEl0hgPyWBQvE85klFLZLoj7p+joDY1XNQztmfA rnJ9x+YV4igjWImINAZSlmEcYtd+xy3Li/8oeYDAqrsnrOjb+WvGhCykJk4urBog2LNtcyCj kTs7F+WeXGUo0NDhbd3Z6AyFfqeF7uJ3D5hlpX2nI9no/ugPrrTVoVZAgrrnNz0iZG2DVx46 x913pVKHl5mlYQARAQABwsFfBBgBAgAJBQJafgNKAhsMAAoJELKItV//nCLBwNIP/AiIHE8b oIqReFQyaMzxq6lE4YZCZNj65B/nkDOvodSiwfwjjVVE2V3iEzxMHbgyTCGA67+Bo/d5aQGj gn0TPtsGzelyQHipaUzEyrsceUGWYoKXYyVWKEfyh0cDfnd9diAm3VeNqchtcMpoehETH8fr RHnJdBcjf112PzQSdKC6kqU0Q196c4Vp5HDOQfNiDnTf7gZSj0BraHOByy9LEDCLhQiCmr+2 E0rW4tBtDAn2HkT9uf32ZGqJCn1O+2uVfFhGu6vPE5qkqrbSE8TG+03H8ecU2q50zgHWPdHM OBvy3EhzfAh2VmOSTcRK+tSUe/u3wdLRDPwv/DTzGI36Kgky9MsDC5gpIwNbOJP2G/q1wT1o Gkw4IXfWv2ufWiXqJ+k7HEi2N1sree7Dy9KBCqb+ca1vFhYPDJfhP75I/VnzHVssZ/rYZ9+5 1yDoUABoNdJNSGUYl+Yh9Pw9pE3Kt4EFzUlFZWbE4xKL/NPno+z4J9aWemLLszcYz/u3XnbO vUSQHSrmfOzX3cV4yfmjM5lewgSstoxGyTx2M8enslgdXhPthZlDnTnOT+C+OTsh8+m5tos8 HQjaPM01MKBiAqdPgksm1wu2DrrwUi6ChRVTUBcj6+/9IJ81H2P2gJk3Ls3AVIxIffLoY34E +MYSfkEjBz0E8CLOcAw7JIwAaeBT Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------yQCTqYiU2OPL9Rn5eHCgZrFt" Message-ID-Hash: XMDRU3LLFIOFUE5OR63LFPWNBQBM6B6F X-Message-ID-Hash: XMDRU3LLFIOFUE5OR63LFPWNBQBM6B6F X-MailFrom: demiobenour@gmail.com X-Mailman-Rule-Hits: member-moderation 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 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: This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --------------yQCTqYiU2OPL9Rn5eHCgZrFt Content-Type: multipart/mixed; boundary="------------X5WsLE22LCLuMWI4kBORF0P5"; protected-headers="v1"; hp="clear" Message-ID: <2ce7b61d-faad-49b1-9f15-019140e2dca1@gmail.com> Date: Tue, 4 Aug 2026 21:36:00 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Demi Marie Obenour Subject: Re: [PATCH v5 02/19] tools: Add control group manager To: Alyssa Ross Cc: Spectrum OS Development References: <20260731-cgroups-v5-0-b325bac9d34f@gmail.com> <20260731-cgroups-v5-2-b325bac9d34f@gmail.com> Content-Language: en-US In-Reply-To: Autocrypt: addr=demiobenour@gmail.com; keydata= xsFNBFp+A0oBEADffj6anl9/BHhUSxGTICeVl2tob7hPDdhHNgPR4C8xlYt5q49yB+l2nipd aq+4Gk6FZfqC825TKl7eRpUjMriwle4r3R0ydSIGcy4M6eb0IcxmuPYfbWpr/si88QKgyGSV Z7GeNW1UnzTdhYHuFlk8dBSmB1fzhEYEk0RcJqg4AKoq6/3/UorR+FaSuVwT7rqzGrTlscnT DlPWgRzrQ3jssesI7sZLm82E3pJSgaUoCdCOlL7MMPCJwI8JpPlBedRpe9tfVyfu3euTPLPx wcV3L/cfWPGSL4PofBtB8NUU6QwYiQ9Hzx4xOyn67zW73/G0Q2vPPRst8LBDqlxLjbtx/WLR 6h3nBc3eyuZ+q62HS1pJ5EvUT1vjyJ1ySrqtUXWQ4XlZyoEFUfpJxJoN0A9HCxmHGVckzTRl 5FMWo8TCniHynNXsBtDQbabt7aNEOaAJdE7to0AH3T/Bvwzcp0ZJtBk0EM6YeMLtotUut7h2 Bkg1b//r6bTBswMBXVJ5H44Qf0+eKeUg7whSC9qpYOzzrm7+0r9F5u3qF8ZTx55TJc2g656C 9a1P1MYVysLvkLvS4H+crmxA/i08Tc1h+x9RRvqba4lSzZ6/Tmt60DPM5Sc4R0nSm9BBff0N m0bSNRS8InXdO1Aq3362QKX2NOwcL5YaStwODNyZUqF7izjK4QARAQABzTxEZW1pIE1hcmll IE9iZW5vdXIgKGxvdmVyIG9mIGNvZGluZykgPGRlbWlvYmVub3VyQGdtYWlsLmNvbT7CwXgE EwECACIFAlp+A0oCGwMGCwkIBwMCBhUIAgkKCwQWAgMBAh4BAheAAAoJELKItV//nCLBhr8Q AK/xrb4wyi71xII2hkFBpT59ObLN+32FQT7R3lbZRjVFjc6yMUjOb1H/hJVxx+yo5gsSj5LS 9AwggioUSrcUKldfA/PKKai2mzTlUDxTcF3vKx6iMXKA6AqwAw4B57ZEJoMM6egm57TV19kz PMc879NV2nc6+elaKl+/kbVeD3qvBuEwsTe2Do3HAAdrfUG/j9erwIk6gha/Hp9yZlCnPTX+ VK+xifQqt8RtMqS5R/S8z0msJMI/ajNU03kFjOpqrYziv6OZLJ5cuKb3bZU5aoaRQRDzkFIR 6aqtFLTohTo20QywXwRa39uFaOT/0YMpNyel0kdOszFOykTEGI2u+kja35g9TkH90kkBTG+a EWttIht0Hy6YFmwjcAxisSakBuHnHuMSOiyRQLu43ej2+mDWgItLZ48Mu0C3IG1seeQDjEYP tqvyZ6bGkf2Vj+L6wLoLLIhRZxQOedqArIk/Sb2SzQYuxN44IDRt+3ZcDqsPppoKcxSyd1Ny 2tpvjYJXlfKmOYLhTWs8nwlAlSHX/c/jz/ywwf7eSvGknToo1Y0VpRtoxMaKW1nvH0OeCSVJ itfRP7YbiRVc2aNqWPCSgtqHAuVraBRbAFLKh9d2rKFB3BmynTUpc1BQLJP8+D5oNyb8Ts4x Xd3iV/uD8JLGJfYZIR7oGWFLP4uZ3tkneDfYzsFNBFp+A0oBEAC9ynZI9LU+uJkMeEJeJyQ/ 8VFkCJQPQZEsIGzOTlPnwvVna0AS86n2Z+rK7R/usYs5iJCZ55/JISWd8xD57ue0eB47bcJv VqGlObI2DEG8TwaW0O0duRhDgzMEL4t1KdRAepIESBEA/iPpI4gfUbVEIEQuqdqQyO4GAe+M kD0Hy5JH/0qgFmbaSegNTdQg5iqYjRZ3ttiswalql1/iSyv1WYeC1OAs+2BLOAT2NEggSiVO txEfgewsQtCWi8H1SoirakIfo45Hz0tk/Ad9ZWh2PvOGt97Ka85o4TLJxgJJqGEnqcFUZnJJ riwoaRIS8N2C8/nEM53jb1sH0gYddMU3QxY7dYNLIUrRKQeNkF30dK7V6JRH7pleRlf+wQcN fRAIUrNlatj9TxwivQrKnC9aIFFHEy/0mAgtrQShcMRmMgVlRoOA5B8RTulRLCmkafvwuhs6 dCxN0GNAORIVVFxjx9Vn7OqYPgwiofZ6SbEl0hgPyWBQvE85klFLZLoj7p+joDY1XNQztmfA rnJ9x+YV4igjWImINAZSlmEcYtd+xy3Li/8oeYDAqrsnrOjb+WvGhCykJk4urBog2LNtcyCj kTs7F+WeXGUo0NDhbd3Z6AyFfqeF7uJ3D5hlpX2nI9no/ugPrrTVoVZAgrrnNz0iZG2DVx46 x913pVKHl5mlYQARAQABwsFfBBgBAgAJBQJafgNKAhsMAAoJELKItV//nCLBwNIP/AiIHE8b oIqReFQyaMzxq6lE4YZCZNj65B/nkDOvodSiwfwjjVVE2V3iEzxMHbgyTCGA67+Bo/d5aQGj gn0TPtsGzelyQHipaUzEyrsceUGWYoKXYyVWKEfyh0cDfnd9diAm3VeNqchtcMpoehETH8fr RHnJdBcjf112PzQSdKC6kqU0Q196c4Vp5HDOQfNiDnTf7gZSj0BraHOByy9LEDCLhQiCmr+2 E0rW4tBtDAn2HkT9uf32ZGqJCn1O+2uVfFhGu6vPE5qkqrbSE8TG+03H8ecU2q50zgHWPdHM OBvy3EhzfAh2VmOSTcRK+tSUe/u3wdLRDPwv/DTzGI36Kgky9MsDC5gpIwNbOJP2G/q1wT1o Gkw4IXfWv2ufWiXqJ+k7HEi2N1sree7Dy9KBCqb+ca1vFhYPDJfhP75I/VnzHVssZ/rYZ9+5 1yDoUABoNdJNSGUYl+Yh9Pw9pE3Kt4EFzUlFZWbE4xKL/NPno+z4J9aWemLLszcYz/u3XnbO vUSQHSrmfOzX3cV4yfmjM5lewgSstoxGyTx2M8enslgdXhPthZlDnTnOT+C+OTsh8+m5tos8 HQjaPM01MKBiAqdPgksm1wu2DrrwUi6ChRVTUBcj6+/9IJ81H2P2gJk3Ls3AVIxIffLoY34E +MYSfkEjBz0E8CLOcAw7JIwAaeBT --------------X5WsLE22LCLuMWI4kBORF0P5 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 8/3/26 08:47, Alyssa Ross wrote: > Demi Marie Obenour writes: >=20 >> The cgroup-setup Rust program can create and purge cgroups. It can al= so >> wait for one to become empty, spawn a program in a cgroup, and more. = In >> the future, it will also support cgroup-based resource control. Locki= ng >> is used to ensure that concurrent invocations are safe. >> >> Signed-off-by: Demi Marie Obenour >> --- >> .codespellrc | 2 +- >> host/rootfs/default.nix | 6 +- >> pkgs/default.nix | 1 + >> tools/cgroup-setup/Cargo.lock | 67 ++++++++ >> tools/cgroup-setup/Cargo.lock.license | 2 + >> tools/cgroup-setup/Cargo.toml | 10 ++ >> tools/cgroup-setup/default.nix | 22 +++ >> tools/cgroup-setup/src/cgroup.rs | 308 +++++++++++++++++++++++++= +++++++++ >> tools/cgroup-setup/src/main.rs | 186 ++++++++++++++++++++ >> 9 files changed, 600 insertions(+), 4 deletions(-) >=20 > Looking much better, thank you! >=20 >> +fn push_child_fds(fds: &mut Vec<(Rc>, PathBuf)>, fd: Own= edFd) { >=20 > Would it not make more sense to take Dir than OwnedFd? Easy fix. >> + // The rustix source code shows that Dir::new() never fails. >> + let child_fd =3D Rc::new(RefCell::new(Dir::new(fd).unwrap())); >> + while let Some(element) =3D child_fd.borrow_mut().next() { >> + let element =3D element.expect("Iterating through a cgroup di= rectory failed?"); >> + if element.file_type() !=3D rustix::fs::FileType::Directory {= >> + continue; >> + } >> + match element.file_name().to_bytes() { >> + b"." | b".." =3D> {} >> + other =3D> { >> + let other =3D Path::new(OsStr::from_bytes(other)).to_= owned(); >> + assert_single_component(&other); >> + fds.push((child_fd.clone(), other)); >=20 > The data structures used here are still very confusing. Why are we > storing a reference to the same file descriptor in every entry in the > Vec? Consider the recursive implementation (in pseudo-Rust): fn recursive_remove(fd) { for entry in get_entries(&fd) { if entry.is_dir_and_not_dot_or_dotdot() { let directory =3D open_dir(&fd, &entry.path())?; recursive_remove(directory)?; remove_dir(&fd, entry.path())?; } } } The compiler knows that fd will stay open through recursive calls, so this doesn't need any unsafe code. Using an explicit stack takes away this information from the compiler, so unsafe code is required. Using Rc> avoids the need for unsafe code at a cost in performance. For what it is worth, the standard library implementation of fs::remove_dir_all() is recursive. Standard library security hole? >> + } >> + } >> + } >> +} >> + >> +// Remove all subdirectories of the given directory recursively, >> +// but not the directory itself. The directory file descriptor >> +// is closed. >> +// >> +// This isn't the most efficient possible algorithm, but >> +// simplicity is more important than performance in this >> +// case. Also, it keeps open more file descriptors than >> +// strictly necessary, but Spectrum runs with a very high >> +// limit for the number of open file descriptors, and it >> +// uses shallow control group hierarchies. >> +fn remove_child_directories(dirfd: OwnedFd) -> Result<(), Errno> { >> + let mut fds =3D Vec::new(); >> + // Push the children of this directory onto the stack. >> + push_child_fds(&mut fds, dirfd); >> + while let Some((d, path)) =3D fds.pop() { >=20 > Couldn't we call push_child_fds() once here, rather than twice as is > currently done? (And then consider inlining it, depending on how > complex it's looking at the time.) Can you provide an example? I don't see how to make this change while preserving semantics. Only directories meant for deletion can appear on the stack, and the root of the traversal must not be deleted (yet). >> + assert_single_component(&path); >> + // Try to delete the directory. If that fails because there = are child >> + // directories, push the child directories onto the stack, th= en push >> + // this directory again. >> + match rustix::fs::unlinkat( >> + // The rustix source code shows that Dir::fd() never fail= s. >> + d.borrow().fd().unwrap(), >> + &path, >> + AtFlags::REMOVEDIR, >> + ) { >> + Err(Errno::NOTEMPTY) =3D> {} >> + Ok(()) =3D> continue, >> + Err(bad) =3D> return Err(bad), >> + } >> + let fd =3D openat2_simple( >> + // The rustix source code shows that Dir::fd() never fail= s. >> + &d.borrow().fd().unwrap(), >> + &path, >> + OFlags::DIRECTORY | OFlags::RDONLY, >> + )?; >> + // Process child directories first, then attempt to delete th= e >> + // directory again. >> + fds.push((d, path)); >> + push_child_fds(&mut fds, fd); >> + } >> + Ok(()) >> +} >=20 >> +impl Cgroup { >> + pub fn open_beneath(&self, path: &Path, flags: OFlags) -> Result<= OwnedFd, Errno> { >> + openat2_simple(self, path, flags) >> + } >=20 > This method looks pretty redundant now. Will remove. >> + 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}"))?; >=20 > Can we not use openat2_simple here? It's missing e.g. NOCTTY. NOCTTY is redundant with DIRECTORY, as directories can't be terminals. openat2_simple() uses NO_XDEV, which won't work here because /sys/fs/cgroup is not the same filesystem as /. >> + 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 =3D match component { >> + Component::Normal(component) =3D> component, >> + _ =3D> unreachable!(), >> + }; >=20 > I think it would be slightly more idiomatic to do: >=20 > let Component::Normal(component) =3D component else { > unreachable!() > }; Will fix in v6. >> + let sub_fd =3D cgroup >> + .open_beneath(Path::new(component), OFlags::RDONLY | = OFlags::DIRECTORY) >> + .map_err(|e| format!("Cannot open sub-cgroup {compone= nt:?}: {e}"))?; >> + // Take a shared lock on the *previous* file descriptor. >> + rustix::fs::flock(&cgroup, FlockOperation::LockShared) >> + .map_err(|e| format!("Cannot lock sub-cgroup {compone= nt:?}: {e}"))?; >> + cgroup.fd.push(sub_fd); >> + } >> + // Take an exclusive lock on the final file descriptor. >> + rustix::fs::flock(&cgroup, FlockOperation::LockExclusive) >> + .map_err(|e| format!("Cannot lock {path:?}: {e}"))?; >> + Ok(cgroup) >> + } >> + >> + pub fn wait_for_empty(fd: &dyn AsFd) -> std::io::Result<()> { >> + let wait_file =3D openat2_simple(fd, Path::new("cgroup.events= "), OFlags::RDONLY)?; >> + let poll_fd =3D wait_file.as_raw_fd(); >> + let mut wait_fd =3D File::from(wait_file); >> + let mut fds =3D libc::pollfd { >> + fd: poll_fd, >=20 > I would inline poll_fd here. RawFd is easy to misuse, so I like to > avoid having them hang around. I will move `fds` into the inner loop. >> + events: libc::POLLPRI | libc::POLLERR, >> + revents: 0, >> + }; >> + 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 wa= it >> + // forever. >> + if v.split(|&c| c =3D=3D b'\n').any(|line| line =3D=3D b"= populated 0") { >> + break; >> + } >> + // SAFETY: FFI call, valid arguments, fds contains 1 elem= ent >> + 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::new(path), AtFlags::R= EMOVEDIR) { >> + // 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 self.open_beneath(path, OFlags::RDONLY |= OFlags::DIRECTORY) { >> + 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 >> + // removed. This avoids concurrent executions of this progra= m >> + // operating on deleted sub-cgroups. >> + rustix::fs::flock(&sub_fd, FlockOperation::LockExclusive) >> + .map_err(|e| format!("Cannot lock sub-cgroup: {e}"))?; >> + >> + // Drop the exclusive lock on the original cgroup, >> + // This avoids blocking concurrent operations on other >> + // child cgroups while the cgroup is being purged, >> + // or while waiting for programs to exit. >> + rustix::fs::flock(&self, FlockOperation::LockShared) >> + .map_err(|e| format!("Cannot relock: {e}"))?; >=20 > Could you add some extra explanation here of why it's okay for the > exclusive lock to be temporarily dropped here? >=20 > I'm wondering whether taking a lock, then dropping it temporarily is a > sign that we're taking the lock too early in the first place, and shoul= d > scope it better to where it's actually needed. Indeed so. Programs that are modifying a cgroup need an exclusive lock on it. Adding or removing to the cgroup does *not* count as modification: both operations are idempotent, removing an in-use cgroup fails with -EBUSY, and operating on a deleted cgroup fails with -ENODEV or -ENOENT depending on what one is doing. Operations on control files *do* require an exclusive lock. >> + >> + // 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 em= pty: {e}")) >> + .inspect_err(|_| { >> + self.fd.pop().unwrap(); >> + })?; >> + >> + // Remove the child cgroup and its contents recursively. >> + remove_child_directories(sub_fd).map_err(|e| format!("Cannot = remove: {e}"))?; >> + // Re-take an exclusive lock on the parent of the cgroup bein= g purged. >> + // Otherwise, a concurrent instance of cgroup-setup might cre= ate a cgroup >> + // only for this one to delete it. The other instance could = then try to >> + // create a sub-cgroup of a deleted cgroup, which would fail.= Waiting >> + // until nobody is using the parent cgroup ensures these prob= lems can't >> + // happen. >> + // >> + // This must happen *after* the lock on the cgroup being purg= ed is released. >> + // Another instance of the program might have a shared lock o= n the parent >> + // and be waiting for an exclusive lock on the child. Trying= to take an >> + // exclusive lock on the parent while a lock is held on the c= hild would >> + // result in an ABBA deadlock. >> + rustix::fs::flock(&self, FlockOperation::LockExclusive) >> + .map_err(|e| format!("Cannot re-lock exclusively: {e}"))?= ; >> + // Delete the cgroup. If it's been re-created in the meantim= e >> + // and is currently in use, this is not an error. Another >> + // process deleting the cgroup is also not an error. Both of= >> + // these can happen because of the time period between >> + // remove_child_directories() closing the file 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/m= ain.rs >> new file mode 100644 >> index 0000000000000000000000000000000000000000..e8d9e9d7c2111857cc25b3= 6433d8f33ddb28c27b >> --- /dev/null >> +++ b/tools/cgroup-setup/src/main.rs >> @@ -0,0 +1,186 @@ >> +// SPDX-License-Identifier: EUPL-1.2+ >> +// SPDX-FileCopyrightText: 2026 Demi Marie Obenour >> + >> +mod cgroup; >> + >> +use cgroup::{Cgroup, openat2_simple, write_value}; >> +use rustix::{ >> + fs::{FlockOperation, Mode, OFlags, XattrFlags}, >> + io::Errno, >> +}; >> +use std::{ >> + env::ArgsOs, >> + ffi::OsStr, >> + fs::File, >> + io::Read as _, >> + os::unix::prelude::*, >> + path::{Path, PathBuf}, >> +}; >> + >> +// 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) >> + .map(|()| (path.parent().unwrap(), Path::new(path.file_name()= =2Eunwrap()))) >=20 > Doing this with map rather than ? is a little strange. Will change in v6. >> +} >> + >> +fn read_control_file(fd: &dyn AsFd, p: &Path) -> Result, Stri= ng> { >> + let mut buf =3D Vec::new(); >> + File::from( >> + openat2_simple(&fd, Path::new(p), OFlags::RDONLY) >> + .map_err(|e| format!("Cannot open {p:?}: {e}"))?, >> + ) >> + .read_to_end(&mut buf) >> + .map_err(|e| format!("Cannot read {p:?}: {e}"))?; >> + Ok(buf) >> +} >=20 > Nothing control-file-specific about this method. It just reads a file.= > And a bit odd for write_value to be in cgroup.rs while this is here. It's only used once, so I'll inline it. >> + >> +fn enable_subtree_control(fd: &dyn AsFd) -> Result<(), String> { >=20 > Would it not make sense for this to be an instance method on Cgroup, > since it's a Cgroup-specific operation? We don't create a Cgroup struct for the child cgroup FD on which this function is called. >> + let p =3D Path::new("cgroup.controllers"); >> + let buf =3D read_control_file(fd, p)?; >=20 > p is only used here, so can just be inlined. If read_control_file took= > AsRef like the standard library functions do, you wouldn't even > need to construct the path here. I will just inline the whole read_control_file function. It's only used once. >> + let mut subtree =3D vec![]; >> + for controller in buf.split(|&b| b =3D=3D b' ').filter(|e| !e.is_= empty()) { >=20 > Are there ever likely to be empty works in this file? No, there will not be unless there is a kernel bug. >> + 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"), &subtre= e)?; >> + } >> + Ok(()) >> +} >> + >> +fn cgroup_setup(mut args: ArgsOs) -> Result<(), String> { >> + let mut leaf =3D false; >> + let mut cgroup_path; >> + let mut systemd_compat =3D false; >> + let mut wait =3D true; >> + loop { >> + cgroup_path =3D args.next(); >> + let Some(ref arg_) =3D cgroup_path else { >> + break; >> + }; >> + let arg_ =3D arg_.as_bytes(); >> + if arg_ =3D=3D b"--" { >> + cgroup_path =3D args.next(); >=20 > This cgroup_path thing is a bit complicated. I feel like this could > probably be cleaned up with a peekable iterator and a while loop. Easy to fix. >> + break; >> + } >> + if !arg_.starts_with(b"-") { >> + break; >> + } >> + >> + if !arg_.starts_with(b"--") { >> + return Err("takes no short options".to_owned()); >> + } >> + >> + match &arg_[2..] { >> + b"leaf" =3D> leaf =3D true, >> + b"wait" =3D> wait =3D true, >> + b"no-wait" =3D> wait =3D false, >> + b"systemd-compat" =3D> systemd_compat =3D true, >=20 > Why do we have --wait and --no-wait, but no --no-leaf or --no-systemd-c= ompat? Spectrum never needs --wait, so I'll remove it. >> + arg =3D> return Err(format!("unknown long option {:?}", O= sStr::from_bytes(arg))), >> + } >> + } >> + let Some(cgroup_path) =3D cgroup_path.map(PathBuf::from) else { >> + return Err("have no positional arguments, expected at least 1= ".to_owned()); >> + }; >> + >> + let (parent_cgroup_path, child_cgroup_path) =3D split_path(&cgrou= p_path)?; >> + 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 make child cgroup: {e}= ")), >> + } >> + let child =3D cgroup >> + .open_beneath(child_cgroup_path, OFlags::RDONLY | OFlags::DIR= ECTORY) >> + .map_err(|e| format!("Cannot make child cgroup: {e}"))?; >> + if wait { >> + // While waiting, only hold an exclusive lock on the child, n= ot the parent. >> + rustix::fs::flock(&child, FlockOperation::LockExclusive) >> + .map_err(|e| format!("Cannot take an exclusive lock on ch= ild cgroup: {e}"))?; >> + rustix::fs::flock(&cgroup, FlockOperation::LockShared) >> + .map_err(|e| format!("Cannot downgrade lock on cgroup to = a shared lock: {e}"))?; >> + Cgroup::wait_for_empty(&child) >> + .map_err(|e| format!("Cannot wait for {parent_cgroup_path= :?} to be empty: {e}"))?; >> + } >> + let pid =3D std::process::id().to_string(); >> + if leaf { >> + if args.len() !=3D 0 { >> + // If we aren't delegating any cgroups, don't create a su= b-cgroup. >> + write_value(&child, Path::new("cgroup.procs"), pid.as_byt= es()) >> + .map_err(|e| format!("Cannot move process to child cg= roup: {e}"))?; >> + } >> + } else { >> + // If the child process will need to manage cgroups itself, i= t 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 alrea= dy exists, >> + // that isn't an error. >> + match rustix::fs::mkdirat(&child, cgroup::DEFAULT_LEAF, Mode:= :from_raw_mode(0o755)) { >> + Ok(()) | Err(Errno::EXIST) =3D> {} >> + Err(e) =3D> return Err(format!("Cannot make child cgroup:= {e}")), >> + } >> + if args.len() !=3D 0 { >> + let child_proc_path =3D Path::new(cgroup::DEFAULT_LEAF).j= oin(Path::new("cgroup.procs")); >> + write_value(&child, &child_proc_path, pid.as_bytes()) >> + .map_err(|e| format!("Cannot move process to child cg= roup: {e}"))?; >> + } >> + if systemd_compat { >> + // systemd-aware programs expect to have user.delegate=3D= 1 >> + // and to set cgroup.subtree_control themselves >> + rustix::fs::fsetxattr(&child, c"user.delegate", b"1", Xat= trFlags::empty()).map_err( >> + |e| format!("Cannot enable cgroup delegation in {pare= nt_cgroup_path:?}: {e}"), >> + )? >> + } else { >> + // Spectrum's programs do not check for user.delegate=3D1= >> + // and expect the caller to set cgroup.subtree_control. >> + enable_subtree_control(&child)?; >> + } >> + } >=20 > So looking at this I still see several different modes and am wondering= > whether we could simplify this further. >=20 > =E2=80=A2 Why do we need a separate leaf mode? Why not just still use= a > $inner.service in that case? $inner.service is just wasteful and makes it harder to inspect the cgroup= tree by hand. > =E2=80=A2 What would the consequences be if we took the systemd_compat= branch > for a non-cgroup-aware Spectrum program? Non-cgroup-aware programs would be fine, but nested calls to cgroup-setup= would break because they need the enable_subtree_control() call. However= , in the future, I would like to check the user.delegate xattr to determine= if one can safely write to the control files of the cgroup or if the cgro= up is owned by another program. >> + let Some(program_name) =3D args.next() else { >> + return Ok(()); >> + }; >> + let e =3D std::process::Command::new(&program_name).args(args).ex= ec(); >> + Err(format!("Cannot spawn child {:?}: {}", program_name, e)) >> +} >> + >> +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: &OsStr, args: ArgsOs) -> Result<(), String> { >> + match prog_name >> + .as_bytes() >> + .split(|&b| b =3D=3D b'/') >> + .next_back() >> + .unwrap() >=20 > prog_name.file_name(), where prog_name is &Path? Works for me :) >> + { >> + b"cgroup-setup" =3D> cgroup_setup(args), >> + 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(&prog_name, args) { >> + Ok(()) =3D> {} >> + Err(e) =3D> { >> + eprintln!("{prog_name:?}: {}", e); >> + std::process::exit(1); >> + } >> + } >> +} >> >> -- >> 2.55.0 --=20 Sincerely, Demi Marie Obenour (she/her/hers) --------------X5WsLE22LCLuMWI4kBORF0P5-- --------------yQCTqYiU2OPL9Rn5eHCgZrFt Content-Type: application/pgp-signature; name="OpenPGP_signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="OpenPGP_signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEopQtqVJW1aeuo9/sszaHOrMp8lMFAmpyk4AACgkQszaHOrMp 8lPWbg/+M55w03VOdPwj7rQR2hF0+PMam9wP9e3/cdwgURNbvxLUECpxRT9LVXX3 JxCLosISgV8IBtk+2YRmvqQLMWcJ78yHEn22tV8+vz5NpNa36pBiFXz3q/VPxEOR +dfPbYoWinJ1BQH4yqz3sKqOXC1NConD7zB4hkD/taw7V3rSyNFgDlc7QVO4zKl+ E/GspXpOXZSEZxfN701h+3j2vbs89EpTlhNM+gtEhau/WpdzZf6MKxOs/jQOYOYG NRMmWJ7XbU3fPmNO/J/415VvIheBtqnWjffUR121XqwXsFa2GeAniA8iLEyLoBVG rRHTS8AaTq2UWSA7YC26kRf4d6vLJLCl2e1j184wPWG6uYPIYaojAPCm58+M5FAU P7dtoq6U0CjcDWRvu+RM0vgAMO1KRWF7DzuEwU10lAtGSXHFxNZQL9QqHzCLi3JZ WUyH74ENIjhBnKUZZ6H1acqh5q6gxxgVNkEmAJ2C/Vd6RKxxg/jeWYKCfX3b+im7 03PwpRIHMsA3dQ9QfhLaSoI/eJp2VYf8aGRQMs88e3EyzJyLfNBJeEWC5Gl8kz4a aZNZX2kLPS8vTp5rICftgs7tnQALmyiRs1cGeki+HBz1qbLzjDZN23zLxd9Bobeo x5kl41WqzF+q6cV1nvg1GI0V1pwYy/86iOcxS74n4l34HQuv12k= =+p44 -----END PGP SIGNATURE----- --------------yQCTqYiU2OPL9Rn5eHCgZrFt--