Re: [PATCH 1/5] io_uring: optimise iowq refcounting

[Jens Axboe <axboe@kernel.dk>]

On 8/14/21 1:31 PM, Pavel Begunkov wrote:
> On 8/14/21 8:13 PM, Jens Axboe wrote:
>> On 8/14/21 10:26 AM, Pavel Begunkov wrote:
>>> If a requests is forwarded into io-wq, there is a good chance it hasn't
>>> been refcounted yet and we can save one req_ref_get() by setting the
>>> refcount number to the right value directly.
>>
>> Not sure this really matters, but can't hurt either. But...
> 
> The refcount patches made this one atomic worse, and I just prefer
> to not regress, even if slightly

Not really against it, but doubt it's measurable if you end up hitting
the io-wq slower path anyway. But as I said, can't really hurt, so not
aginst it.

>>> @@ -1115,14 +1115,19 @@ static inline void req_ref_get(struct io_kiocb *req)
>>>  	atomic_inc(&req->refs);
>>>  }
>>>  
>>> -static inline void io_req_refcount(struct io_kiocb *req)
>>> +static inline void __io_req_refcount(struct io_kiocb *req, int nr)
>>>  {
>>>  	if (!(req->flags & REQ_F_REFCOUNT)) {
>>>  		req->flags |= REQ_F_REFCOUNT;
>>> -		atomic_set(&req->refs, 1);
>>> +		atomic_set(&req->refs, nr);
>>>  	}
>>>  }
>>>  
>>> +static inline void io_req_refcount(struct io_kiocb *req)
>>> +{
>>> +	__io_req_refcount(req, 1);
>>> +}
>>> +
>>
>> I really think these should be io_req_set_refcount() or something like
>> that, making it clear that we're actively setting/manipulating the ref
>> count.
> 
> Agree. A separate patch, maybe?

Maybe just fold it into this one, as it's splitting out a helper anyway.
Or do it as a prep patch before this one, up to you.

-- 
Jens Axboe