On 8/15/21 3:41 AM, Pavel Begunkov wrote:
> On 8/14/21 8:38 PM, Jens Axboe wrote:
>> On 8/14/21 1:36 PM, Pavel Begunkov wrote:
>>> On 8/14/21 8: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
>>>>
>>>>>> @@ -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?
>>>
>>> I mean it just would be a bit easier for me, instead of rebasing
>>> this series and not yet sent patches.
>>
>> I think it should come before this series at least, or be folded into the
>> first patch. So probably no way around the rebase, sorry...
>
> Don't see the point, but anyway, just resent it
That's the usual approach, first a prep patch to clean it up, then change
on top. The opposite might be easier since the other patches already exist,
but it's backwards in terms of ordering imho.
--
Jens Axboe
