On 8/1/23 16:17, Keith Busch wrote:
On Tue, Aug 01, 2023 at 03:13:59PM +0100, Pavel Begunkov wrote:
On 7/31/23 22:00, Jens Axboe wrote:
On 7/31/23 6:53?AM, Pavel Begunkov wrote:
On 7/28/23 21:14, Keith Busch wrote:
From: Keith Busch <kbusch@xxxxxxxxxx>
Split the req initialization and link handling from the submit. This
simplifies the submit path since everything that can fail is separate
from it, and makes it easier to create batched submissions later.
Keith, I don't think this prep patch does us any good, I'd rather
shove the link assembling code further out of the common path. I like
the first version more (see [1]). I'd suggest to merge it, and do
cleaning up after.
I'll also say that IMHO the overhead is well justified. It's not only
about having multiple nvmes, the problem slows down cases mixing storage
with net and the rest of IO in a single ring.
[1] https://lore.kernel.org/io-uring/20230504162427.1099469-1-kbusch@xxxxxxxx/
The downside of that one, to me, is that it just serializes all of it
and we end up looping over the submission list twice.
Right, and there is nothing can be done if we want to know about all
requests in advance, at least without changing uapi and/or adding
userspace hints.
With alloc+init
split, at least we get some locality wins by grouping the setup side of
the requests.
I don't think I follow, what grouping do you mean? As far as I see, v1
and v2 are essentially same with the difference of whether you have a
helper for setting up links or not, see io_setup_link() from v2. In both
cases it's executed in the same sequence:
1) init (generic init + opcode init + link setup) each request and put
into a temporary list.
2) go go over the list and submit them one by one
And after inlining they should look pretty close.
The main difference in this one compared to the original version is that
everything in the 2nd loop is just for the final dispatch. Anything that
can fail, fallback, or defer to async happens in the first loop. I'm not
sure that makes a difference in runtime, but having the 2nd loop handle
only fast-path requests was what I set out to do for this version.
For performance it doesn't matter, it's a very slow path and we should
not be hitting it. And it only smears single req submission over multiple
places, for instance it won't be legal to use io_submit_sqe() without
those extra checks. Those are all minor points, but I don't think it's
anyhow better than v1 in this aspect.
--
Pavel Begunkov
