[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Jan,

On 30/09/2020 11:16, Jan Beulich wrote:
On 30.09.2020 10:52, Paul Durrant wrote:
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().

I don't think so: The adjustment of v->virq_to_evtchn[] in
evtchn_close() would then happen with just the domain's event
lock held, which the sending paths don't use at all. The per-
channel lock gets acquired in evtchn_close() a bit later only
(and this lock can't possibly protect per-vCPU state).

In fact I'm now getting puzzled by evtchn_bind_virq() updating
this array with (just) the per-domain lock held. Since it's
the last thing in the function, there's technically no strict
need for acquiring the vIRQ lock,

Well, we at least need to prevent the compiler to tear the store/load. If we don't use a lock, then we would should use ACCESS_ONCE() or {read,write}_atomic() for all the usage.

but holding the event lock
definitely doesn't help.

It helps because spin_unlock() and write_unlock() use the same barrier (arch_lock_release_barrier()). So ...

All that looks to be needed is the
barrier implied from write_unlock().

No barrier should be necessary. Although, I would suggest to add a comment explaining it.

Cheers,

--
Julien Grall



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.