Re: [PATCH v3] perf bench: add --write-size option to sched pipe

[Breno Leitao <leitao@debian.org>]

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