From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv3 5/6] evtchn: remove the locking when unmasking an event channel Date: Thu, 18 Jun 2015 13:17:38 +0100 Message-ID: <5582B6E2.8030001@citrix.com> References: <1434542583-28073-1-git-send-email-david.vrabel@citrix.com> <1434542583-28073-6-git-send-email-david.vrabel@citrix.com> <5582C80B0200007800086948@mail.emea.novell.com> <5582AD5A.3000007@citrix.com> <5582D0F902000078000869BE@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z5YlN-0000XV-KM for xen-devel@lists.xenproject.org; Thu, 18 Jun 2015 12:17:45 +0000 In-Reply-To: <5582D0F902000078000869BE@mail.emea.novell.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: Jan Beulich , David Vrabel Cc: xen-devel@lists.xenproject.org, Keir Fraser , Ian Campbell , Tim Deegan List-Id: xen-devel@lists.xenproject.org On 18/06/15 13:08, Jan Beulich wrote: >>>> On 18.06.15 at 13:36, wrote: >> On 18/06/15 12:30, Jan Beulich wrote: >>>>>> On 17.06.15 at 14:03, wrote: >>>> --- a/xen/common/event_channel.c >>>> +++ b/xen/common/event_channel.c >>>> @@ -978,8 +978,6 @@ int evtchn_unmask(unsigned int port) >>>> struct domain *d = current->domain; >>>> struct evtchn *evtchn; >>>> >>>> - ASSERT(spin_is_locked(&d->event_lock)); >>>> - >>>> if ( unlikely(!port_is_valid(d, port)) ) >>>> return -EINVAL; >>>> >>>> @@ -1146,9 +1144,7 @@ long do_event_channel_op(int cmd, >> XEN_GUEST_HANDLE_PARAM(void) arg) >>>> struct evtchn_unmask unmask; >>>> if ( copy_from_guest(&unmask, arg, 1) != 0 ) >>>> return -EFAULT; >>>> - spin_lock(¤t->domain->event_lock); >>>> rc = evtchn_unmask(unmask.port); >>>> - spin_unlock(¤t->domain->event_lock); >>> >>> And, looking particularly at evtchn_fifo_unmask() (and its descendant >>> evtchn_fifo_set_pending()), you get away without acquiring the port >>> lock in or around evtchn_port_unmask()? If indeed so, this one would >>> again be independent on 1, 2, and 4, i.e. could go in together with 3. >> >> Yes. This is only dependent on #3 (simplify port_is_valid()). > > I'm still not convinced that not taking the port lock is correct: It > is my understanding that e.g. the (last_vcpu_id,last_priority) pair > needs to be updated atomically. Yet nothing prevents the > (notify_vcpu_id,priority) pair now from getting updated in the > middle of it being snapshot in evtchn_fifo_set_pending(). Are you > saying this is no problem? This is serialized by the q lock. evtchn_fifo_set_pending() has never been serialized by the event lock because concurrent evtchn_send() calls with two interdomain channels from different domains would have taken different event locks. David