From: Ming Lei <ming.lei@redhat.com>
To: Ziyang Zhang <ZiyangZhang@linux.alibaba.com>
Cc: linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH] ublk_drv: don't call task_work_add for queueing io commands
Date: Thu, 27 Oct 2022 23:38:20 +0800 [thread overview]
Message-ID: <Y1ql7K2zpVL4LskH@T590> (raw)
In-Reply-To: <228487d2-373c-57ae-c60d-53989324908e@linux.alibaba.com>
On Thu, Oct 27, 2022 at 11:00:44AM +0800, Ziyang Zhang wrote:
> On 2022/10/26 19:29, Ming Lei wrote:
>
> [...]
> >>
> >> Intel(R) Xeon(R) Platinum 8369B CPU @ 2.70GHz 16 cores
> >> 64GB MEM, CentOS 8, kernel 6.0+
> >> with IORING_SETUP_COOP_TASKRUN, without this kernel patch
> >>
> >> ucmd: io_uring_cmd_complete_in_task(), ublk_drv is a module
> >>
> >> ucmd-not-touch-pdu: use llist && do not touch 'cmd'/'pdu'/'io' in ublk_queue_rq()
> >>
> >> tw: task_work_add(), ublk is built-in.
> >>
> >>
> >> --------
> >> fio test
> >>
> >> iodepth=128 numjobs=1 direct=1 bs=4k
> >>
> >> --------------------------------------------
> >> ublk loop target, the backend is a file.
> >>
> >> IOPS(k)
> >>
> >> type ucmd tw ucmd-not-touch-pdu
> >> seq-read 54.1 53.8 53.6
> >> rand-read 52.0 52.0 52.0
> >>
> >> --------------------------------------------
> >> ublk null target
> >> IOPS(k)
> >>
> >> type ucmd tw ucmd-not-touch-pdu
> >> seq-read 272 286 275
> >> rand-read 262 278 269
> >>
> >>
> >> ------------
> >> ublksrv test
> >>
> >> -------------
> >> ucmd
> >>
> >> running loop/001
> >> fio (ublk/loop( -f /root/work/ubdsrv/tests/tmp/ublk_loop_1G_BZ85U), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 66737
> >> randread: jobs 1, iops 64935
> >> randrw: jobs 1, iops read 32694 write 32710
> >> rw(512k): jobs 1, iops read 772 write 819
> >>
> >> running null/001
> >> fio (ublk/null(), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 715863
> >> randread: jobs 1, iops 758449
> >> randrw: jobs 1, iops read 357407 write 357183
> >> rw(512k): jobs 1, iops read 5895 write 5875
> >>
> >> -------------
> >> tw
> >>
> >> running loop/001
> >> fio (ublk/loop( -f /root/work/ubdsrv/tests/tmp/ublk_loop_1G_pvLTL), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 66856
> >> randread: jobs 1, iops 65015
> >> randrw: jobs 1, iops read 32751 write 32767
> >> rw(512k): jobs 1, iops read 776 write 823
> >>
> >> running null/001
> >> fio (ublk/null(), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 739450
> >> randread: jobs 1, iops 787500
> >> randrw: jobs 1, iops read 372956 write 372831
> >> rw(512k): jobs 1, iops read 5798 write 5777
> >>
> >> -------------
> >> ucmd-not-touch-pdu
> >>
> >> running loop/001
> >> fio (ublk/loop( -f /root/work/ubdsrv/tests/tmp/ublk_loop_1G_oH0eG), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 66754
> >> randread: jobs 1, iops 65032
> >> randrw: jobs 1, iops read 32776 write 32792
> >> rw(512k): jobs 1, iops read 772 write 818
> >>
> >> running null/001
> >> fio (ublk/null(), libaio, bs 4k, dio, hw queues:1, uring_comp: 0, get_data: 0)...
> >> randwrite: jobs 1, iops 725334
> >> randread: jobs 1, iops 741105
> >> randrw: jobs 1, iops read 360285 write 360047
> >> rw(512k): jobs 1, iops read 5770 write 5748
> >>
> >> Not touching cmd/pdu/io in ublk_queue_rq() improves IOPS.
> >> But it is worse than using task_work_add().
> >
> > Thanks for the test! It is better to not share ucmd between
> > ublk blk-mq io context and ubq daemon context, and we can
> > improve it for using io_uring_cmd_complete_in_task(), and I
> > have one patch by using the batch handing approach in
> > io_uring_cmd_complete_in_task().
> >
> > Another reason could be the extra __set_notify_signal() in
> > __io_req_task_work_add() via task_work_add(). When task_work_add()
> > is available, we just need to call __set_notify_signal() once
> > for the whole batch, but it can't be done in case of using
> > io_uring_cmd_complete_in_task().
> >
> > Also the patch of 'use llist' is actually wrong since we have to
> > call io_uring_cmd_complete_in_task() once in ->commit_rqs(), but
> > that couldn't be easy because ucmd isn't available at that time.
>
> Yes, you are correct.
>
> >
> > I think we may have to live with task_work_add() until the perf
> > number is improved to same basically with io_uring_cmd_complete_in_task().
>
> OK, we can keep task_work_add() && io_uring_cmd_complete_in_task().
> BTW, from test:loop/001, I think with real backends, the performance
> gap between them seems not too big.
Actually in VM created in my laptop, the gap isn't small on ublk-loop:
1) ublk-null, single job, change nr_hw_queue to 2 for using none scheduler
[root@ktest-09 ublksrv]# make test T=null/001:null/004
make -C tests run T=null/001:null/004 R=10 D=tests/tmp/
make[1]: Entering directory '/root/git/ublksrv/tests'
./run_test.sh null/001:null/004 10 tests/tmp/
running null/001
fio (ublk/null(), libaio, dio, hw queues:2, io jobs: 1, uring_comp: 0, get_data: 0)...
randwrite(4k): jobs 1, iops 1028774, cpu_util(29% 69%)
randread(4k): jobs 1, iops 958598, cpu_util(27% 72%)
randrw(4k): jobs 1, iops read 453382 write 453239, cpu_util(29% 70%)
rw(512k): jobs 1, iops read 5264 write 5242, cpu_util(7% 6%)
running null/004
fio (ublk/null(), libaio, dio, hw queues:2, io jobs: 1, uring_comp: 1, get_data: 0)...
randwrite(4k): jobs 1, iops 950755, cpu_util(30% 69%)
randread(4k): jobs 1, iops 855984, cpu_util(26% 73%)
randrw(4k): jobs 1, iops read 439369 write 439287, cpu_util(29% 70%)
rw(512k): jobs 1, iops read 5225 write 5202, cpu_util(7% 5%)
2) ublk-loop, single job, change nr_hw_queue to 2 for using none scheduler
[root@ktest-09 ublksrv]# make test T=loop/001:loop/004
make -C tests run T=loop/001:loop/004 R=10 D=tests/tmp/
make[1]: Entering directory '/root/git/ublksrv/tests'
./run_test.sh loop/001:loop/004 10 tests/tmp/
running loop/001
fio (ublk/loop( -f /root/git/ublksrv/tests/tmp/ublk_loop_2G_STbdZ), libaio, dio, hw queues:2, io jobs: 1, uring_comp: 0, get_data: 0)...
randwrite(4k): jobs 1, iops 163177, cpu_util(7% 20%)
randread(4k): jobs 1, iops 141584, cpu_util(6% 14%)
randrw(4k): jobs 1, iops read 75034 write 75148, cpu_util(7% 17%)
rw(512k): jobs 1, iops read 1574 write 1663, cpu_util(3% 4%)
running loop/004
fio (ublk/loop( -f /root/git/ublksrv/tests/tmp/ublk_loop_2G_AdrHl), libaio, dio, hw queues:2, io jobs: 1, uring_comp: 1, get_data: 0)...
randwrite(4k): jobs 1, iops 142353, cpu_util(7% 19%)
randread(4k): jobs 1, iops 133883, cpu_util(6% 14%)
randrw(4k): jobs 1, iops read 72581 write 72691, cpu_util(7% 18%)
rw(512k): jobs 1, iops read 929 write 981, cpu_util(2% 2%)
Thanks,
Ming
prev parent reply other threads:[~2022-10-27 15:38 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-23 9:38 [PATCH] ublk_drv: don't call task_work_add for queueing io commands Ming Lei
2022-10-24 9:48 ` Ziyang Zhang
2022-10-24 13:20 ` Ming Lei
2022-10-25 3:15 ` Ziyang Zhang
2022-10-25 7:19 ` Ming Lei
2022-10-25 7:46 ` Ziyang Zhang
2022-10-25 8:43 ` Ziyang Zhang
2022-10-25 15:17 ` Ming Lei
2022-10-26 10:32 ` Ziyang Zhang
2022-10-26 11:29 ` Ming Lei
2022-10-27 3:00 ` Ziyang Zhang
2022-10-27 15:38 ` Ming Lei [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y1ql7K2zpVL4LskH@T590 \
--to=ming.lei@redhat.com \
--cc=ZiyangZhang@linux.alibaba.com \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox