On Mon, Feb 20, 2017 at 03:59:42PM +0100, Peter Lieven wrote:
> Am 20.02.2017 um 15:50 schrieb Stefan Hajnoczi:
> > On Fri, Feb 17, 2017 at 05:00:24PM +0100, Peter Lieven wrote:
> > > +        if (s->wr_in_order) {
> > > +            /* reenter the coroutine that might have waited
> > > +             * for this write to complete */
> > > +            s->wr_offs = sector_num + n;
> > > +            for (i = 0; i < s->num_coroutines; i++) {
> > > +                if (s->co[i] && s->wait_sector_num[i] == s->wr_offs) {
> > > +                    qemu_coroutine_enter(s->co[i]);
> > > +                    break;
> > This qemu_coroutine_enter() call relies on the yield pattern between
> > sibling coroutines having no recursive qemu_coroutine_enter() calls.
> > QEMU aborts if there is a code path where coroutine A enters B and then
> > B enters A again before yielding.
> > 
> > Paolo's new aio_co_wake() API solves this issue by deferring the
> > qemu_coroutine_enter() to the event loop.  It's similar to CoQueue
> > wakeup.  aio_co_wake() is part of my latest block pull request (should
> > be merged into qemu.git soon).
> > 
> > I *think* this patch has no A -> B -> A situation thanks to yields in
> > the code path, but it would be nicer to use aio_co_wake() where this can
> > never happen.
> 
> I also believe this can't happen. Would it be also okay to use
> qemu_coroutine_enter_if_inactive() here?

Perhaps this comment is enough:

  /*
   * A -> B -> A cannot occur because A has s->wait_sector_num[i] == -1
   * during A -> B.  Therefore B will never enter A during this time
   * window.
   */
  qemu_coroutine_enter(s->co[i]);

Normally a comment isn't necessary because qemu_coroutine_enter() is
used for parent-child relationships.  But here it's a sibling
relationship so we need proof that A -> B -> A doesn't occur.