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-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
