Re: Re: [PATCH liburing] test: add test cases for hybrid iopoll

[hexue <xue01.he@samsung.com>]

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