linux-kernel - Re: [PATCH] perf tools: sched-pipe bench: add (-n) nonblocking benchmark

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fWy7geJGM-T3XPjbbmdYg4spj4H9s+jE_+aqCSEUJeSFQ@mail.gmail.com>
Date: Wed, 16 Oct 2024 12:27:50 -0700
From: Ian Rogers <irogers@...gle.com>
To: Brian Geffon <bgeffon@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, 
	Arnaldo Carvalho de Melo <acme@...nel.org>, linux-perf-users@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Adrian Hunter <adrian.hunter@...el.com>, 
	Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, 
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH] perf tools: sched-pipe bench: add (-n) nonblocking benchmark

On Wed, Oct 16, 2024 at 12:00 PM Brian Geffon <bgeffon@...gle.com> wrote:
>
> The -n mode will benchmark pipes in a non-blocking mode using
> epoll_wait.
>
> This specific mode was added to demonstrate the broken sync nature
> of epoll: https://lore.kernel.org/lkml/20240426-zupfen-jahrzehnt-5be786bcdf04@brauner
>
> Signed-off-by: Brian Geffon <bgeffon@...gle.com>

Reviewed-by: Ian Rogers <irogers@...gle.com>

I liked the commentary in:
https://lore.kernel.org/lkml/ZxAOgj9RWm4NTl9d@google.com/

Thanks,
Ian

> ---
>  tools/perf/bench/sched-pipe.c | 43 +++++++++++++++++++++++++++++------
>  1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/bench/sched-pipe.c b/tools/perf/bench/sched-pipe.c
> index 3af6d3c55aba..e2562677df96 100644
> --- a/tools/perf/bench/sched-pipe.c
> +++ b/tools/perf/bench/sched-pipe.c
> @@ -23,6 +23,7 @@
>  #include <errno.h>
>  #include <fcntl.h>
>  #include <assert.h>
> +#include <sys/epoll.h>
>  #include <sys/time.h>
>  #include <sys/types.h>
>  #include <sys/syscall.h>
> @@ -34,6 +35,8 @@ struct thread_data {
>         int                     nr;
>         int                     pipe_read;
>         int                     pipe_write;
> +       struct epoll_event      epoll_ev;
> +       int                     epoll_fd;
>         bool                    cgroup_failed;
>         pthread_t               pthread;
>  };
> @@ -44,6 +47,7 @@ static        int                     loops = LOOPS_DEFAULT;
>  /* Use processes by default: */
>  static bool                    threaded;
>
> +static bool                    nonblocking;
>  static char                    *cgrp_names[2];
>  static struct cgroup           *cgrps[2];
>
> @@ -81,6 +85,7 @@ static int parse_two_cgroups(const struct option *opt __maybe_unused,
>  }
>
>  static const struct option options[] = {
> +       OPT_BOOLEAN('n', "nonblocking", &nonblocking,   "Use non-blocking operations"),
>         OPT_INTEGER('l', "loop",        &loops,         "Specify number of loops"),
>         OPT_BOOLEAN('T', "threaded",    &threaded,      "Specify threads/process based task setup"),
>         OPT_CALLBACK('G', "cgroups", NULL, "SEND,RECV",
> @@ -165,11 +170,25 @@ static void exit_cgroup(int nr)
>         free(cgrp_names[nr]);
>  }
>
> +static inline int read_pipe(struct thread_data *td)
> +{
> +       int ret, m;
> +retry:
> +       if (nonblocking) {
> +               ret = epoll_wait(td->epoll_fd, &td->epoll_ev, 1, -1);
> +               if (ret < 0)
> +                       return ret;
> +       }
> +       ret = read(td->pipe_read, &m, sizeof(int));
> +       if (nonblocking && ret < 0 && errno == EWOULDBLOCK)
> +               goto retry;
> +       return ret;
> +}
> +
>  static void *worker_thread(void *__tdata)
>  {
>         struct thread_data *td = __tdata;
> -       int m = 0, i;
> -       int ret;
> +       int i, ret, m = 0;
>
>         ret = enter_cgroup(td->nr);
>         if (ret < 0) {
> @@ -177,16 +196,23 @@ static void *worker_thread(void *__tdata)
>                 return NULL;
>         }
>
> +       if (nonblocking) {
> +               td->epoll_ev.events = EPOLLIN;
> +               td->epoll_fd = epoll_create(1);
> +               BUG_ON(td->epoll_fd < 0);
> +               BUG_ON(epoll_ctl(td->epoll_fd, EPOLL_CTL_ADD, td->pipe_read, &td->epoll_ev) < 0);
> +       }
> +
>         for (i = 0; i < loops; i++) {
>                 if (!td->nr) {
> -                       ret = read(td->pipe_read, &m, sizeof(int));
> +                       ret = read_pipe(td);
>                         BUG_ON(ret != sizeof(int));
>                         ret = write(td->pipe_write, &m, sizeof(int));
>                         BUG_ON(ret != sizeof(int));
>                 } else {
>                         ret = write(td->pipe_write, &m, sizeof(int));
>                         BUG_ON(ret != sizeof(int));
> -                       ret = read(td->pipe_read, &m, sizeof(int));
> +                       ret = read_pipe(td);
>                         BUG_ON(ret != sizeof(int));
>                 }
>         }
> @@ -209,13 +235,16 @@ int bench_sched_pipe(int argc, const char **argv)
>          * discarding returned value of read(), write()
>          * causes error in building environment for perf
>          */
> -       int __maybe_unused ret, wait_stat;
> +       int __maybe_unused ret, wait_stat, flags = 0;
>         pid_t pid, retpid __maybe_unused;
>
>         argc = parse_options(argc, argv, options, bench_sched_pipe_usage, 0);
>
> -       BUG_ON(pipe(pipe_1));
> -       BUG_ON(pipe(pipe_2));
> +       if (nonblocking)
> +               flags |= O_NONBLOCK;
> +
> +       BUG_ON(pipe2(pipe_1, flags));
> +       BUG_ON(pipe2(pipe_2, flags));
>
>         gettimeofday(&start, NULL);
>
> --
> 2.47.0.rc1.288.g06298d1525-goog
>
>