diff mbox series

[v2,4/8] evtchn: let evtchn_set_priority() acquire the per-channel lock

Message ID 266c9178-700b-5663-4b5f-69f160a165ab@suse.com (mailing list archive)
State New, archived
Headers show
Series evtchn: recent XSAs follow-on | expand

Commit Message

Jan Beulich Oct. 20, 2020, 2:09 p.m. UTC
Some lock wants to be held to make sure the port doesn't change state,
but there's no point holding the per-domain event lock here. Switch to
using the finer grained per-channel lock instead.

FAOD this doesn't guarantee anything towards in particular
evtchn_fifo_set_pending(), as for interdomain channels that function
would be called with the remote side's per-channel lock held.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Drop acquiring of event lock. Re-write title and description.

Comments

Roger Pau Monné Oct. 22, 2020, 11:17 a.m. UTC | #1
On Tue, Oct 20, 2020 at 04:09:41PM +0200, Jan Beulich wrote:
> Some lock wants to be held to make sure the port doesn't change state,
> but there's no point holding the per-domain event lock here. Switch to
> using the finer grained per-channel lock instead.

While true that's a fine grained lock, it also disables interrupts,
which the global event_lock didn't.

> FAOD this doesn't guarantee anything towards in particular
> evtchn_fifo_set_pending(), as for interdomain channels that function
> would be called with the remote side's per-channel lock held.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
Jan Beulich Oct. 22, 2020, 1:34 p.m. UTC | #2
On 22.10.2020 13:17, Roger Pau Monné wrote:
> On Tue, Oct 20, 2020 at 04:09:41PM +0200, Jan Beulich wrote:
>> Some lock wants to be held to make sure the port doesn't change state,
>> but there's no point holding the per-domain event lock here. Switch to
>> using the finer grained per-channel lock instead.
> 
> While true that's a fine grained lock, it also disables interrupts,
> which the global event_lock didn't.

True, yet we're aiming at dropping this aspect again. Hence I've
added "(albeit as a downside for the time being this requires
disabling interrupts for a short period of time)".

>> FAOD this doesn't guarantee anything towards in particular
>> evtchn_fifo_set_pending(), as for interdomain channels that function
>> would be called with the remote side's per-channel lock held.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

Jan
diff mbox series

Patch

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1154,20 +1154,17 @@  static long evtchn_set_priority(const st
 {
     struct domain *d = current->domain;
     unsigned int port = set_priority->port;
+    struct evtchn *chn;
     long ret;
-
-    spin_lock(&d->event_lock);
+    unsigned long flags;
 
     if ( !port_is_valid(d, port) )
-    {
-        spin_unlock(&d->event_lock);
         return -EINVAL;
-    }
-
-    ret = evtchn_port_set_priority(d, evtchn_from_port(d, port),
-                                   set_priority->priority);
 
-    spin_unlock(&d->event_lock);
+    chn = evtchn_from_port(d, port);
+    spin_lock_irqsave(&chn->lock, flags);
+    ret = evtchn_port_set_priority(d, chn, set_priority->priority);
+    spin_unlock_irqrestore(&chn->lock, flags);
 
     return ret;
 }