From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCHv2 3/5] evtchn: use a per-event channel lock for sending events Date: Tue, 16 Jun 2015 17:39:38 +0100 Message-ID: <5580514A.5000702@citrix.com> References: <1434383299-21833-1-git-send-email-david.vrabel@citrix.com> <1434383299-21833-4-git-send-email-david.vrabel@citrix.com> <5580060502000078000855A9@mail.emea.novell.com> <55803E73.2010603@citrix.com> <558047AA.3040600@citrix.com> <558068BE0200007800085B14@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 1Z4tto-0004RR-Cm for xen-devel@lists.xenproject.org; Tue, 16 Jun 2015 16:39:44 +0000 In-Reply-To: <558068BE0200007800085B14@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 16/06/15 17:19, Jan Beulich wrote: >>>> On 16.06.15 at 17:58, wrote: >> On 16/06/15 16:19, David Vrabel wrote: >>>>> @@ -1221,6 +1277,8 @@ void notify_via_xen_event_channel(struct domain *ld, >> int lport) >>>>> evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn); >>>>> } >>>>> >>>>> + spin_unlock(&lchn->lock); >>>>> + >>>>> spin_unlock(&ld->event_lock); >>>>> } >>>> >>>> Again I think the event lock can be dropped earlier now. >>> >>> Ditto. >> >> Uh, no. This is notify. I've kept the locking like this because of the >> ld->is_dying check. I think we need the ld->event_lock in case >> d->is_dying is set and evtchn_destroy(ld) is called. > > Right, but if evtchn_destroy() was a concern, then this wouldn't > apply just here, but also in the sending path you are relaxing. > Afaict due to the channel lock being taken in __evtchn_close() > you can drop the even lock here as the latest after you acquired > the channel one (I haven't been able to convince myself yet that > dropping it even before that would be okay). But in the evtchn_send() case, we're in a hypercall so we know ld->is_dying is false and thus cannot be racing with evtchn_destroy(ld). It would be good to remove event_lock from notify_xen_event_channel() as well since this is heavily used for ioreqs and vm events. Let me have a more careful look. David