Re: [PATCH v4 04/10] evtchn: don't call Xen consumer callback with per-channel lock held

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 05/01/2021 13:10, Jan Beulich wrote:
> While there don't look to be any problems with this right now, the lock
> order implications from holding the lock can be very difficult to follow
> (and may be easy to violate unknowingly). The present callbacks don't
> (and no such callback should) have any need for the lock to be held.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

> ---
> v4: Go back to v2.
> v3: Drain callbacks before proceeding with closing. Re-base.
>
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -767,9 +767,18 @@ int evtchn_send(struct domain *ld, unsig
>           rport = lchn->u.interdomain.remote_port;
>           rchn  = evtchn_from_port(rd, rport);
>           if ( consumer_is_xen(rchn) )
> -            xen_notification_fn(rchn)(rd->vcpu[rchn->notify_vcpu_id], rport);
> -        else
> -            evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn);
> +        {
> +            /* Don't keep holding the lock for the call below. */
> +            xen_event_channel_notification_t fn = xen_notification_fn(rchn);
> +            struct vcpu *rv = rd->vcpu[rchn->notify_vcpu_id];
> +
> +            rcu_lock_domain(rd);
> +            evtchn_read_unlock(lchn);
> +            fn(rv, rport);
> +            rcu_unlock_domain(rd);
> +            return 0;
> +        }
> +        evtchn_port_set_pending(rd, rchn->notify_vcpu_id, rchn);
>           break;
>       case ECS_IPI:
>           evtchn_port_set_pending(ld, lchn->notify_vcpu_id, lchn);
Cheers,

--
Julien Grall