Re: [PATCH 3/5] cgroup: selftests: Move cgroup_util into its own library

From: James Houghton
Date: Thu Mar 27 2025 - 14:08:05 EST


On Thu, Mar 27, 2025 at 2:43 AM Michal Koutný <mkoutny@xxxxxxxx> wrote:
>
> Hello James.
>
> On Thu, Mar 27, 2025 at 01:23:48AM +0000, James Houghton <jthoughton@xxxxxxxxxx> wrote:
> > KVM selftests will soon need to use some of the cgroup creation and
> > deletion functionality from cgroup_util.
>
> Thanks, I think cross-selftest sharing is better than duplicating
> similar code.
>
> +Cc: Yafang as it may worth porting/unifying with
> tools/testing/selftests/bpf/cgroup_helpers.h too
>
> > --- a/tools/testing/selftests/cgroup/cgroup_util.c
> > +++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
> > @@ -16,8 +16,7 @@
> >  #include <sys/wait.h>
> >  #include <unistd.h>
> >
> > -#include "cgroup_util.h"
> > -#include "../clone3/clone3_selftests.h"
> > +#include <cgroup_util.h>
>
> The clone3_selftests.h header is not needed anymore?

Ah, sorry.

We do indeed still reference `sys_clone3()` from cgroup_util.c, so it should
stay in (as "../../clone3/clone3_selftests.h"). I realize now that it compiled
just fine because the call to `sys_clone3()` is dropped entirely when
clone3_selftests.h is not included.

So I'll apply the following diff:

diff --git a/tools/testing/selftests/cgroup/lib/cgroup_util.c b/tools/testing/selftests/cgroup/lib/cgroup_util.c
index d5649486a11df..fe15541f3a07d 100644
--- a/tools/testing/selftests/cgroup/lib/cgroup_util.c
+++ b/tools/testing/selftests/cgroup/lib/cgroup_util.c
@@ -18,6 +18,8 @@

#include <cgroup_util.h>

+#include "../../clone3/clone3_selftests.h"
+
/* Returns read len on success, or -errno on failure. */
static ssize_t read_text(const char *path, char *buf, size_t max_len)
{
diff --git a/tools/testing/selftests/cgroup/lib/libcgroup.mk b/tools/testing/selftests/cgroup/lib/libcgroup.mk
index 2cbf07337c23f..12323041a5ce6 100644
--- a/tools/testing/selftests/cgroup/lib/libcgroup.mk
+++ b/tools/testing/selftests/cgroup/lib/libcgroup.mk
@@ -6,7 +6,9 @@ LIBCGROUP_O := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBCGROUP_C))

CFLAGS += -I$(CGROUP_DIR)/lib/include

-$(LIBCGROUP_O): $(OUTPUT)/%.o : $(CGROUP_DIR)/%.c
+EXTRA_HDRS := $(selfdir)/clone3/clone3_selftests.h
+
+$(LIBCGROUP_O): $(OUTPUT)/%.o : $(CGROUP_DIR)/%.c $(EXTRA_HDRS)
$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@

EXTRA_CLEAN += $(LIBCGROUP_O)