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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c0eb2b6f-1145-4ce3-a2b3-98d32cdfa623@kernel.dk>
Date: Tue, 12 Nov 2024 08:43:14 -0700
From: Jens Axboe <axboe@...nel.dk>
To: Anuj Gupta <anuj20.g@...sung.com>, hexue <xue01.he@...sung.com>
Cc: asml.silence@...il.com, io-uring@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH liburing] test: add test cases for hybrid iopoll

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