On 24/02/2020 20:19, Jens Axboe wrote:
> On 2/24/20 9:53 AM, Andres Freund wrote:
>> Hi,
>>
>> On 2020-02-24 08:40:16 -0700, Jens Axboe wrote:
>>> 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.
>>
>> Without extracting it into an argument the compiler can't know that
>> io_kiocb->opcode doesn't change between the two switches - and therefore
>> is unable to merge the switches.
>>
>> To my knowledge there's no easy and general way to avoid that in C,
>> unfortunately. const pointers etc aren't generally a workaround, even
>> they were applicable here - due to the potential for other pointers
>> existing, the compiler can't assume values don't change.  With
>> sufficient annotations of pointers with restrict, pure, etc. one can get
>> it there sometimes.
>>
>> Another possibility is having a const copy of the struct on the stack,
>> because then the compiler often is able to deduce that the value
>> changing would be undefined behaviour.
>>
>>
>> I'm not sure that means it's worth going for the separate argument - I
>> was doing that mostly to address your concern about the duplicated
>> switch cost.
> 
> Yeah I get that, but I don't think that's worth the pain. An alternative
> solution might be to make the prep an indirect call, and just pair it
> with some variant of INDIRECT_CALL(). This would be trivial, as the
> arguments should be the same, and each call site knows exactly what
> the function should be.

And unfortunately, instead of a nice jump table or offset near-jump, it'll
generate a mess. Though, the attempt itself may prove useful by clarifying
handler/core/etc API.

-- 
Pavel Begunkov