Re: [PATCH RFC 5.13 1/2] io_uring: add support for ns granularity of io_sq_thread_idle

[Jens Axboe <axboe@kernel.dk>]

On 4/28/21 8:53 AM, Pavel Begunkov wrote:
> On 4/28/21 3:16 PM, Jens Axboe wrote:
>> On 4/28/21 8:07 AM, Pavel Begunkov wrote:
>>>> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
>>>> index e1ae46683301..311532ff6ce3 100644
>>>> --- a/include/uapi/linux/io_uring.h
>>>> +++ b/include/uapi/linux/io_uring.h
>>>> @@ -98,6 +98,7 @@ enum {
>>>>  #define IORING_SETUP_CLAMP	(1U << 4)	/* clamp SQ/CQ ring sizes */
>>>>  #define IORING_SETUP_ATTACH_WQ	(1U << 5)	/* attach to existing wq */
>>>>  #define IORING_SETUP_R_DISABLED	(1U << 6)	/* start with ring disabled */
>>>> +#define IORING_SETUP_IDLE_NS	(1U << 7)	/* unit of thread_idle is nano second */
>>>>  
>>>>  enum {
>>>>  	IORING_OP_NOP,
>>>> @@ -259,7 +260,7 @@ struct io_uring_params {
>>>>  	__u32 cq_entries;
>>>>  	__u32 flags;
>>>>  	__u32 sq_thread_cpu;
>>>> -	__u32 sq_thread_idle;
>>>> +	__u64 sq_thread_idle;
>>>
>>> breaks userspace API
>>
>> And I don't think we need to. If you're using IDLE_NS, then the value
>> should by definition be small enough that it'd fit in 32-bits. If you
>> need higher timeouts, don't set it and it's in usec instead.
>>
>> So I'd just leave this one alone.
> 
> Sounds good
> 
> u64 time_ns = p->sq_thread_idle;
> if (!IDLE_NS)
>     time_ns *= NSEC_PER_MSEC;
> idel = ns_to_jiffies(time_ns);

Precisely! With the overlap being there, there's no need to make it bigger.
And having nsec granularity if your idle time is in the msecs doesn't make
a lot of sense.

-- 
Jens Axboe