All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@citrix.com>
To: Jan Beulich <JBeulich@suse.com>, David Vrabel <david.vrabel@citrix.com>
Cc: xen-devel@lists.xenproject.org, Keir Fraser <keir@xen.org>,
	Ian Campbell <ian.campbell@citrix.com>, Tim Deegan <tim@xen.org>
Subject: Re: [PATCHv3 5/6] evtchn: remove the locking when unmasking an event channel
Date: Thu, 18 Jun 2015 13:17:38 +0100	[thread overview]
Message-ID: <5582B6E2.8030001@citrix.com> (raw)
In-Reply-To: <5582D0F902000078000869BE@mail.emea.novell.com>

On 18/06/15 13:08, Jan Beulich wrote:
>>>> On 18.06.15 at 13:36, <david.vrabel@citrix.com> wrote:
>> On 18/06/15 12:30, Jan Beulich wrote:
>>>>>> On 17.06.15 at 14:03, <david.vrabel@citrix.com> 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(&current->domain->event_lock);
>>>>          rc = evtchn_unmask(unmask.port);
>>>> -        spin_unlock(&current->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

  reply	other threads:[~2015-06-18 12:17 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 12:02 [PATCHv3 0/6] evtchn: Improve scalebility David Vrabel
2015-06-17 12:02 ` [PATCHv3 1/6] evtchn: clear xen_consumer when clearing state David Vrabel
2015-06-18 10:30   ` Jan Beulich
2015-06-17 12:02 ` [PATCHv3 2/6] evtchn: defer freeing struct evtchn's until evtchn_destroy_final() David Vrabel
2015-06-18 10:36   ` Jan Beulich
2015-06-18 10:40     ` David Vrabel
2015-06-18 11:01       ` Jan Beulich
2015-06-19  9:29       ` Jan Beulich
2015-06-19  9:52         ` David Vrabel
2015-06-19 10:55           ` Jan Beulich
2015-06-19 12:23             ` David Vrabel
2015-06-19 13:04               ` Jan Beulich
2015-06-19 16:58                 ` David Vrabel
2015-06-17 12:03 ` [PATCHv3 3/6] evtchn: simplify port_is_valid() David Vrabel
2015-06-17 12:03 ` [PATCHv3 4/6] evtchn: use a per-event channel lock for sending events David Vrabel
2015-06-18 11:20   ` Jan Beulich
2015-06-18 11:39     ` David Vrabel
2015-06-17 12:03 ` [PATCHv3 5/6] evtchn: remove the locking when unmasking an event channel David Vrabel
2015-06-18 11:30   ` Jan Beulich
2015-06-18 11:36     ` David Vrabel
2015-06-18 12:08       ` Jan Beulich
2015-06-18 12:17         ` David Vrabel [this message]
2015-06-17 12:03 ` [PATCHv3 6/6] evtchn: pad struct evtchn to 64 bytes David Vrabel
2015-06-18 11:31   ` Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5582B6E2.8030001@citrix.com \
    --to=david.vrabel@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=ian.campbell@citrix.com \
    --cc=keir@xen.org \
    --cc=tim@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.