This is the third version of fixed worker implementation.
Wrote a nop test program to test it, 3 fixed-workers VS 3 normal workers.
normal workers:
./run_nop_wqe.sh nop_wqe_normal 200000 100 3 1-3
        time spent: 10464397 usecs      IOPS: 1911242
        time spent: 9610976 usecs       IOPS: 2080954
        time spent: 9807361 usecs       IOPS: 2039284

fixed workers:
./run_nop_wqe.sh nop_wqe_fixed 200000 100 3 1-3
        time spent: 17314274 usecs      IOPS: 1155116
        time spent: 17016942 usecs      IOPS: 1175299
        time spent: 17908684 usecs      IOPS: 1116776

About 2x improvement. From perf result, almost no acct->lock contension.
Test program: https://github.com/HowHsu/liburing/tree/fixed_worker
liburing/test/nop_wqe.c

v2->v3:
 - change dispatch work strategy from random to round-robin

things to be done:
 - Still need some thinking about the work cancellation
 - not very sure IO_WORKER_F_EXIT is safe enough on synchronization
 - the iowq hash stuff is not compatible with fixed worker for now

Any comments are welcome. Thanks in advance.

Hao Xu (9):
  io-wq: add a worker flag for individual exit
  io-wq: change argument of create_io_worker() for convienence
  io-wq: add infra data structure for fixed workers
  io-wq: tweak io_get_acct()
  io-wq: fixed worker initialization
  io-wq: fixed worker exit
  io-wq: implement fixed worker logic
  io-wq: batch the handling of fixed worker private works
  io_uring: add register fixed worker interface

 fs/io-wq.c                    | 460 ++++++++++++++++++++++++++++++----
 fs/io-wq.h                    |   8 +
 fs/io_uring.c                 |  71 ++++++
 include/uapi/linux/io_uring.h |  11 +
 4 files changed, 501 insertions(+), 49 deletions(-)

-- 
2.36.0

On 4/29/22 4:18 AM, Hao Xu wrote:
> This is the third version of fixed worker implementation.
> Wrote a nop test program to test it, 3 fixed-workers VS 3 normal workers.
> normal workers:
> ./run_nop_wqe.sh nop_wqe_normal 200000 100 3 1-3
>         time spent: 10464397 usecs      IOPS: 1911242
>         time spent: 9610976 usecs       IOPS: 2080954
>         time spent: 9807361 usecs       IOPS: 2039284
> 
> fixed workers:
> ./run_nop_wqe.sh nop_wqe_fixed 200000 100 3 1-3
>         time spent: 17314274 usecs      IOPS: 1155116
>         time spent: 17016942 usecs      IOPS: 1175299
>         time spent: 17908684 usecs      IOPS: 1116776

I saw these numbers in v2 as well, and I have to admit I don't
understand them. Because on the surface, it sure looks like the first
set of results (labeled "normal") are better than the second "fixed"
set. Am I reading them wrong, or did you transpose them?

I think this patch series would benefit from a higher level description
of what fixed workers mean in this context. How are they different from
the existing workers, and why would it improve things.

> things to be done:
>  - Still need some thinking about the work cancellation

Can you expand? What are the challenges with fixed workers and
cancelation?

>  - not very sure IO_WORKER_F_EXIT is safe enough on synchronization
>  - the iowq hash stuff is not compatible with fixed worker for now

We might need to extract the hashing out a bit so it's not as tied to
the existing implementation.

-- 
Jens Axboe

On 4/30/22 21:11, Jens Axboe wrote:
> On 4/29/22 4:18 AM, Hao Xu wrote:
>> This is the third version of fixed worker implementation.
>> Wrote a nop test program to test it, 3 fixed-workers VS 3 normal workers.
>> normal workers:
>> ./run_nop_wqe.sh nop_wqe_normal 200000 100 3 1-3
>>          time spent: 10464397 usecs      IOPS: 1911242
>>          time spent: 9610976 usecs       IOPS: 2080954
>>          time spent: 9807361 usecs       IOPS: 2039284
>>
>> fixed workers:
>> ./run_nop_wqe.sh nop_wqe_fixed 200000 100 3 1-3
>>          time spent: 17314274 usecs      IOPS: 1155116
>>          time spent: 17016942 usecs      IOPS: 1175299
>>          time spent: 17908684 usecs      IOPS: 1116776
> 
> I saw these numbers in v2 as well, and I have to admit I don't
> understand them. Because on the surface, it sure looks like the first
> set of results (labeled "normal") are better than the second "fixed"
> set. Am I reading them wrong, or did you transpose them?
Sorry, I transposed them..
> 
> I think this patch series would benefit from a higher level description
> of what fixed workers mean in this context. How are they different from
> the existing workers, and why would it improve things.
Sure, put that in the Patch 7/9, I'll move it to the cover letter as
well.
> 
>> things to be done:
>>   - Still need some thinking about the work cancellation
> 
> Can you expand? What are the challenges with fixed workers and
> cancelation?
Currently, when a fixed worker fetch all the works from its private work
list, I use a temporary acct struct to hold them. This means at that
moment the cancellation cannot find these works which are going to run
but not in the private work list already. This won't be a big problem,
another acct member in io_worker{} should be good enough to resolve
that.
> 
>>   - not very sure IO_WORKER_F_EXIT is safe enough on synchronization
>>   - the iowq hash stuff is not compatible with fixed worker for now
> 
> We might need to extract the hashing out a bit so it's not as tied to
> the existing implementation.
>