Re: [PATCH 2/3] io_uring: avoid whole io_wq_work copy for inline requests

[Pavel Begunkov <asml.silence@gmail.com>]

On 26/05/2020 09:43, Xiaoguang Wang wrote:
> If requests can be submitted inline, we don't need to copy whole
> io_wq_work in io_init_req(), which is an expensive operation. I
> use my io_uring_nop_stress to evaluate performance improvement.
> 
> In my physical machine, before this patch:
> $sudo taskset -c 60 ./io_uring_nop_stress -r 120
> total ios: 749093872
> IOPS:      6242448
> 
> $sudo taskset -c 60 ./io_uring_nop_stress -r 120
> total ios: 786083712
> IOPS:      6550697
> 
> About 4.9% improvement.

Interesting, what's the contribution of fast check in *drop_env() separately
from not zeroing req->work.

> 
> Signed-off-by: Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>
> ---
>  fs/io-wq.h    | 13 +++++++++----
>  fs/io_uring.c | 33 ++++++++++++++++++++++++++-------
>  2 files changed, 35 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/io-wq.h b/fs/io-wq.h
> index 5ba12de7572f..11d981a67006 100644
> --- a/fs/io-wq.h
> +++ b/fs/io-wq.h
> @@ -94,10 +94,15 @@ struct io_wq_work {
>  	pid_t task_pid;
>  };
>  
> -#define INIT_IO_WORK(work, _func)				\
> -	do {							\
> -		*(work) = (struct io_wq_work){ .func = _func };	\
> -	} while (0)						\
> +static inline void init_io_work(struct io_wq_work *work,
> +		void (*func)(struct io_wq_work **))
> +{
> +	if (!work->func)

Not really a good name for a function, which expects some of the fields to be
already initialised, too subtle. Unfortunately, it'll break at some point.

I think, a flag in req->flags for that would be better.
e.g. REQ_F_WORK_INITIALIZED

And it'd be better to somehow separate field of struct io_wq_work,
1. always initialised ones like ->func (and maybe ->creds).
2. lazy initialised controlled by the mentioned flag.


> +		*(work) = (struct io_wq_work){ .func = func };
> +	else
> +		work->func = func;
> +}
> +
>
-- 
Pavel Begunkov