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

[Julien Grall <julien@xxxxxxx>]

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.
Cheers,

--
Julien Grall