Re: [PATCH] selftests: memcg: Don't hang in the inotify tests without IN_DELETE_SELF

[T.J. Mercier]

On Sat, Sep 26, 2026 at 7:42 AM Sebastian Chlad
<sebastianchlad@xxxxxxxxx> wrote:

Hi,

> test_memcg_inotify_delete_file() and test_memcg_inotify_delete_dir()
> wait for IN_DELETE_SELF with a blocking read() on the inotify fd. On a
> kernel that does not send IN_DELETE_SELF for kernfs files and
> directories, the read() never returns and test_memcontrol hangs until
> it is killed by the kselftest timeout. The remaining tests do not run
> and no result is reported for the inotify tests.
>
> Wait for the first event with poll() and a timeout. If IN_DELETE_SELF
> does not arrive in time, report it and fail the test instead of
> blocking, so that test_memcontrol completes and the remaining results
> are reported.

But this polls for every read() through read_event(), not just IN_DELETE_SELF.

> Assisted-by: claude-opus-5-5
> Signed-off-by: Sebastian Chlad <sebastian.chlad@xxxxxxxx>
> ---
>  .../selftests/cgroup/test_memcontrol.c        | 27 ++++++++++++++++---
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> index 0ed82347044e..c00420acc9a1 100644
> --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
> @@ -11,6 +11,7 @@
>  #include <sys/types.h>
>  #include <unistd.h>
>  #include <sys/inotify.h>
> +#include <poll.h>
>  #include <sys/socket.h>
>  #include <sys/wait.h>
>  #include <arpa/inet.h>
> @@ -1655,10 +1656,20 @@ static int test_memcg_oom_group_score_events(const char *root)
>         return ret;
>  }
>
> +#define INOTIFY_TIMEOUT_MS 5000
> +
>  static int read_event(int inotify_fd, int expected_event, int expected_wd)
>  {
> +       struct pollfd pfd = { .fd = inotify_fd, .events = POLLIN };
>         struct inotify_event event;
>         ssize_t len = 0;
> +       int ret;
> +
> +       ret = poll(&pfd, 1, INOTIFY_TIMEOUT_MS);
> +       if (ret == 0)
> +               return -ETIMEDOUT;

This mixes -1 with -ETIMEDOUT. The lack of consistency for error
handling is kinda weird.

> +       if (ret < 0)
> +               return -1;
>
>         len = read(inotify_fd, &event, sizeof(event));
>         if (len < (ssize_t)sizeof(event))
> @@ -1678,7 +1689,7 @@ static int test_memcg_inotify_delete_file(const char *root)
>  {
>         int ret = KSFT_FAIL;
>         char *memcg = NULL;
> -       int fd, wd;
> +       int fd, wd, err;
>
>         memcg = cg_name(root, "memcg_test_0");
>
> @@ -1701,7 +1712,11 @@ static int test_memcg_inotify_delete_file(const char *root)
>         free(memcg);
>         memcg = NULL;
>
> -       if (read_event(fd, IN_DELETE_SELF, wd))
> +       err = read_event(fd, IN_DELETE_SELF, wd);
> +       if (err == -ETIMEDOUT)
> +               fprintf(stderr, "no IN_DELETE_SELF event within %d ms\n",
> +                       INOTIFY_TIMEOUT_MS);
> +       if (err)
>                 goto cleanup;
>
>         if (read_event(fd, IN_IGNORED, wd))
> @@ -1723,7 +1738,7 @@ static int test_memcg_inotify_delete_dir(const char *root)
>  {
>         int ret = KSFT_FAIL;
>         char *memcg = NULL;
> -       int fd, wd;
> +       int fd, wd, err;
>
>         memcg = cg_name(root, "memcg_test_0");
>
> @@ -1746,7 +1761,11 @@ static int test_memcg_inotify_delete_dir(const char *root)
>         free(memcg);
>         memcg = NULL;
>
> -       if (read_event(fd, IN_DELETE_SELF, wd))
> +       err = read_event(fd, IN_DELETE_SELF, wd);
> +       if (err == -ETIMEDOUT)
> +               fprintf(stderr, "no IN_DELETE_SELF event within %d ms\n",
> +                       INOTIFY_TIMEOUT_MS);
> +       if (err)
>                 goto cleanup;
>
>         if (read_event(fd, IN_IGNORED, wd))

This IN_IGNORED case now fails silently while IN_DELETE_SELF prints a
timeout error message. What about moving the error message up into
read_event? That would address the comment about inconsistent return
values too.