Messages in this thread | | | From | Jeff Moyer <> | Subject | Re: [PATCH 10/10] fs: make aio plug | Date | Mon, 24 Jan 2011 14:29:33 -0500 |
| |
Jens Axboe <jaxboe@fusionio.com> writes:
> On 2011-01-24 20:15, Jeff Moyer wrote: >> Jens Axboe <jaxboe@fusionio.com> writes: >> >>>>> @@ -1698,6 +1701,7 @@ long do_io_submit(aio_context_t ctx_id, long nr, >>>>> if (ret) >>>>> break; >>>>> } >>>>> + blk_finish_plug(&plug); >>>>> aio_batch_free(batch_hash); >>>> >>>> I'm pretty sure you want blk_finish_plug to run after aio_batch_free. >>> >>> You mean to cover the iput()? That's not a bad idea, if that ends up >>> writing it out. I did a few read tests here and confirmed it's sending >>> down batches of whatever number is submitted. >> >> No, I actually didn't make it all the way through 5/10, so I didn't >> realize you got rid of the blk_run_address_space. I agree with the TODO >> item to get rid of the aio batching, since it's now taken care of with >> the on-stack plugging. > > Oh, so that was the whole point of the series :-)
I've said dumber things, I'm sure. =)
>> As for the iput, I don't think you will get the final iput here, since >> you've just scheduled I/O against the file. > > Good point, so the original comment is moot - it wont make a difference. > Still, may not be a bad idea to do the freeing first. I was sort-of > hoping to be able to kill the batching in fs/aio.c completely, though...
Yes, that's what I meant above when I said I agreed with the TODO item. Go ahead and nuke it.
Cheers, Jeff
| |