> -----Original Message----- > From: Xen-devel <[email protected]> On Behalf Of Jan > Beulich > Sent: 28 September 2020 11:57 > To: [email protected] > Cc: Andrew Cooper <[email protected]>; George Dunlap > <[email protected]>; Ian > Jackson <[email protected]>; Julien Grall <[email protected]>; Wei Liu > <[email protected]>; Stefano Stabellini > <[email protected]> > 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 <[email protected]> > --- > 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; > } >
