From: Jan Beulich <jbeulich@suse.com>
To: "Julien Grall" <julien@xen.org>,
"Roger Pau Monné" <roger.pau@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v6 1/3] evtchn: slightly defer lock acquire where possible
Date: Tue, 1 Jun 2021 13:54:36 +0200 [thread overview]
Message-ID: <e5e6ab32-815c-49d8-94f3-a75d975465b3@suse.com> (raw)
In-Reply-To: <938eb888-ec15-feb1-19f7-b90dfee822ae@xen.org>
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.
Jan
next prev parent reply other threads:[~2021-06-01 11:54 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 11:27 [PATCH v6 0/3] evtchn: (not so) recent XSAs follow-on Jan Beulich
2021-05-27 11:28 ` [PATCH v6 1/3] evtchn: slightly defer lock acquire where possible Jan Beulich
2021-05-27 13:46 ` Roger Pau Monné
2021-05-27 18:48 ` Julien Grall
2021-05-28 8:30 ` Roger Pau Monné
2021-05-28 10:23 ` Jan Beulich
2021-05-28 10:48 ` Julien Grall
2021-05-28 13:31 ` Roger Pau Monné
2021-05-28 13:41 ` Jan Beulich
2021-05-28 14:26 ` Julien Grall
2021-06-01 11:54 ` Jan Beulich [this message]
2021-06-07 18:15 ` Julien Grall
2021-05-27 11:28 ` [PATCH v6 2/3] evtchn: add helper for port_is_valid() + evtchn_from_port() Jan Beulich
2021-05-27 11:28 ` [PATCH v6 3/3] evtchn: type adjustments Jan Beulich
2021-05-27 13:52 ` Roger Pau Monné
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e5e6ab32-815c-49d8-94f3-a75d975465b3@suse.com \
--to=jbeulich@suse.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=iwj@xenproject.org \
--cc=julien@xen.org \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).