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
