Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH] perf bench sched: Add --threaded 0/1 option | Date | Mon, 23 Sep 2013 15:52:33 +0900 |
| |
Hi Ingo,
Just a few nitpicks...
On Tue, 17 Sep 2013 13:42:56 +0200, Ingo Molnar wrote: > Allow the measurement of thread versus process context switch performance. > > The default stays at 'process' based measurement, like lmbench's lat_ctx > benchmark. > > Sample output: > > comet:~/tip/tools/perf> taskset 1 ./perf bench sched pipe --threaded 0 > # Running sched/pipe benchmark... > # Executed 1000000 pipe operations between two processes > > Total time: 4.138 [sec] > > 4.138729 usecs/op > 241620 ops/sec > comet:~/tip/tools/perf> taskset 1 ./perf bench sched pipe --threaded 1
Why not just make it boolean? IMHO the integer argument can confuse users as they can think it a number of threads..
> # Running sched/pipe benchmark... > # Executed 1000000 pipe operations between two threads > > Total time: 3.667 [sec] > > 3.667667 usecs/op > 272652 ops/sec > > Signed-off-by: Ingo Molnar <mingo@kernel.org> > > diff --git a/tools/perf/bench/sched-pipe.c b/tools/perf/bench/sched-pipe.c > index 69cfba8..7630dc6 100644 > --- a/tools/perf/bench/sched-pipe.c > +++ b/tools/perf/bench/sched-pipe.c > @@ -28,12 +28,25 @@ > #include <sys/time.h> > #include <sys/types.h> > > +#include <pthread.h> > + > +struct thread_data { > + int nr; > + int pipe_read; > + int pipe_write; > + pthread_t pthread; > +}; > + > #define LOOPS_DEFAULT 1000000 > static int loops = LOOPS_DEFAULT; > > +static int threaded = 0; > + > static const struct option options[] = { > OPT_INTEGER('l', "loop", &loops, > "Specify number of loops"), > + OPT_INTEGER('T', "threaded", &threaded, > + "Specify threaded/process based task setup"), > OPT_END() > }; > > @@ -42,13 +55,38 @@ static const char * const bench_sched_pipe_usage[] = { > NULL > }; > > +static void *worker_thread(void *__tdata) > +{ > + struct thread_data *td = __tdata;
Whitespace damaged. Otherwise looks good to me!
Thanks, Namhyung
> + int m = 0, i; > + int ret; > + > + for (i = 0; i < loops; i++) { > + if (!td->nr) { > + ret = read(td->pipe_read, &m, sizeof(int)); > + 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)); > + BUG_ON(ret != sizeof(int)); > + } > + } > + > + return NULL; > +}
| |