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

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 30/09/2020 07:26, Jan Beulich wrote:
> On 29.09.2020 19:18, Julien Grall wrote:
> > On 29/09/2020 14:37, Jan Beulich wrote:
> > > On 29.09.2020 15:03, Julien Grall wrote:
> > > > I am thinking that it may be easier to hold the write lock when doing
> > > > the update.
> > > ... perhaps this is indeed better. I have to admit that I never
> > > fully understood the benefit of using spin_barrier() in this code
> > > (as opposed to the use in evtchn_destroy()).
> > I am not entirely sure either. It looks like it is an attempt to make
> > v->virq_to_evtchn[X] visible without holding a lock.
> >
> > Any holder of the lock after spin_barrier() has completed will read 0
> > and not try to use the lock.
> I'm not sure I follow: A holder of the lock is obviously already
> making use of the lock.
My point is the barrier is meant to split the holders of the lock in two 
category:
   - Anyone acquiring the lock before the spin_barrier() completed may 
see either the port open or close.
   - Anyone acquiring the lock after the spin_barrier() completed will 
see a close port.
> Or are you talking of two different locks
> here (recall that before XSA-343 there was just one lock involved
> in sending)?
>
> > But the update of v->virq_to_evtchn[X] should have used either
> > ACCESS_ONCE() or write_atomic().
> Of course, like in so many other places in the code base.
This is known. What I meant is if we are going to continue to use a 
spin_barrier() (or rw_barrier()), then we should also switch to use 
ACCESS_ONCE()/write_atomic().
Anyway, I think we discussed to acquire the write lock instead. So it 
should not be a concern.
Cheers,

--
Julien Grall