From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
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>
Subject: Re: [PATCH 11/12] evtchn: convert vIRQ lock to an r/w one
Date: Wed, 30 Sep 2020 10:09:20 +0100 [thread overview]
Message-ID: <c3d565e6-0094-557b-6828-8280de68b590@xen.org> (raw)
In-Reply-To: <03d30b08-c0ce-0d24-444d-ec20f3528039@suse.com>
Hi Jan,
On 30/09/2020 07:26, Jan Beulich wrote:
> On 29.09.2020 19:18, Julien Grall wrote:
>> On 29/09/2020 14:37, Jan Beulich wrote:
>>> On 29.09.2020 15:03, Julien Grall wrote:
>>>> I am thinking that it may be easier to hold the write lock when doing
>>>> the update.
>>>
>>> ... perhaps this is indeed better. I have to admit that I never
>>> fully understood the benefit of using spin_barrier() in this code
>>> (as opposed to the use in evtchn_destroy()).
>>
>> I am not entirely sure either. It looks like it is an attempt to make
>> v->virq_to_evtchn[X] visible without holding a lock.
>>
>> Any holder of the lock after spin_barrier() has completed will read 0
>> and not try to use the lock.
>
> I'm not sure I follow: A holder of the lock is obviously already
> making use of the lock.
My point is the barrier is meant to split the holders of the lock in two
category:
- Anyone acquiring the lock before the spin_barrier() completed may
see either the port open or close.
- Anyone acquiring the lock after the spin_barrier() completed will
see a close port.
> Or are you talking of two different locks
> here (recall that before XSA-343 there was just one lock involved
> in sending)?
>
>> But the update of v->virq_to_evtchn[X] should have used either
>> ACCESS_ONCE() or write_atomic().
>
> Of course, like in so many other places in the code base.
This is known. What I meant is if we are going to continue to use a
spin_barrier() (or rw_barrier()), then we should also switch to use
ACCESS_ONCE()/write_atomic().
Anyway, I think we discussed to acquire the write lock instead. So it
should not be a concern.
Cheers,
--
Julien Grall
next prev parent reply other threads:[~2020-09-30 9:10 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-28 10:54 [PATCH 00/12] evtchn: recent XSAs follow-on Jan Beulich
2020-09-28 10:56 ` [PATCH 01/12] evtchn: refuse EVTCHNOP_status for Xen-bound event channels Jan Beulich
2020-09-29 8:49 ` Julien Grall
2020-09-29 15:39 ` Paul Durrant
2020-09-28 10:56 ` [PATCH 02/12] evtchn: avoid race in get_xen_consumer() Jan Beulich
2020-09-29 9:35 ` Julien Grall
2020-09-29 15:44 ` Paul Durrant
2020-09-29 15:58 ` Jan Beulich
2020-09-28 10:57 ` [PATCH 03/12] evtchn: don't call Xen consumer callback with per-channel lock held Jan Beulich
2020-09-29 10:16 ` Julien Grall
2020-09-29 10:54 ` Jan Beulich
2020-09-28 10:57 ` [PATCH 04/12] evtchn: evtchn_set_priority() needs to acquire the per-channel lock Jan Beulich
2020-09-29 10:21 ` Julien Grall
2020-09-29 11:49 ` Jan Beulich
2020-09-29 16:31 ` Paul Durrant
2020-09-30 6:29 ` Jan Beulich
2020-09-30 6:41 ` Jan Beulich
2020-09-30 7:31 ` Paul Durrant
2020-09-30 8:31 ` Jan Beulich
2020-09-30 8:36 ` Paul Durrant
2020-09-30 8:41 ` Jan Beulich
2020-09-28 10:58 ` [PATCH 05/12] evtchn/sched: reject poll requests for unusable ports Jan Beulich
2020-09-29 12:17 ` Julien Grall
2020-09-29 13:00 ` Dario Faggioli
2020-09-29 16:51 ` Paul Durrant
2020-09-28 10:59 ` [PATCH 06/12] evtchn: don't bypass unlinking pIRQ when closing port Jan Beulich
2020-09-29 17:07 ` Paul Durrant
2020-09-28 11:00 ` [PATCH 07/12] evtchn: cut short evtchn_reset()'s loop in the common case Jan Beulich
2020-09-29 17:16 ` Paul Durrant
2020-10-01 15:54 ` Julien Grall
2020-09-28 11:00 ` [PATCH 08/12] evtchn: ECS_CLOSED => ECS_FREE Jan Beulich
2020-09-29 12:19 ` Julien Grall
2020-09-29 16:56 ` Paul Durrant
2020-09-28 11:00 ` [PATCH 09/12] evtchn: move FIFO-private struct declarations Jan Beulich
2020-09-29 12:26 ` Julien Grall
2020-09-29 12:49 ` Jan Beulich
2020-09-30 8:43 ` Julien Grall
2020-09-30 7:37 ` Paul Durrant
2020-09-30 8:32 ` Jan Beulich
2020-09-28 11:01 ` [PATCH 10/12] evtchn/fifo: use stable fields when recording "last queue" information Jan Beulich
2020-09-29 12:29 ` Julien Grall
2020-09-30 7:35 ` Paul Durrant
2020-09-30 8:35 ` Jan Beulich
2020-09-30 8:38 ` Paul Durrant
2020-09-28 11:02 ` [PATCH 11/12] evtchn: convert vIRQ lock to an r/w one Jan Beulich
2020-09-29 13:03 ` Julien Grall
2020-09-29 13:37 ` Jan Beulich
2020-09-29 17:18 ` Julien Grall
2020-09-30 6:26 ` Jan Beulich
2020-09-30 9:09 ` Julien Grall [this message]
2020-09-30 7:58 ` Paul Durrant
2020-09-30 8:37 ` Jan Beulich
2020-09-30 8:52 ` Paul Durrant
2020-09-30 10:16 ` Jan Beulich
2020-10-01 16:21 ` Julien Grall
2020-10-02 6:12 ` Jan Beulich
2020-10-02 8:43 ` Julien Grall
2020-09-28 11:02 ` [PATCH 12/12] evtchn: convert domain event " Jan Beulich
2020-09-28 16:44 ` Roger Pau Monné
2020-09-29 14:32 ` Jan Beulich
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=c3d565e6-0094-557b-6828-8280de68b590@xen.org \
--to=julien@xen.org \
--cc=George.Dunlap@eu.citrix.com \
--cc=andrew.cooper3@citrix.com \
--cc=iwj@xenproject.org \
--cc=jbeulich@suse.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.