> -----Original Message-----
> From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of Jan
> Beulich
> Sent: 28 September 2020 11:57
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <andrew.coop...@citrix.com>; George Dunlap
> <george.dun...@eu.citrix.com>; Ian
> Jackson <i...@xenproject.org>; Julien Grall <jul...@xen.org>; Wei Liu
> <w...@xen.org>; Stefano Stabellini
> <sstabell...@kernel.org>
> Subject: [PATCH 02/12] evtchn: avoid race in get_xen_consumer()
>
> There's no global lock around the updating of this global piece of data.
> Make use of cmpxchg() to avoid two entities racing with their updates.
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
> ---
> TBD: Initially I used cmpxchgptr() here, until realizing Arm doesn't
> have it. It's slightly more type-safe than cmpxchg() (requiring
> all arguments to actually be pointers), so I now wonder whether Arm
> should gain it (perhaps simply by moving the x86 implementation to
> xen/lib.h), or whether we should purge it from x86 as being
> pointless.
>
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -57,7 +57,8 @@
> * with a pointer, we stash them dynamically in a small lookup array which
> * can be indexed by a small integer.
> */
> -static xen_event_channel_notification_t xen_consumers[NR_XEN_CONSUMERS];
> +static xen_event_channel_notification_t __read_mostly
> + xen_consumers[NR_XEN_CONSUMERS];
>
> /* Default notification action: wake up from wait_on_xen_event_channel(). */
> static void default_xen_notification_fn(struct vcpu *v, unsigned int port)
> @@ -81,7 +82,7 @@ static uint8_t get_xen_consumer(xen_even
> for ( i = 0; i < ARRAY_SIZE(xen_consumers); i++ )
> {
> if ( xen_consumers[i] == NULL )
> - xen_consumers[i] = fn;
> + (void)cmpxchg(&xen_consumers[i], NULL, fn);
> if ( xen_consumers[i] == fn )
Why not use the return from cmpxchg() to determine success and break out of the
loop rather than re-accessing the global array?
Paul
> break;
> }
>
