Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Sender: Ingo Molnar <mingo.kernel.org@gmail.com>
Date:   Sat, 14 Oct 2023 10:44:09 +0200
From:   Ingo Molnar <mingo@kernel.org>
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>, Ian Rogers <irogers@google.com>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v2] perf bench sched pipe: Add -G/--cgroups option
Message-ID: <ZSpU2Q7A9ViZe7DB@gmail.com>
References: <20231013232435.1012585-1-namhyung@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231013232435.1012585-1-namhyung@kernel.org>
Precedence: bulk


* Namhyung Kim <namhyung@kernel.org> wrote:

> +	cgrp_send = cgroup__new(p, /*do_open=*/true);
> +	if (cgrp_send == NULL) {
> +		fprintf(stderr, "cannot open sender cgroup: %s", p);
> +		goto out;
> +	}

Maybe in this case print out a small suggestion of how to create this 
particular cgroup?

Most distro users and even kernel developers don't ever have to create
new cgroups.

Maybe even allow the creation of new cgroups for this testing, if they 
don't already exist? As long as we don't delete any cgroups I don't think 
much harm can be done - and the increase in usability is substantial.

> +static void enter_cgroup(struct cgroup *cgrp)
> +{
> +	char buf[32];
> +	int fd, len;
> +	pid_t pid;
> +
> +	if (cgrp == NULL)
> +		return;
> +
> +	if (threaded)
> +		pid = syscall(__NR_gettid);
> +	else
> +		pid = getpid();
> +
> +	snprintf(buf, sizeof(buf), "%d\n", pid);
> +	len = strlen(buf);
> +
> +	/* try cgroup v2 interface first */
> +	if (threaded)
> +		fd = openat(cgrp->fd, "cgroup.threads", O_WRONLY);
> +	else
> +		fd = openat(cgrp->fd, "cgroup.procs", O_WRONLY);
> +
> +	/* try cgroup v1 if failed */
> +	if (fd < 0)
> +		fd = openat(cgrp->fd, "tasks", O_WRONLY);
> +
> +	if (fd < 0) {
> +		printf("failed to open cgroup file in %s\n", cgrp->name);
> +		return;
> +	}
> +
> +	if (write(fd, buf, len) != len)
> +		printf("cannot enter to cgroup: %s\n", cgrp->name);

The failures here should probably result in termination of the run with an 
error code, not just messages which are easy to skip in automated tests?

Thanks,

	Ingo