From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 409194A6CFA for ; Wed, 2 Sep 2026 18:47:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788374856; cv=none; b=eD2y0dOsjw/329ir919F2XXOcm4QJMcpjsBQBU0YpObkp3D/ikJSZWPZ5V9NTnAqrrKu/LPdlArko6SbHgiauikoq08El/QRHDEK3HE1mCyocCf5iz7dhoYEOd52aDkSgboFmLp7MvSsE7SHE/S+hN5bMr8zEwnHLKgmAZBIdVI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788374856; c=relaxed/simple; bh=EKFYkxW33JQakAeWcdMhEomjNk4xgLKTRBDk38OiwiQ=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Xi+wKomp0eLXQacXQtqai31tBsiIMpbJKo8BDnGHynLSgCue4h6CZiMVcurfx86m4ndfr5Lu8pwhpVq87R5lvqbBk7bBPucG0F0eGR+GY6xcPkVmG1PYW4SqBGHeiKSjKTN3SIjdx3S9/nrkM6G1EeneS/3qaUSjYNKI0CKvxFU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=AFCKenhk; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="AFCKenhk" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-49a97714f5dso10629065e9.0 for ; Wed, 02 Sep 2026 11:47:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1788374852; x=1788979652; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=IdCJ7DhUlcSYo5R128c9LaTPAZTDWZg8qrvo1ipn3/A=; b=AFCKenhkUuhZMDVClNVR+mQwMovkD+fOyQIeToFLjgXHkw/v7jaixe2GYZImeCFR1t eDK4cy8eSbh+sDwcN8anJfAzgbHqW6HrL+g6c2nnpbM33ZzdI39Rj29CbnSdNT9Hfre6 aQeeWG7044G8sdWLTABpZ7gVZIlkQxj9Ml+b5tQKU+/523OUVhX/4eqqiEOZMDeznApp N6PrPVitkLZqhh7i9XHdEQ3eAKOcD7gB1Nn373NEctcyY3zHBeanuawi0C8S1yiz2l0c tH/MGNqbM7YVqmXF8lOQVNrQOP7TKqDwWk4p6sGzel4Rnt//CsZoa2ZS9KPK2Vts/31I v0Ew== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788374852; x=1788979652; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=IdCJ7DhUlcSYo5R128c9LaTPAZTDWZg8qrvo1ipn3/A=; b=SQ3l3tYnsq4rR+v7KQ14ZUeTohHwXT+rLymTeVvg8KTPJ68vpH6ra43LQ6Csh84GCp JJCS0dvgjGh0lKBb4tBd0ucWIfLjOviAXMbIOFrDDlYiU0WQ4Hh1GGlwrM8hpb0QKh7G v+8bU55LrXyRszDr3Pm4IqY/grN1YP3xtJSwcB2tcZf6oMvnkJN/GbQKrUykUUlRLWMp Z1F8Q5PDRuROx/9tJsNGqdzfUgSR6PhMflJswgCqSBzsORtDxCNuWpJlG0MBKE0OlyYR nYqZZYzjO+wxARw0JzJRaoWexftc2nVxANXWzBfqT+ZXaP61sampGp5J0/LRm8Gkh80+ 6znQ== X-Forwarded-Encrypted: i=1; AKwUvBzpk99XZlplSK32YdeyhYW6jiLg74XGlxSxKdtcOPRTffD/a76q08Ci+5k+OnX8kLudylEMeEKP6Z+8W+D896A=@vger.kernel.org X-Gm-Message-State: AFuF++m9wbO44HIUbXqWN6LuDX31zbGNCv63zqAdtV8FYr0+3uoRaFdp lPGLYhPWYMKzoCmkT8gflsUW7Ia0LCaIGnrB80EteFW9h7QLk/iDwjH5thlwbo07RwQ= X-Gm-Gg: AYBFou1Ws08v0NqaBFTcEjMtoOy4ykF18s5wV2Pcn7Eurf6D5ge3bFf40xXOu6F6sQq LBOgv0ATeub7CPACMW2hLa9HzE+Y5kHOoPFE5IjqJ909I+EpmauKKk3RyGNc+QPz5+NoqBkMTRG 3QkSr+frCmqIpFOTD3Qbuyolc5mQ/EUeAjRX747XVmnIhwNqLkxkMsH4gCY0ld/23m7KxZKomZK zUSghWK2zYirB/9Q1zHg4VZln1gXxBVcnLVvtfQHu/FZ2O+ycJQUoNldiJuXUaFXprCROWScCtA PHKLhTx//aaROuTq4H/f6gnX076pqusMDowkgeylABcn6omKsXGHXYkUImebzHYkpYqEhxTXxCz NdQ5na8+PCBFRKCWFKjY1ttPR404wJzF4Nbk972H2CYIGJ0c+KENqHHr9dPvtUBnpl6gh255ZFi aDhsk0/xZ+v30Xlg9v7+SSHSsZ6anpEBghxDGKYqAjbOh892QL4BPjf1EYAnzuETQqdW1bD2ZjM A== X-Received: by 2002:a05:600c:3b25:b0:49c:cfae:34d2 with SMTP id 5b1f17b1804b1-49ce55fb5f7mr127189595e9.4.1788374852065; Wed, 02 Sep 2026 11:47:32 -0700 (PDT) Received: from localhost.localdomain ([2001:af0:8000:1409:193:86:92:181]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49cee5d2019sm11290985e9.2.2026.09.02.11.47.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 02 Sep 2026 11:47:31 -0700 (PDT) Date: Wed, 2 Sep 2026 20:47:29 +0200 From: Michal =?utf-8?Q?Koutn=C3=BD?= To: Tao Cui Cc: Suren Baghdasaryan , Tejun Heo , Johannes Weiner , Shuah Khan , cgroups@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, Ziyang Men , Tao Cui Subject: Re: [PATCH v5] selftests/cgroup: add PSI pressure trigger and validation tests Message-ID: References: <20260902040725.877155-1-cui.tao@linux.dev> Precedence: bulk X-Mailing-List: linux-kselftest@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="k7dsd4vdmhwtmf2v" Content-Disposition: inline In-Reply-To: <20260902040725.877155-1-cui.tao@linux.dev> --k7dsd4vdmhwtmf2v Content-Type: text/plain; protected-headers=v1; charset=us-ascii Content-Disposition: inline Subject: Re: [PATCH v5] selftests/cgroup: add PSI pressure trigger and validation tests MIME-Version: 1.0 Hello Tao. On Wed, Sep 02, 2026 at 12:07:25PM +0800, Tao Cui wrote: > +/* PSI triggers are written with a trailing NUL the kernel parser expects. */ > +static ssize_t write_trigger(int fd, const char *trigger) > +{ > + return write(fd, trigger, strlen(trigger) + 1); > +} Hyrum's law. It all works for me: NUL, \n or just write(2) the exact length of the string. For conventionality, I'd prefer the simple literals and plain strlen() + 0. (I reckon cg_write() cannot be used because of FD access.) > + > +static int pressure_open(const char *resource) > +{ > + char path[PATH_MAX]; > + int fd; > + > + snprintf(path, sizeof(path), "/proc/pressure/%s", resource); > + fd = open(path, O_RDWR); > + if (fd < 0) > + ksft_perror(path); This outputs: | # /proc/pressure/irq: No such file or directory (2) | # SKIP /proc/pressure/irq unavailable I.e. similar message is printed twice. Since strace is a companion of cgroup selftests, I'd keep this helper silent. > + return fd; > +} > + > +FIXTURE(psi) > +{ > + char root[PATH_MAX]; > + char *cg; > +}; > + > +FIXTURE_SETUP(psi) > +{ > + int psi_fd; > + > + if (cg_find_unified_root(self->root, sizeof(self->root), NULL)) > + SKIP(return, "cgroup v2 isn't mounted"); > + > + /* PSI must be enabled (CONFIG_PSI=y, not disabled on the cmdline). */ > + psi_fd = open("/proc/pressure/memory", O_RDONLY); > + if (psi_fd < 0) > + SKIP(return, "PSI unavailable (CONFIG_PSI=n or psi=0)"); > + close(psi_fd); > + > + self->cg = cg_name(self->root, "psi_trigger_test"); > + if (!self->cg) > + SKIP(return, "failed to allocate cgroup name"); > + if (cg_create(self->cg)) > + SKIP(return, "failed to create cgroup: %s", strerror(errno)); Why are these two SKIPs (not failures)? > +TEST_F(psi, cgroup_trigger_fire) > +{ > + char *cpupress; > + struct pollfd pfd = { .events = POLLPRI }; > + long ncpus; > + int fd; > + int i; > + > + cpupress = cg_control(self->cg, "cpu.pressure"); > + ASSERT_NE(NULL, cpupress); > + fd = open(cpupress, O_RDWR); > + free(cpupress); > + ASSERT_GE(fd, 0); > + pfd.fd = fd; > + > + /* > + * 1usec threshold over a 2s window: any CPU stall fires it. The 2s > + * window is the smallest unprivileged users are allowed to arm. > + */ > + ASSERT_GT(write_trigger(fd, "some 1 2000000"), 0); The selftest rarely can be run as unprivileged user (even test cgroup creation needs privileges), so this comment is irrelevant. (But it's fine to test with that value.) On the more abstract level -- I was playing with this and thinking about a value that'd test both sides, i.e. false triggers as well as false non-triggers. I'd find that to be the half of the window and the number of tasks should be then (3*ncpus + 1) / 2. Or perhaps test two thresholds, one tiny like you did and one maximum (whole window) with same amount tasks but expect trigger, no trigger respectively. > + > + ncpus = sysconf(_SC_NPROCESSORS_ONLN); > + if (ncpus == -1) > + TH_LOG("sysconf(_SC_NPROCESSORS_ONLN): %s", strerror(errno)); > + ASSERT_NE(-1, ncpus); Same as messages from pressure_open() above. Simply assert. > + > + /* ncpus+1 hogs guarantee CPU contention inside the cgroup. */ > + for (i = 0; i < ncpus + 1; i++) > + ASSERT_GE(cg_run_nowait(self->cg, hog_cpu, NULL), 0); > + > + ASSERT_EQ(1, poll(&pfd, 1, PSI_POLL_TIMEOUT_MS)); > + ASSERT_NE(0, pfd.revents & POLLPRI); > + close(fd); > +} > + > +TEST_HARNESS_MAIN All in all, this looks so much better than the initial version, well done. Just a few polishing touches. Michal --k7dsd4vdmhwtmf2v Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iJEEABYKADkWIQRCE24Fn/AcRjnLivR+PQLnlNv4CAUCaphvPRsUgAAAAAAEAA5t YW51MiwyLjUrMS4xMiwyLDIACgkQfj0C55Tb+AiKJwD+PqtkHd3blRPy13sFyLqF 8DxwZSomQ1H3UCKVICDOT/8A/A56P4uCIa6DGF+2hWa4rtwuRod+M+rCD4tqo/Cf mVQH =xP5B -----END PGP SIGNATURE----- --k7dsd4vdmhwtmf2v--