Re: [External] Re: [PATCH v2] io_uring: add IORING_SETUP_SQTHREAD_STATS flag to enable sqthread stats collection

[Fengnan Chang <changfengnan@bytedance.com>]

Jens Axboe <axboe@kernel.dk> 于2025年10月20日周一 23:12写道：
>
> On 10/20/25 5:30 AM, Fengnan Chang wrote:
> > In previous versions, getrusage was always called in sqrthread
> > to count work time, but this could incur some overhead.
> > This patch turn off stats by default, and introduces a new flag
> > IORING_SETUP_SQTHREAD_STATS that allows user to enable the
> > collection of statistics in the sqthread.
> >
> > ./t/io_uring -p1 -d128 -b4096 -s32 -c1 -F1 -B1 -R1 -X1 -n1 ./testfile
> > IOPS base: 570K, patch: 590K
> >
> > ./t/io_uring -p1 -d128 -b4096 -s32 -c1 -F1 -B1 -R1 -X1 -n1 /dev/nvme1n1
> > IOPS base: 826K, patch: 889K
>
> That's a crazy amount of overhead indeed. I'm assuming this is
> because the task has lots of threads? And/or going through the retry
> a lot? Might make more sense to have a cheaper and more rough
> getrusage() instead? All we really use is ru_stime.{tv_sec,tv_nsec}.
> Should it be using RUSAGE_THREAD? Correct me if I'm wrong, but using
> PTHREAD_SELF actually seems wrong as-is.

Only one sqpoll thread, no retry.  Only enable sq_thread_poll by default in
 ./t/io_uring.c to test.
Yeh, getrusage is not correct, I'll try to find a cheaper way.

>
> In any case, I don't think a setup flag is the right choice here. That
> space is fairly limited, and SQPOLL is also a bit of an esoteric
> feature. Hence not sure a setup flag is the right approach. Would
> probably make more sense to have a REGISTER opcode to get/set various
> features like this, I'm sure it's not the last thing like this we'll run
> into. But as mentioned, I do think just having a more light weight
> getrusage would perhaps be prudent.
Get your point,  I'll make it in REGISTER opcode.
>
> --
> Jens Axboe