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

Re: [PATCH v6 1/3] evtchn: slightly defer lock acquire where possible

Hi Jan,

On 01/06/2021 12:54, Jan Beulich wrote:
On 27.05.2021 20:48, Julien Grall wrote:
On 27/05/2021 12:28, Jan Beulich wrote:
port_is_valid() and evtchn_from_port() are fine to use without holding
any locks. Accordingly acquire the per-domain lock slightly later in
evtchn_close() and evtchn_bind_vcpu().

So I agree that port_is_valid() and evtchn_from_port() are fine to use
without holding any locks in evtchn_bind_vcpu(). However, this is
misleading to say there is no problem with evtchn_close().

evtchn_close() can be called with current != d and therefore, there is a
risk that port_is_valid() may be valid and then invalid because
d->valid_evtchns is decremented in evtchn_destroy().

While this is the case for other functions as well (and hence a
comment along the lines of what you ask for below should have
been in place already), I've added

  * While calling the function is okay without holding a suitable lock yet
  * (see the comment ahead of struct evtchn_port_ops for which ones those
  * are), for a dying domain it may start returning false at any point - see
  * evtchn_destroy(). This is not a fundamental problem though, as the
  * struct evtchn instance won't disappear (and will continue to hold valid
  * data) until final cleanup of the domain, at which point the domain itself
  * cannot be looked up anymore and hence calls here can't occur anymore in
  * the first place.


Thankfully the memory is still there. So the current code is okayish and
I could reluctantly accept this behavior to be spread. However, I don't
think this should be left uncommented in both the code (maybe on top of
port_is_valid()?) and the commit message.

... ahead of port_is_valid() (and not, as I did intend originally,
in evtchn_close()). As far as the commit message goes, I'll have it
refer to the comment only.

I hope this satisfies the requests of both of you. I'll take the
liberty and retain your ack, Roger.

Yes, this satistfies my requests. Feel free to add my reviewed-by on the patch.


Julien Grall



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