[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] linux-aio: fix submit aio as a batch
From: |
Ming Lei |
Subject: |
Re: [Qemu-devel] [PATCH] linux-aio: fix submit aio as a batch |
Date: |
Fri, 11 Jul 2014 23:43:31 +0800 |
On Wed, Jul 9, 2014 at 4:29 PM, Stefan Hajnoczi <address@hidden> wrote:
> On Tue, Jul 08, 2014 at 11:45:10PM +0800, Ming Lei wrote:
>> In the enqueue path, we can't complete request, otherwise
>> "Co-routine re-entered recursively" may be caused, so this
>> patch fixes the issue with below ideas:
>
> Thi probably happens when the caller is in coroutine context and its
> completion function invokes qemu_coroutine_enter() on itself. The
> solution is to invoke completions from a BH (other places in the block
> layer do this too).
Yes, BH is solution, but for this case, I prefer to take the approach in
the patch because the completion shouldn't have been called if submit
is failed, which should be kept consistent as before.
>
>> - for -EAGAIN, retry the submission in an introduced event handler
>
> I agree with Paolo that a BH is appropriate.
It isn't a big deal about BH vs. event, and I take event just for
not making resubmission too quick.
>
>> - for part of completion, just update the io queue, since it is
>> moving on after all
>
> If we do this then we need to guarantee that io_submit() will be called
> at some point soon. Otherwise requests could get stuck if the guest
> doesn't submit any more I/O requests to push the queue.
Good point, I will let the BH or event handler to resubmit the remainder.
>
> Please split this into separate patches. You're trying to do too much.
>
> Overall, I would prefer it if we avoid the extra complexity of deferring
> io_submit() on EAGAIN and partial submission. Do you understand why the
It might be a bit difficult to avoid the problem completely with a fixed/static
max events, especially after multi virtqueue of virtio-blk is introduced.
> kernel is producing this behavior? Can we set the right capacity in
It is caused by not enough request resources.
> io_setup() so it doesn't happen?
Yes, it can be helpful, but won't easy to avoid -EAGAIN completely.
>
>> + if (enqueue)
>> + return ret;
>
> Please set up a git hook to run checkpatch.pl. It will alert you when
> you violate QEMU coding style:
> http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html
>
> I already mentioned coding style in previous patches, using a git hook
> will avoid it happening again.
Sorry for missing that again.
Thanks,