[PATCH v3 3/3] selftests: memcg: Add tests IN_DELETE_SELF and IN_IGNORED on memory.events

T.J. Mercier posted 3 patches 1 month, 1 week ago
[PATCH v3 3/3] selftests: memcg: Add tests IN_DELETE_SELF and IN_IGNORED on memory.events
Posted by T.J. Mercier 1 month, 1 week ago
Add two new tests that verify inotify events are sent when memcg files
are removed.

Signed-off-by: T.J. Mercier <tjmercier@google.com>
Acked-by: Tejun Heo <tj@kernel.org>
---
 .../selftests/cgroup/test_memcontrol.c        | 122 ++++++++++++++++++
 1 file changed, 122 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
index 4e1647568c5b..2b065d03b730 100644
--- a/tools/testing/selftests/cgroup/test_memcontrol.c
+++ b/tools/testing/selftests/cgroup/test_memcontrol.c
@@ -10,6 +10,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <unistd.h>
+#include <sys/inotify.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
 #include <arpa/inet.h>
@@ -1625,6 +1626,125 @@ static int test_memcg_oom_group_score_events(const char *root)
 	return ret;
 }
 
+static int read_event(int inotify_fd, int expected_event, int expected_wd)
+{
+	struct inotify_event event;
+	ssize_t len = 0;
+
+	len = read(inotify_fd, &event, sizeof(event));
+	if (len < (ssize_t)sizeof(event))
+		return -1;
+
+	if (event.mask != expected_event || event.wd != expected_wd) {
+		fprintf(stderr,
+			"event does not match expected values: mask %d (expected %d) wd %d (expected %d)\n",
+			event.mask, expected_event, event.wd, expected_wd);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int test_memcg_inotify_delete_file(const char *root)
+{
+	int ret = KSFT_FAIL;
+	char *memcg = NULL, *child_memcg = NULL;
+	int fd, wd;
+
+	memcg = cg_name(root, "memcg_test_0");
+
+	if (!memcg)
+		goto cleanup;
+
+	if (cg_create(memcg))
+		goto cleanup;
+
+	if (cg_write(memcg, "cgroup.subtree_control", "+memory"))
+		goto cleanup;
+
+	child_memcg = cg_name(memcg, "child");
+	if (!child_memcg)
+		goto cleanup;
+
+	if (cg_create(child_memcg))
+		goto cleanup;
+
+	fd = inotify_init1(0);
+	if (fd == -1)
+		goto cleanup;
+
+	wd = inotify_add_watch(fd, cg_control(child_memcg, "memory.events"), IN_DELETE_SELF);
+	if (wd == -1)
+		goto cleanup;
+
+	cg_write(memcg, "cgroup.subtree_control", "-memory");
+
+	if (read_event(fd, IN_DELETE_SELF, wd))
+		goto cleanup;
+
+	if (read_event(fd, IN_IGNORED, wd))
+		goto cleanup;
+
+	ret = KSFT_PASS;
+
+cleanup:
+	if (fd >= 0)
+		close(fd);
+	if (child_memcg)
+		cg_destroy(child_memcg);
+	free(child_memcg);
+	if (memcg)
+		cg_destroy(memcg);
+	free(memcg);
+
+	return ret;
+}
+
+static int test_memcg_inotify_delete_rmdir(const char *root)
+{
+	int ret = KSFT_FAIL;
+	char *memcg = NULL;
+	int fd, wd;
+
+	memcg = cg_name(root, "memcg_test_0");
+
+	if (!memcg)
+		goto cleanup;
+
+	if (cg_create(memcg))
+		goto cleanup;
+
+	fd = inotify_init1(0);
+	if (fd == -1)
+		goto cleanup;
+
+	wd = inotify_add_watch(fd, cg_control(memcg, "memory.events"), IN_DELETE_SELF);
+	if (wd == -1)
+		goto cleanup;
+
+	if (cg_destroy(memcg))
+		goto cleanup;
+	free(memcg);
+	memcg = NULL;
+
+	if (read_event(fd, IN_DELETE_SELF, wd))
+		goto cleanup;
+
+	if (read_event(fd, IN_IGNORED, wd))
+		goto cleanup;
+
+	ret = KSFT_PASS;
+
+cleanup:
+	if (fd >= 0)
+		close(fd);
+	if (memcg)
+		cg_destroy(memcg);
+	free(memcg);
+
+	return ret;
+}
+
 #define T(x) { x, #x }
 struct memcg_test {
 	int (*fn)(const char *root);
@@ -1644,6 +1764,8 @@ struct memcg_test {
 	T(test_memcg_oom_group_leaf_events),
 	T(test_memcg_oom_group_parent_events),
 	T(test_memcg_oom_group_score_events),
+	T(test_memcg_inotify_delete_file),
+	T(test_memcg_inotify_delete_rmdir),
 };
 #undef T
 
-- 
2.53.0.310.g728cabbaf7-goog
Re: [PATCH v3 3/3] selftests: memcg: Add tests IN_DELETE_SELF and IN_IGNORED on memory.events
Posted by Amir Goldstein 1 month, 1 week ago
On Wed, Feb 18, 2026 at 5:22 AM T.J. Mercier <tjmercier@google.com> wrote:
>
> Add two new tests that verify inotify events are sent when memcg files
> are removed.
>
> Signed-off-by: T.J. Mercier <tjmercier@google.com>
> Acked-by: Tejun Heo <tj@kernel.org>

Feel free to add:
Acked-by: Amir Goldstein <amir73il@gmail.com>

Although...

> ---
>  .../selftests/cgroup/test_memcontrol.c        | 122 ++++++++++++++++++
>  1 file changed, 122 insertions(+)
>
> diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> index 4e1647568c5b..2b065d03b730 100644
> --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
> @@ -10,6 +10,7 @@
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <unistd.h>
> +#include <sys/inotify.h>
>  #include <sys/socket.h>
>  #include <sys/wait.h>
>  #include <arpa/inet.h>
> @@ -1625,6 +1626,125 @@ static int test_memcg_oom_group_score_events(const char *root)
>         return ret;
>  }
>
> +static int read_event(int inotify_fd, int expected_event, int expected_wd)
> +{
> +       struct inotify_event event;
> +       ssize_t len = 0;
> +
> +       len = read(inotify_fd, &event, sizeof(event));
> +       if (len < (ssize_t)sizeof(event))
> +               return -1;
> +
> +       if (event.mask != expected_event || event.wd != expected_wd) {
> +               fprintf(stderr,
> +                       "event does not match expected values: mask %d (expected %d) wd %d (expected %d)\n",
> +                       event.mask, expected_event, event.wd, expected_wd);
> +               return -1;
> +       }
> +
> +       return 0;
> +}
> +
> +static int test_memcg_inotify_delete_file(const char *root)
> +{
> +       int ret = KSFT_FAIL;
> +       char *memcg = NULL, *child_memcg = NULL;
> +       int fd, wd;
> +
> +       memcg = cg_name(root, "memcg_test_0");
> +
> +       if (!memcg)
> +               goto cleanup;
> +
> +       if (cg_create(memcg))
> +               goto cleanup;
> +
> +       if (cg_write(memcg, "cgroup.subtree_control", "+memory"))
> +               goto cleanup;
> +
> +       child_memcg = cg_name(memcg, "child");
> +       if (!child_memcg)
> +               goto cleanup;
> +
> +       if (cg_create(child_memcg))
> +               goto cleanup;
> +
> +       fd = inotify_init1(0);
> +       if (fd == -1)
> +               goto cleanup;
> +
> +       wd = inotify_add_watch(fd, cg_control(child_memcg, "memory.events"), IN_DELETE_SELF);
> +       if (wd == -1)
> +               goto cleanup;
> +
> +       cg_write(memcg, "cgroup.subtree_control", "-memory");
> +
> +       if (read_event(fd, IN_DELETE_SELF, wd))
> +               goto cleanup;
> +
> +       if (read_event(fd, IN_IGNORED, wd))
> +               goto cleanup;
> +
> +       ret = KSFT_PASS;
> +
> +cleanup:
> +       if (fd >= 0)
> +               close(fd);
> +       if (child_memcg)
> +               cg_destroy(child_memcg);
> +       free(child_memcg);
> +       if (memcg)
> +               cg_destroy(memcg);
> +       free(memcg);
> +
> +       return ret;
> +}
> +
> +static int test_memcg_inotify_delete_rmdir(const char *root)
> +{
> +       int ret = KSFT_FAIL;
> +       char *memcg = NULL;
> +       int fd, wd;
> +
> +       memcg = cg_name(root, "memcg_test_0");
> +
> +       if (!memcg)
> +               goto cleanup;
> +
> +       if (cg_create(memcg))
> +               goto cleanup;
> +
> +       fd = inotify_init1(0);
> +       if (fd == -1)
> +               goto cleanup;
> +
> +       wd = inotify_add_watch(fd, cg_control(memcg, "memory.events"), IN_DELETE_SELF);
> +       if (wd == -1)
> +               goto cleanup;
> +
> +       if (cg_destroy(memcg))
> +               goto cleanup;
> +       free(memcg);
> +       memcg = NULL;
> +
> +       if (read_event(fd, IN_DELETE_SELF, wd))
> +               goto cleanup;
> +
> +       if (read_event(fd, IN_IGNORED, wd))
> +               goto cleanup;
> +
> +       ret = KSFT_PASS;
> +
> +cleanup:
> +       if (fd >= 0)
> +               close(fd);
> +       if (memcg)
> +               cg_destroy(memcg);
> +       free(memcg);
> +
> +       return ret;
> +}
> +
>  #define T(x) { x, #x }
>  struct memcg_test {
>         int (*fn)(const char *root);
> @@ -1644,6 +1764,8 @@ struct memcg_test {
>         T(test_memcg_oom_group_leaf_events),
>         T(test_memcg_oom_group_parent_events),
>         T(test_memcg_oom_group_score_events),
> +       T(test_memcg_inotify_delete_file),
> +       T(test_memcg_inotify_delete_rmdir),

How about another test case:
- Watch the cgroup directory (not the child file)
- Destroy cgroup
- Expect IN_DELETE_SELF | IN_ISDIR

I realize that this test won't pass with your implementation (right?)
but that is not ok IMO.

If we wish to make IN_DELETE_SELF available for kernfs,
it should not be confined to regular files IMO.

Thanks,
Amir.
Re: [PATCH v3 3/3] selftests: memcg: Add tests IN_DELETE_SELF and IN_IGNORED on memory.events
Posted by T.J. Mercier 1 month, 1 week ago
On Wed, Feb 18, 2026 at 12:31 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Wed, Feb 18, 2026 at 5:22 AM T.J. Mercier <tjmercier@google.com> wrote:
> >
> > Add two new tests that verify inotify events are sent when memcg files
> > are removed.
> >
> > Signed-off-by: T.J. Mercier <tjmercier@google.com>
> > Acked-by: Tejun Heo <tj@kernel.org>
>
> Feel free to add:
> Acked-by: Amir Goldstein <amir73il@gmail.com>

Thanks!

> Although...
>
> > ---
> >  .../selftests/cgroup/test_memcontrol.c        | 122 ++++++++++++++++++
> >  1 file changed, 122 insertions(+)
> >
> > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c
> > index 4e1647568c5b..2b065d03b730 100644
> > --- a/tools/testing/selftests/cgroup/test_memcontrol.c
> > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c
> > @@ -10,6 +10,7 @@
> >  #include <sys/stat.h>
> >  #include <sys/types.h>
> >  #include <unistd.h>
> > +#include <sys/inotify.h>
> >  #include <sys/socket.h>
> >  #include <sys/wait.h>
> >  #include <arpa/inet.h>
> > @@ -1625,6 +1626,125 @@ static int test_memcg_oom_group_score_events(const char *root)
> >         return ret;
> >  }
> >
> > +static int read_event(int inotify_fd, int expected_event, int expected_wd)
> > +{
> > +       struct inotify_event event;
> > +       ssize_t len = 0;
> > +
> > +       len = read(inotify_fd, &event, sizeof(event));
> > +       if (len < (ssize_t)sizeof(event))
> > +               return -1;
> > +
> > +       if (event.mask != expected_event || event.wd != expected_wd) {
> > +               fprintf(stderr,
> > +                       "event does not match expected values: mask %d (expected %d) wd %d (expected %d)\n",
> > +                       event.mask, expected_event, event.wd, expected_wd);
> > +               return -1;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static int test_memcg_inotify_delete_file(const char *root)
> > +{
> > +       int ret = KSFT_FAIL;
> > +       char *memcg = NULL, *child_memcg = NULL;
> > +       int fd, wd;
> > +
> > +       memcg = cg_name(root, "memcg_test_0");
> > +
> > +       if (!memcg)
> > +               goto cleanup;
> > +
> > +       if (cg_create(memcg))
> > +               goto cleanup;
> > +
> > +       if (cg_write(memcg, "cgroup.subtree_control", "+memory"))
> > +               goto cleanup;
> > +
> > +       child_memcg = cg_name(memcg, "child");
> > +       if (!child_memcg)
> > +               goto cleanup;
> > +
> > +       if (cg_create(child_memcg))
> > +               goto cleanup;
> > +
> > +       fd = inotify_init1(0);
> > +       if (fd == -1)
> > +               goto cleanup;
> > +
> > +       wd = inotify_add_watch(fd, cg_control(child_memcg, "memory.events"), IN_DELETE_SELF);
> > +       if (wd == -1)
> > +               goto cleanup;
> > +
> > +       cg_write(memcg, "cgroup.subtree_control", "-memory");
> > +
> > +       if (read_event(fd, IN_DELETE_SELF, wd))
> > +               goto cleanup;
> > +
> > +       if (read_event(fd, IN_IGNORED, wd))
> > +               goto cleanup;
> > +
> > +       ret = KSFT_PASS;
> > +
> > +cleanup:
> > +       if (fd >= 0)
> > +               close(fd);
> > +       if (child_memcg)
> > +               cg_destroy(child_memcg);
> > +       free(child_memcg);
> > +       if (memcg)
> > +               cg_destroy(memcg);
> > +       free(memcg);
> > +
> > +       return ret;
> > +}
> > +
> > +static int test_memcg_inotify_delete_rmdir(const char *root)
> > +{
> > +       int ret = KSFT_FAIL;
> > +       char *memcg = NULL;
> > +       int fd, wd;
> > +
> > +       memcg = cg_name(root, "memcg_test_0");
> > +
> > +       if (!memcg)
> > +               goto cleanup;
> > +
> > +       if (cg_create(memcg))
> > +               goto cleanup;
> > +
> > +       fd = inotify_init1(0);
> > +       if (fd == -1)
> > +               goto cleanup;
> > +
> > +       wd = inotify_add_watch(fd, cg_control(memcg, "memory.events"), IN_DELETE_SELF);
> > +       if (wd == -1)
> > +               goto cleanup;
> > +
> > +       if (cg_destroy(memcg))
> > +               goto cleanup;
> > +       free(memcg);
> > +       memcg = NULL;
> > +
> > +       if (read_event(fd, IN_DELETE_SELF, wd))
> > +               goto cleanup;
> > +
> > +       if (read_event(fd, IN_IGNORED, wd))
> > +               goto cleanup;
> > +
> > +       ret = KSFT_PASS;
> > +
> > +cleanup:
> > +       if (fd >= 0)
> > +               close(fd);
> > +       if (memcg)
> > +               cg_destroy(memcg);
> > +       free(memcg);
> > +
> > +       return ret;
> > +}
> > +
> >  #define T(x) { x, #x }
> >  struct memcg_test {
> >         int (*fn)(const char *root);
> > @@ -1644,6 +1764,8 @@ struct memcg_test {
> >         T(test_memcg_oom_group_leaf_events),
> >         T(test_memcg_oom_group_parent_events),
> >         T(test_memcg_oom_group_score_events),
> > +       T(test_memcg_inotify_delete_file),
> > +       T(test_memcg_inotify_delete_rmdir),
>
> How about another test case:
> - Watch the cgroup directory (not the child file)
> - Destroy cgroup
> - Expect IN_DELETE_SELF | IN_ISDIR
>
> I realize that this test won't pass with your implementation (right?)
> but that is not ok IMO.

Yes, that is correct, but I don't think any docs mention support for
monitoring kernfs directories, only files.

> If we wish to make IN_DELETE_SELF available for kernfs,
> it should not be confined to regular files IMO.

Based on what Honza discovered about i_nlink, it looks like we could
get VFS to send IN_DELETE_SELF for kernfs dirs with something like
this:

@@ -1540,6 +1558,9 @@ static void __kernfs_remove(struct kernfs_node *kn)
                         if (kernfs_type(pos) == KERNFS_FILE)
                                kernfs_notify_file_deleted(pos);
+                        else if (kernfs_type(pos) == KERNFS_DIR)
+                               kernfs_clear_inode_nlink(pos); <--
calls clear_nlink(inode); for every super

But I don't have a use for it at the moment.