On Wed, Jul 9, 2014 at 4:29 PM, Stefan Hajnoczi <stefa...@redhat.com> 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,
