Re: SQPOLL / uring_cmd_iopoll

[Jens Axboe <axboe@kernel.dk>]

On 4/22/23 6:55?AM, Ming Lei wrote:
> On Fri, Apr 21, 2023 at 10:09:36PM +0000, Bernd Schubert wrote:
>> Hello,
>>
>> I was wondering if I could set up SQPOLL for fuse/IORING_OP_URING_CMD 
>> and what would be the latency win. Now I get a bit confused what the 
>> f_op->uring_cmd_iopoll() function is supposed to do.
>>
>> Is it just there to check if SQEs are can be completed as CQE? In rw.c 
>> io_do_iopoll() it looks like this. I don't follow all code paths in 
>> __io_sq_thread yet, but it looks a like it already checks if the ring 
>> has new entries
>>
>> to_submit = io_sqring_entries(ctx);
>> ...
>> ret = io_submit_sqes(ctx, to_submit);
>>
>>    --> it will eventually call into ->uring_cmd() ?
>>
>> And then io_do_iopoll ->  file->f_op->uring_cmd_iopoll is supposed to 
>> check for available cq entries and will submit these? I.e. I just return 
>> 1 if when the request is ready? And also ensure that 
>> req->iopoll_completed is set?
>>
>>
>> I'm also not sure what I should do with struct io_comp_batch * - I don't 
>> have struct request *req_list anywhere in my fuse-uring changes, seems 
>> to be blk-mq specific? So I should just ignore that parameter?
>>
>>
>> Btw, this might be useful for ublk as well?
> 
> For the in-tree ublk driver, we need to copy data inside ->uring_cmd()
> between block request pages and user buffer, so SQPOLL may not be done
> because it isn't efficient for the kthread to copy on remote task mm
> space. However, ublk user copy feature[1](posted recently) doesn't
> need the copy in ->uring_cmd() any more, so SQPOLL becomes possible for
> ublk uring cmd.

That hasn't been true for a long time, and isn't even true in
5.10-stable anymore or anything newer. The SQPOLL thread is not a
kthread, and it doesn't need to do anything to copy the data that the
inline submission wouldn't also do. There is no "remote task mm". The
cost would be the same, outside of caching effects.

-- 
Jens Axboe