[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 02/10] evtchn: bind-interdomain doesn't need to hold both domains' event locks
Hi Jan, On 05/01/2021 13:09, Jan Beulich wrote: The local domain's lock is needed for the port allocation, but for the remote side the per-channel lock is sufficient. The per-channel locks then need to be acquired slightly earlier, though. I was expecting is little bit more information in the commit message because there are a few changes in behavior with this change: 1) AFAICT, evtchn_allocate_port() rely on rchn->state to be protected by the rd->event_lock. Now that you dropped the rd->event_lock, rchn->state may be accessed while it is updated in evtchn_bind_interdomain(). The port cannot go back to ECS_FREE here, but I think the access needs to be switched to {read, write}_atomic() or ACCESS_ONCE. 2) xsm_evtchn_interdomain() is now going to be called without the rd->event_lock. Can you confirm that the lock is not needed by XSM? Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> --- v4: New. --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -355,18 +355,7 @@ static long evtchn_bind_interdomain(evtc if ( (rd = rcu_lock_domain_by_id(rdom)) == NULL ) return -ESRCH;- /* Avoid deadlock by first acquiring lock of domain with smaller id. */- if ( ld < rd ) - { - spin_lock(&ld->event_lock); - spin_lock(&rd->event_lock); - } - else - { - if ( ld != rd ) - spin_lock(&rd->event_lock); - spin_lock(&ld->event_lock); - } + spin_lock(&ld->event_lock);if ( (lport = get_free_port(ld)) < 0 )ERROR_EXIT(lport); @@ -375,15 +364,19 @@ static long evtchn_bind_interdomain(evtc if ( !port_is_valid(rd, rport) ) ERROR_EXIT_DOM(-EINVAL, rd); rchn = evtchn_from_port(rd, rport); + + double_evtchn_lock(lchn, rchn); + if ( (rchn->state != ECS_UNBOUND) || (rchn->u.unbound.remote_domid != ld->domain_id) ) You want to unlock the event channels here. ERROR_EXIT_DOM(-EINVAL, rd);rc = xsm_evtchn_interdomain(XSM_HOOK, ld, lchn, rd, rchn);if ( rc ) + { + double_evtchn_unlock(lchn, rchn); goto out; - - double_evtchn_lock(lchn, rchn); + }lchn->u.interdomain.remote_dom = rd;lchn->u.interdomain.remote_port = rport; @@ -407,8 +400,6 @@ static long evtchn_bind_interdomain(evtc out: check_free_port(ld, lport); spin_unlock(&ld->event_lock); - if ( ld != rd ) - spin_unlock(&rd->event_lock);rcu_unlock_domain(rd); Cheers, -- Julien Grall
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |