On 2/24/2020 6:50 PM, Pavel Begunkov wrote:
> On 24/02/2020 18:46, Jens Axboe wrote:
>> On 2/24/20 8:44 AM, Pavel Begunkov wrote:
>>>> Fine like this, though easier if you inline the patches so it's easier
>>>> to comment on them.
>>>>
>>>> Agree that the first patch looks fine, though I don't quite see why
>>>> you want to pass in opcode as a separate argument as it's always
>>>> req->opcode. Seeing it separate makes me a bit nervous, thinking that
>>>> someone is reading it again from the sqe, or maybe not passing in
>>>> the right opcode for the given request. So that seems fragile and it
>>>> should go away.
>>>
>>> I suppose it's to hint a compiler, that opcode haven't been changed
>>> inside the first switch. And any compiler I used breaks analysis there
>>> pretty easy. Optimising C is such a pain...
>>
>> But if the choice is between confusion/fragility/performance vs obvious
>> and safe, then I'll go with the latter every time. We should definitely
>> not pass in req and opcode separately.
>
> Yep, and even better to go with the latter, and somehow hint, that it won't
> change. Though, never found a way to do that. Have any tricks in a sleeve?
It seems I have one. It can be done by using a const-attributed getter
function. And I see nothing against it in gcc manuals.
__attribute__((const))
static inline u8 io_get_opcode(struct io_kiocb *req)
{
return req->opcode;
}
--
Pavel Begunkov
