RE: [PATCH 11/12] evtchn: convert vIRQ lock to an r/w one

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich <jbeulich@xxxxxxxx>
> Sent: 30 September 2020 09:37
> To: paul@xxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; 'Andrew Cooper' 
> <andrew.cooper3@xxxxxxxxxx>; 'George Dunlap'
> <George.Dunlap@xxxxxxxxxxxxx>; 'Ian Jackson' <iwj@xxxxxxxxxxxxxx>; 'Julien 
> Grall' <julien@xxxxxxx>;
> 'Wei Liu' <wl@xxxxxxx>; 'Stefano Stabellini' <sstabellini@xxxxxxxxxx>
> Subject: Re: [PATCH 11/12] evtchn: convert vIRQ lock to an r/w one
> 
> On 30.09.2020 09:58, Paul Durrant wrote:
> >> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Jan 
> >> Beulich
> >> Sent: 28 September 2020 12:02
> >>
> >> @@ -334,6 +334,12 @@ void _spin_unlock_recursive(spinlock_t *
> >>      }
> >>  }
> >>
> >> +void _rw_barrier(rwlock_t *lock)
> >> +{
> >> +    check_barrier(&lock->lock.debug);
> >> +    do { smp_mb(); } while ( _rw_is_locked(lock) );
> >> +}
> >
> > Should you not have a cpu_relax() somewhere in here?
> >
> > TBH though, the fact this lock is never taken as a writer makes me
> > wonder whether there needs to be a lock at all.
> 
> For both of these - see the discussion Julien and I also had. The
> construct will now move to the subsequent patch anyway, and change
> as per Julien's request.
> 

OK.

Looking again, given that both send_guest_vcpu_virq() and 
send_guest_global_virq() (rightly) hold the evtchn lock before calling 
evtchn_port_set_pending() I think you could do away with the virq lock by 
adding checks in those functions to verify evtchn->state == ECS_VIRQ and u.virq 
== virq after having acquired the channel lock but before calling 
evtchn_port_set_pending().

  Paul

> Jan