On 11/12/24 15:43, Jens Axboe wrote: >On 11/12/24 3:44 AM, Anuj Gupta wrote: >>> +utilization than polling. Similarly, this feature also requires the devices >>> +to support polling configuration. >> This feature would work if a device doesn't have polled queues,right? >> The performance might be suboptimal in that case, but the userspace won't >> get any errors. >We've traditionally been a mix of lax and strict on this. IMHO we should >return -EOPTNOTSUPP for IOPOLL (and IOPOLL|HYBRID) if polling isn't >configured correctly. I've seen way too many not realize that they need >to configure their nvme side for pollable queues for it to do what it >needs to do. If you don't and it's just allowed, then you don't really >get much of a win, you're just burning CPU. > >Hence I do think that this should strongly recommend that the devices >support polling, that part is fine. > >Agree with your other comments, thanks for reviewing it! >> This patch mostly looks fine. But the code here seems to be largely >> duplicated from "test/io_uring_passthrough.c" and "test/iopoll.c". >> Can we consider adding the hybrid poll test as a part of the existing >> tests as it seems that it would only require passing a extra flag >> during ring setup. > >Yeah I do think modifying test/iopoll.c to test all the same >configurations but with HYBRID added would be the way to go, rather than >duplicate all of this. Ditto for passthrough. Got it, will add it and submit v2 patch. -- hexue
