Re: Deduplicate io_*_prep calls?

[Pavel Begunkov <asml.silence@gmail.com>]

On 24/02/2020 10:12, Andres Freund wrote:
> Hi,
> 
> On 2020-02-23 20:52:26 -0700, Jens Axboe wrote:
>> The fast case is not being deferred, that's by far the common (and hot)
>> case, which means io_issue() is called with sqe != NULL. My worry is
>> that by moving it into a prep helper, the compiler isn't smart enough to
>> not make that basically two switches.
> 
> I'm not sure that benefit of a single switch isn't offset by the lower
> code density due to the additional per-opcode branches.  Not inlining
> the prepare function results in:
> 

The first looks good, I like the change. Do you have performance numbers?
e.g. tools/io_uring/io_uring-bench (do_nop=1, with high DEPTH e.g. 100)
would be good enough to estimate relative overhead.
I don't expect any difference, TBH.


> There's still some unnecessary branching on force_nonblocking. The
> second patch just separates the cases needing force_nonblocking
> out. Probably not quite the right structure.
> 

It's trickier there. It can get into io_prep_issue_sqe_nonblock() ->
io_req_prep() with sqe=NULL. With a glance look, it should crash.
The culprit is __io_queue_sqe() with linked requests.

Also, io_issue_sqe_nonblock() would look better than io_prep_issue_sqe_nonblock().

BTW, did you tried to run regression tests? It's under liburing repository.

> 
> Not quite sure what the policy is with attaching POC patches? Also send
> as separate emails?

I'd prefer it inlined (i.e. as text, not attachment), so it can be
inline-commented.

-- 
Pavel Begunkov