Re: [Xen-devel] [PATCH 6/7] mini-os: Added rmb to xenbus code

[Thomas Leonard <talex5@xxxxxxxxx>]

On 6 June 2014 17:40, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Fri, 2014-06-06 at 17:10 +0200, Samuel Thibault wrote:
>> Ian Campbell, le Fri 06 Jun 2014 15:15:42 +0100, a Ãcrit :
>> > On Fri, 2014-06-06 at 05:59 +0100, karim.allah.ahmed@xxxxxxxxx wrote:
>> > > In any case, by looking at the patch it seems like rmb was required to
>> > > make sure that the reply message is visible to the current processor
>> > > when this thread wakes up and starts reading it.
>> >
>> > Can we assume (or arrange) that schedule() gives us this guarantee? Do
>> > we want to?
>>
>> I'd tend to think something like that, yes. More precisely, thread
>> wait/wake primitives usually provide a barrier: events before the wake
>> are supposed to complete before the wait returns. So both should have a
>> memory barrier.
>
> Makes sense to me.

I'm not sure I understand this. Looking again at the code:

if (xenstore_buf->rsp_prod - xenstore_buf->rsp_cons < sizeof(msg))
    break;
rmb();
memcpy_from_ring(xenstore_buf->rsp,
    &msg,
    MASK_XENSTORE_IDX(xenstore_buf->rsp_cons),
    sizeof(msg));

I think we are:

1) Checking that the producer response index shows at least one full
unread response is available.

2) Reading the response.

Without the rmb(), we might use stale cached data from before the
response was written, right?

If so, I don't think this has anything to do with Mini-OS's scheduling
(with is always on the same CPU anyway), but with messages between
domains.


-- 
Dr Thomas Leonard        http://0install.net/
GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel