linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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
Date: Sat, 14 Oct 2023 10:44:09 +0200	[thread overview]
Message-ID: <ZSpU2Q7A9ViZe7DB@gmail.com> (raw)
In-Reply-To: <20231013232435.1012585-1-namhyung@kernel.org>


* 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

  reply	other threads:[~2023-10-14  8:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-13 23:24 [PATCH v2] perf bench sched pipe: Add -G/--cgroups option Namhyung Kim
2023-10-14  8:44 ` Ingo Molnar [this message]
2023-10-15 18:53   ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZSpU2Q7A9ViZe7DB@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).