Re: [PATCH v3] perf bench: add --write-size option to sched pipe
From: Breno Leitao
Date: Mon Jun 01 2026 - 07:11:04 EST
Hello Namhyung,
On Wed, May 27, 2026 at 10:31:10AM -0700, Namhyung Kim wrote:
> On Wed, May 27, 2026 at 06:42:59AM -0700, Breno Leitao wrote:
> > +static inline int write_pipe(struct thread_data *td)
> > +{
> > + unsigned int done = 0;
> > + int ret;
> > +
> > + while (done < write_size) {
> > + ret = write(td->pipe_write, td->buf + done, write_size - done);
> > + if (ret < 0) {
> > + if (nonblocking && errno == EWOULDBLOCK)
> > + continue;
>
> Don't we also need the blocking part?
Just to make sure I'm reading this right: do you mean the writer
should epoll_wait() on EPOLLOUT before retrying (symmetric to what
read_pipe() does for EPOLLIN), so we sleep until the pipe drains
instead of spinning on EWOULDBLOCK? Or are you pointing at something
else — e.g. behavior in the blocking (!nonblocking) path?
> > + if (ret == 0)
> > return ret;
>
> Maybe it doesn't matter.. but shouldn't it return 'done' instead?
Ack!
>
> > + done += ret;
> > }
> > - ret = read(td->pipe_read, &m, sizeof(int));
> > - if (nonblocking && ret < 0 && errno == EWOULDBLOCK)
> > - goto retry;
> > - return ret;
> > + return done;
> > }
> >
> > static void *worker_thread(void *__tdata)
> > {
> > struct thread_data *td = __tdata;
> > - int i, ret, m = 0;
> > + int i, ret;
> >
> > ret = enter_cgroup(td->nr);
> > if (ret < 0) {
> > @@ -204,15 +241,38 @@ static void *worker_thread(void *__tdata)
> > }
> >
> > for (i = 0; i < loops; i++) {
> > - ret = write(td->pipe_write, &m, sizeof(int));
> > - BUG_ON(ret != sizeof(int));
> > + ret = write_pipe(td);
> > + BUG_ON(ret < 0 || (unsigned int)ret != write_size);
> > ret = read_pipe(td);
> > - BUG_ON(ret != sizeof(int));
> > + BUG_ON(ret < 0 || (unsigned int)ret != write_size);
>
> Nit: maybe comparing to the write_size is enough as it cannot be
> negative or bigger than INTMAX.
Ack, the following should be enough:
BUG_ON(ret != (int)write_size);
Thanks for the review,
--breno