Hi Jan,
On 03/12/2020 09:46, Jan Beulich wrote:
On 02.12.2020 20:03, Julien Grall wrote:
On 23/11/2020 13:28, Jan Beulich wrote:
The per-vCPU virq_lock, which is being held anyway, together with there
not being any call to evtchn_port_set_pending() when v->virq_to_evtchn[]
is zero, provide sufficient guarantees.
I agree that the per-vCPU virq_lock is going to be sufficient, however
dropping the lock also means the event channel locking is more complex
to understand (the long comment that was added proves it).
In fact, the locking in the event channel code was already proven to be
quite fragile, therefore I think this patch is not worth the risk.
I agree this is a very reasonable position to take. I probably
would even have remained silent if in the meantime the
spin_lock()s there hadn't changed to read_trylock()s. I really
think we want to limit this unusual locking model to where we
strictly need it.
While I appreciate that the current locking is unusual, we should follow
the same model everywhere rather than having a dozen of way to lock the
same structure.
The rationale is quite simple, if you have one way to lock a structure,
then there are less chance to screw up.
The only reason I would be willing to diverge from statement is if the
performance are significantly improved.
Cheers,
--
Julien Grall
