On Mon, Mar 20, 2017 at 6:38 PM, Sergiy Kibrik <
[email protected]> wrote:
>
> + XENCONS_RING_IDX prod = _interface->out_prod;
>
> + constexpr auto ringsize = sizeof(_interface->out) - 1;
>>> + while (true) {
>>> + XENCONS_RING_IDX cons = _interface->out_cons;
>>>
>> + auto delta = prod - cons;
>>> +
>>> + if (unlikely(delta > ringsize)) {
>>> + prod = cons; /* ring is corrupted, reset is the best we can
>>> do */
>>> + delta = 0;
>>> + }
>>> +
>>> + size_t c = 0;
>>> + for (; c < std::min(ringsize - delta, len); c++)
>>> + _interface->out[MASK_XENCONS_IDX(prod++, _interface->out)]
>>> = *str++;
>>> +
>>> + _interface->out_prod = prod;
>>> + wmb();
>>>
>>
> I'm not convinced this wmb() is in the right place (shouldn't it be
> between writing to the array and setting the index). Can you please give
> this another thought? (unfortunately I'm rusty on this topic...).
>
> Barrier must be set before call to notify_remote_via_evtchn(), which
> triggers code execution on the other side of the channel, so that other
> side is guaranteed to have correct content in out_prod when executed.
>
In that case, shouldn't notify_remote_via_evtchn() itself contain the
barrier, to guarantee that any memory written before this call is visible
to the host when it receives the wakeup?
But I'm still worried that we might need an (additional) barrier before
setting out_prod. My thinking is this: What if the host spontaneously
notices the new data before we sent it the wakeup? Usually, this is
possible - one typical example: possibly a *previous* wakeup from a
previous write woke up the host, but the host's handling code still hasn't
run, and before the previous batch of output is handled, we (the guest) is
already writing this second batch.
So now imagine we're writing this batch and the writes get reordered and
the host sees out_prod increased, but does NOT yet see the buffer having
been written. At this point the host will output garbage it finds in the
buffer! So I think that we need to guarantee that the write to out_prod is
never reordered before the write to the buffer - so we need a memory
barrier here too, between the copy loop and the setting of out_prod.
(I might be mistaken, but please think about it).
Nadav.
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
