[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v8 3/3] xen/events: do some cleanups in evtchn_fifo_set_pending()
On 27/11/2020 14:48, Jan Beulich wrote: On 27.11.2020 15:39, Jürgen Groß wrote:On 27.11.20 15:23, Julien Grall wrote:On 25/11/2020 10:51, Juergen Gross wrote:--- a/xen/common/event_fifo.c +++ b/xen/common/event_fifo.c @@ -175,6 +175,18 @@ static void evtchn_fifo_set_pending(struct vcpu *v, struct evtchn *evtchn) return; } + /* + * Control block not mapped. The guest must not unmask an + * event until the control block is initialized, so we can + * just drop the event. + */ + if ( unlikely(!v->evtchn_fifo->control_block) )Sort of unrelated, AFAICT, v->evtchn_fifo->control_block can be set concurrently to this access. Thankfully, once the control block is mapped, it can't be unmapped. However, there is still a possibility that you may see half of the update. Shouldn't the field access with ACCESS_ONCE()?Shouldn't this be another patch? Especially as the writing side needs the same treatment.Indeed. As said on several different occasions - our code base is full of places where we chance torn accesses, if there really was a compiler to let us down on this. I am quite amazed you that you managed to test all the version of GCC/Clang that were built and confirm this is unlikely to happen :). This recurring pattern shouldn't lead to unrelated patches getting bloated, unless _all_ affected sites get touched anyway. You probably missed the point where I say "sort of unrelated". This wasn't not a suggestion to fix it here (I should have been clearer though) but instead point out issue as I see them. Cheers, -- Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |