Re: [PATCH 00/26] AIO performance improvements/cleanups, v2

[Kent Overstreet]

On Sat, Dec 15, 2012 at 10:46:32AM +0100, Jens Axboe wrote:
> On 2012-12-15 10:25, Kent Overstreet wrote:
> > Cool, thanks for the numbers!
> > 
> > I suspect the difference is due to contention on the ringbuffer,
> > completion side. You didn't enable my batched completion stuff, did you?
> 
> No, haven't tried the batching yet.
> 
> > I suspect the numbers would look quite a bit different with that,
> > based on my own profiling. If the driver for the device you're testing
> > on is open source, I'd be happy to do the conversion (it's a 5 minute
> > job).
> 
> Knock yourself out - I already took a quick look at it, and conversion
> should be pretty simple. It's the mtip32xx driver, it's in the kernel. I
> would suggest getting rid of the ->async_callback() (since it's always
> bio_endio()) since that'll make it cleaner.

Just pushed my conversion - it's untested, but it's pretty
straightforward.

> 
> > Also, I don't think our approaches really conflict - it's been awhile
> 
> Completely agree. I split my patches up a bit yesterday, and then I took
> a look at your series. There's a bit of overlap between the two, but
> really most of it would be useful together. You can see the (bit more)
> split series here:
> 
> http://git.kernel.dk/?p=linux-block.git;a=shortlog;h=refs/heads/aio-dio

Cool, taking a look at it now.

> 
> > since I looked at your patch but you're getting rid of the aio
> > ringbuffer and using a linked list instead, right? My batched completion
> > stuff should still benefit that case.
> 
> Yes, I make the ring interface optional. Basically you tell aio to use
> the ring or not at io_queue_init() time. If you don't care about the
> ring, we can use a lockless list for the completions.

Yeah, it is a good idea - I'm certainly not attached to the current
ringbuffer implementation (though a ringbuffer isn't a terrible idea if
we had one that was implemented correctly).

> You completely remove the cancel, I just make it optional for the gadget
> case. I'm fine with either of them, though I did not look at your usb
> change in detail. If it's clean, I suspect we should just kill cancel
> completion as you did.

We (Zach and I) actually made it optional too, more or less - I haven't
looked at how you did it yet, but in my tree the linked list is there,
but the kiocb isn't added to the kioctx's list until something sets a
cancel function.

> > Though - hrm, I'd have expected getting rid of the cancellation linked
> > list to make a bigger difference and both our patchsets do that.
> 
> The machine in question runs out of oomph, which is hampering the
> results. I should have it beefed up next week. It's running E5-2630
> right now, will move to E5-2690. I think that should make the results
> clearer.

Well, the fact that it's cpu bound just means the throughput numbers for
our various patches are different... and what I'm really interested in
is the profiles, I can't think of any reason cpu speed would affect that
much.

> > What device are you testing on, and what's your fio script? I may just
> > have to buy some hardware so I can test this myself.
> 
> Pretty basic script, it's attached. Probably could eek more out of the
> system, but it's been fine for just basic apples-to-apples comparison.
> I'm using 6x p320h for this test case.

Thanks - I'll have to see if I can get something setup that roughly
matches your setup. 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/