Re: [Qemu-devel] [PATCH] linux-aio: fix submit aio as a batch

[Ming Lei]

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,