From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-189.mta1.migadu.com (mta1.migadu.com [37.59.57.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A8B02255E43 for ; Mon, 10 Aug 2026 09:42:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=37.59.57.117 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786354969; cv=none; b=YkaKDlXdWGn2yxzCcoJhpJQHfulA3rh8TtZmVNlcp6KzqyOFy81Tr54x2CYirzIdw7NFfMmHrwwuialxTIOG2vgZYy9gkEWAo4jf5rHCqRL7hWCFSfwoCL1i0cOL4FBglDFlZad4EYltHhwkRWSWcLoP1r3if9RHpiULib/FyeM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786354969; c=relaxed/simple; bh=pXvOdQwVcRI4o3cSBezn8YOPLrt2oF9EEnHUzSsntt0=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=q/Z+o7OrIISeKAnCCHZxH0AUcVBTOCg939kDhtSGYJmtm/E1v8yLYz+d2nuPYfxl+xrfxyuBruTLbDJU09M3FADxtu8qsYjQMkkeQaadVUGaim+7rV8REBPWaKo6+xEnshFr1K/FLqU1fd/NNZGjk4V+5E3p0m2j+VtXEESOSv8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=U6avpM00; arc=none smtp.client-ip=37.59.57.117 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="U6avpM00" Message-ID: <53467765-7ffa-4da4-9473-2ea770048f96@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1786354965; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2x97XEkL9/hfiKG5eM3hQ+3kNVvep8/7s1qs1TGOscQ=; b=U6avpM00iquNlfSe/eaFZcyMpH7DGnAzt+9E6RsiB++oTPNT7b+sKf3iwGakzmo0VBWDXb D4IrkPdKG0WGA5farBEIyqW1NZWZY2XW3+CXsukz8sP0ZD1SZ+rBABtBLoxWrzO66KLaeK Cvonv5c0xhi2vpuF2jCYufqxpW7sGV4= Date: Mon, 10 Aug 2026 17:42:38 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Cc: cui.tao@linux.dev, Tejun Heo , Johannes Weiner , Suren Baghdasaryan , Shuah Khan , cgroups@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Ziyang Men , Tao Cui Subject: Re: [PATCH v2] selftests/cgroup: add PSI pressure trigger and validation tests To: =?UTF-8?Q?Michal_Koutn=C3=BD?= References: <20260728083742.2359320-1-cui.tao@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Tao Cui In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT Hi Michal, 在 2026/8/7 00:52, Michal Koutný 写道: > Hi, > thanks for continuing with this. > Thanks -- though honestly I owe you the thanks. Your v1 review is what got me to drop the memory-pressure approach and churn_memory(), and to keep the runner out of the cgroup. v2 basically exists because you took the time on v1. Still only my second selftests patch, so still feeling my way around the conventions here (clearly -- see below). > On Tue, Jul 28, 2026 at 04:37:42PM +0800, Tao Cui wrote: >> +/* Trigger smoke test: second on same fd -> EBUSY; IRQ rejects "some". */ >> +static int test_proc_triggers(const char *root) >> +{ >> + static const char *const resources[] = { "io", "memory", "cpu" }; >> + int ret = KSFT_FAIL; >> + int fd = -1; >> + int i; >> + >> + (void)root; > > WTH? > Yeah, bogus. Dropping the param entirely; details in my reply to Suren. >> + >> + for (i = 0; i < (int)ARRAY_SIZE(resources); i++) { > > This (int) cast is quite uncommon, look around. > Dropping it, size_t loop var. >> +/* Induce deterministic CPU pressure (more hogs than CPUs). */ >> +static int test_cgroup_trigger_fire(const char *root) >> +{ >> + char *cg = NULL, *cpupress = NULL; >> + int fd = -1, ret = KSFT_FAIL; >> + struct pollfd pfd; >> + long ncpus, i; >> + pid_t pid; >> + >> + cg = cg_name(root, "psi_trigger_test"); >> + if (!cg) >> + goto cleanup; >> + if (cg_create(cg)) >> + goto cleanup; >> + >> + cpupress = cg_control(cg, "cpu.pressure"); >> + if (!cpupress) >> + goto cleanup; >> + fd = open(cpupress, O_RDWR); >> + if (fd < 0) { >> + ksft_print_msg("open cpu.pressure: %s\n", strerror(errno)); >> + goto cleanup; >> + } >> + >> + /* 1us threshold in a 1s window: any cpu stall fires it. */ >> + errno = 0; >> + if (write_trigger(fd, "some 1 1000000") <= 0) { >> + ksft_print_msg("arming trigger failed: %s\n", strerror(errno)); >> + goto cleanup; >> + } > > When I see this, could you increase the window size to 2000000 (so that > it has potential to run for unprivileged users)? > Yes, "some 1 2000000". >> + >> + ncpus = sysconf(_SC_NPROCESSORS_ONLN); >> + if (ncpus <= 0) >> + ncpus = 1; >> + >> + pid = fork(); >> + if (pid < 0) { >> + ksft_print_msg("fork: %s\n", strerror(errno)); >> + goto cleanup; >> + } >> + if (pid == 0) { >> + /* Enter the cgroup, then over-subscribe it with CPU hogs. */ >> + if (cg_enter_current(cg)) >> + _exit(KSFT_FAIL); >> + for (i = 0; i < ncpus; i++) { >> + if (fork() == 0) { >> + for (;;) >> + asm volatile("" ::: "memory"); >> + _exit(0); >> + } >> + } >> + for (;;) >> + asm volatile("" ::: "memory"); /* child is also a hog */ >> + _exit(0); >> + } > > This part could be replaced with generalized hog_cpus_timed() from > test_cpu.c (after move to cgroup_util.c). > Agreed it shouldn't be open-coded. One snag though: hog_cpus_timed() runs for a fixed duration and returns, but here I want hogs that keep running until the trigger fires and then get killed. So I'll move a shared helper into cgroup_util.[ch] and call it from both spots, rather than using hog_cpus_timed() as-is. >> +int main(int argc, char **argv) >> +{ >> + char root[PATH_MAX]; >> + int mempress_fd; >> + int i; >> + >> + (void)argc; > > What's up with this? > Leftover, removed. >> + >> + ksft_print_header(); >> + ksft_set_plan(ARRAY_SIZE(tests)); >> + >> + if (cg_find_unified_root(root, sizeof(root), NULL)) >> + ksft_exit_skip("cgroup v2 isn't mounted\n"); >> + >> + /* PSI must be enabled (CONFIG_PSI=y, not default-disabled). */ >> + mempress_fd = open("/proc/pressure/memory", O_RDONLY); >> + if (mempress_fd < 0) >> + ksft_exit_skip("PSI unavailable (CONFIG_PSI=n or psi=0)\n"); >> + close(mempress_fd); >> + >> + if (cg_read_strstr(root, "cgroup.controllers", "memory")) >> + ksft_exit_skip("memory controller isn't available\n"); >> + if (cg_read_strstr(root, "cgroup.subtree_control", "memory")) >> + if (cg_write(root, "cgroup.subtree_control", "+memory")) >> + ksft_exit_skip("failed to enable memory controller\n"); > > The memory controller is unnecessary now, right? > Right. v1 leftover, CPU pressure now, so that setup is dead code. Dropping it. I'll fold all of this into v3. Thanks, Tao > > Regards, > Michal