From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv2 2/5] evtchn: simplify port_is_valid() Date: Mon, 15 Jun 2015 16:59:00 +0100 Message-ID: <557EF644.6020108@citrix.com> References: <1434383299-21833-1-git-send-email-david.vrabel@citrix.com> <1434383299-21833-3-git-send-email-david.vrabel@citrix.com> <1434383866.13744.28.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z4XdB-000278-K5 for xen-devel@lists.xenproject.org; Mon, 15 Jun 2015 16:53:05 +0000 In-Reply-To: <1434383866.13744.28.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: xen-devel@lists.xenproject.org, Keir Fraser , Tim Deegan , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 15/06/15 16:57, Ian Campbell wrote: > On Mon, 2015-06-15 at 16:48 +0100, David Vrabel wrote: >> By keeping a count of the number of currently valid event channels, >> port_is_valid() can be simplified. >> >> d->valid_evtchns can also be tested without holding d->event_lock which >> will be useful later on. >> >> Signed-off-by: David Vrabel >> --- >> v2: >> - Used unsigned int for d->valid_evtchns. >> --- >> xen/common/event_channel.c | 3 +++ >> xen/include/xen/event.h | 4 +--- >> xen/include/xen/sched.h | 5 +++-- >> 3 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c >> index 947880f..fd48646 100644 >> --- a/xen/common/event_channel.c >> +++ b/xen/common/event_channel.c >> @@ -189,6 +189,8 @@ static int get_free_port(struct domain *d) >> return -ENOMEM; >> bucket_from_port(d, port) = chn; >> >> + write_atomic(&d->valid_evtchns, d->valid_evtchns + EVTCHNS_PER_BUCKET); > > Shouldn't this be atomic_add? Otherwise the result of what you have here > is actually a non-atomic read/add/write. This field is only updated while holding d->event_lock. Only reads are unlocked. David