Re: [PATCH] io_uring: ensure cq_entries is at least equal to or greater than sq_entries

[Jackie Liu <liuyun01@kylinos.cn>]

> 2019年10月24日 03:41，Jens Axboe <axboe@kernel.dk> 写道：
> 
> On 10/23/19 12:42 PM, Jeff Moyer wrote:
>> Jackie Liu <liuyun01@kylinos.cn> writes:
>> 
>>> If cq_entries is smaller than sq_entries, it will cause a lot of overflow
>>> to appear. when customizing cq_entries, at least let him be no smaller than
>>> sq_entries.
>>> 
>>> Fixes: 95d8765bd9f2 ("io_uring: allow application controlled CQ ring size")
>>> Signed-off-by: Jackie Liu <liuyun01@kylinos.cn>
>>> ---
>>>  fs/io_uring.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>> 
>>> diff --git a/fs/io_uring.c b/fs/io_uring.c
>>> index b64cd2c..dfa9731 100644
>>> --- a/fs/io_uring.c
>>> +++ b/fs/io_uring.c
>>> @@ -3784,7 +3784,7 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p)
>>>  		 * to a power-of-two, if it isn't already. We do NOT impose
>>>  		 * any cq vs sq ring sizing.
>>>  		 */
>>> -		if (!p->cq_entries || p->cq_entries > IORING_MAX_CQ_ENTRIES)
>>> +		if (p->cq_entries < p->sq_entries || p->cq_entries > IORING_MAX_CQ_ENTRIES)
>> 
>> What if they're both zero?  I think you want to keep that check.
> 
> sq_entries being zero is already checked and failed at this point.
> So I think the patch looks fine from that perspective.
> 
> Is there really a strong reason to disallow this? Yes, it could
> cause overflows, but it's just doing what was asked for. The
> normal case is of course cq_entries being much larger than
> sq_entries.
> 

There are actually no other stronger reasons. I think it would be better to do a
print job in liburing, but the kernel should still make a limit. Too many overflows
will cause less efficiency.

--
Jackie Liu