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. -- Jens Axboe
