Hello, Sahaj.
The following is a Claude-generated review.
On Tue, Oct 13, 2026 at 12:18:12PM +0530, Sahaj Chaudhari wrote:
> Add a selftest for pids.events notifications. When pids_localevents is
> enabled, a fork rejected by pids.max must increment pids.events and
> generate an IN_MODIFY inotify event on that file.
Can you mention 1765a153d985 ("cgroup/pids: Restore pids.events
notifications in local mode") here as the fix this test covers?
> Check for a failed read() of the inotify event before comparing its
> signed ssize_t result with sizeof(struct inotify_event). Otherwise -1 is
> converted to a large unsigned length and the uninitialized event buffer
> is inspected.
This describes the v1 to v2 change. As v1 never landed, it belongs only in
the changelog below the --- line.
> +TEST_GEN_PROGS += test_pids_local_events
Can this be a test in test_pids.c instead? Its main() already does the
same setup, so the new test fits as another T() entry next to
test_pids_events() without the Makefile and .gitignore changes. It can
then use cg_run_nowait() with run_success() for the fork and cg_control()
for the pids.events path.
> + if ((size_t)n < sizeof(struct inotify_event) ||
> + !(event_buf.align.mask & IN_MODIFY)) {
> + reason = "received no pids.events modify notification";
> + goto cleanup;
> + }
> + if (cg_read_key_long(cgroup, "pids.events", "max ") != 1) {
> + reason = "pids.events did not count the rejected fork";
> + goto cleanup;
> + }
This repeats the counter check before poll(). Nothing can change the
counter in between, so maybe drop it?
> + if (proc_mount_contains("pids_localevents") <= 0)
> + ksft_exit_skip("mount cgroup2 with pids_localevents to run this
> test\n");
Is this skip necessary? In the default mode, the rejected fork also
increments the test cgroup's pids.events and notifies it, as the test
cgroup is the one over the limit. The same checks hold, so without the
skip the test covers the notification in both modes and still catches
the regression on a pids_localevents mount.
Thanks.
--
tejun
