> -----Original Message----- > From: Jan Beulich <[email protected]> > Sent: 30 September 2020 09:37 > To: [email protected] > Cc: [email protected]; 'Andrew Cooper' > <[email protected]>; 'George Dunlap' > <[email protected]>; 'Ian Jackson' <[email protected]>; 'Julien > Grall' <[email protected]>; > 'Wei Liu' <[email protected]>; 'Stefano Stabellini' <[email protected]> > 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 <[email protected]> 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
