Re: [Qemu-devel] [PATCH v2 for-2.11 2/4] coroutine: abort if we try to schedule or enter a pending coroutine

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Jeff Cody <jcody@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, mreitz@redhat.com,
	famz@redhat.com, pbonzini@redhat.com, kwolf@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 for-2.11 2/4] coroutine: abort if we try to schedule or enter a pending coroutine
Date: Tue, 21 Nov 2017 10:59:35 +0000	[thread overview]
Message-ID: <20171121105935.GB25265@stefanha-x1.localdomain> (raw)
In-Reply-To: <1f0fd95c2096688add2c7b3cfcd7016756ef19fb.1511230683.git.jcody@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On Mon, Nov 20, 2017 at 09:23:24PM -0500, Jeff Cody wrote:
> @@ -438,6 +439,16 @@ fail:
>  void aio_co_schedule(AioContext *ctx, Coroutine *co)
>  {
>      trace_aio_co_schedule(ctx, co);
> +    const char *scheduled = atomic_read(&co->scheduled);
> +
> +    if (scheduled) {
> +        fprintf(stderr,
> +                "%s: Co-routine was already scheduled in '%s'\n",
> +                __func__, scheduled);
> +        abort();
> +    }
> +    atomic_set(&co->scheduled, __func__);

According to docs/devel/atomics.txt atomic_set()/atomic_read() are
weakly ordered.  They require memory barriers to provide guarantees
about ordering.  Your patch doesn't include barriers or comments about
where the implicit barriers are.

The docs recommend using the following instead of
atomic_read()/atomic_set() to get ordering:

  typeof(*ptr) atomic_mb_read(ptr)
  void         atomic_mb_set(ptr, val)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]