barriers before {req/rsp}_cons = cons?

barriers before {req/rsp}_cons = cons?

[Samuel Thibault]

Hello,

In a bunch of places, one can read code like

cons = netif->tx.req_cons;
rmb(); /* Ensure that we see the request before we copy it. */
memcpy(&txreq, RING_GET_REQUEST(&netif->tx, i), sizeof(txreq));
some checks
netif->tx.req_cons = ++cons;

Shouldn't there be a full barrier just before the req_cons assignation?
I guess we are currently not seeing bugs at least because the req will
not be overwriten until we loop in the ring, but it seems to me there
may be a bug here.

Samuel

Re: barriers before {req/rsp}_cons = cons?

[Keir Fraser]

Could you describe the race you believe is made possible by the absence of
the barrier?

 -- Keir

On 18/7/08 19:52, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:

> Hello,
> 
> In a bunch of places, one can read code like
> 
> cons = netif->tx.req_cons;
> rmb(); /* Ensure that we see the request before we copy it. */
> memcpy(&txreq, RING_GET_REQUEST(&netif->tx, i), sizeof(txreq));
> some checks
> netif->tx.req_cons = ++cons;
> 
> Shouldn't there be a full barrier just before the req_cons assignation?
> I guess we are currently not seeing bugs at least because the req will
> not be overwriten until we loop in the ring, but it seems to me there
> may be a bug here.
> 
> Samuel
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: barriers before {req/rsp}_cons = cons?

[Samuel Thibault]

Keir Fraser, le Fri 18 Jul 2008 20:31:13 +0100, a écrit :
> > cons = netif->tx.req_cons;
> > rmb(); /* Ensure that we see the request before we copy it. */
> > memcpy(&txreq, RING_GET_REQUEST(&netif->tx, i), sizeof(txreq));
> > some checks
> > netif->tx.req_cons = ++cons;
> 
> Could you describe the race you believe is made possible by the absence of
> the barrier?

It is very unlikely, but the write into req_cons could happen before
actually doing the memcpy, and so another processor could try to reuse
the same request slot.

Samuel

Re: barriers before {req/rsp}_cons = cons?

[Samuel Thibault]

Samuel Thibault, le Fri 18 Jul 2008 20:33:04 +0100, a écrit :
> Keir Fraser, le Fri 18 Jul 2008 20:31:13 +0100, a écrit :
> > > cons = netif->tx.req_cons;
> > > rmb(); /* Ensure that we see the request before we copy it. */
> > > memcpy(&txreq, RING_GET_REQUEST(&netif->tx, i), sizeof(txreq));
> > > some checks
> > > netif->tx.req_cons = ++cons;
> > 
> > Could you describe the race you believe is made possible by the absence of
> > the barrier?
> 
> It is very unlikely, but the write into req_cons could happen before
> actually doing the memcpy,

(more precisely, the read, of course).

> and so another processor could try to reuse
> the same request slot.

Samuel

Re: barriers before {req/rsp}_cons = cons?

[Keir Fraser]

On 18/7/08 20:33, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:

>> Could you describe the race you believe is made possible by the absence of
>> the barrier?
> 
> It is very unlikely, but the write into req_cons could happen before
> actually doing the memcpy, and so another processor could try to reuse
> the same request slot.

req_cons is not shared with the frontend driver, and most accesses within
the backend are serialised within the transmit tasklet.

 -- Keir

Re: barriers before {req/rsp}_cons = cons?

[Samuel Thibault]

Keir Fraser, le Mon 21 Jul 2008 09:28:38 +0100, a écrit :
> On 18/7/08 20:33, "Samuel Thibault" <samuel.thibault@eu.citrix.com> wrote:
> 
> >> Could you describe the race you believe is made possible by the absence of
> >> the barrier?
> > 
> > It is very unlikely, but the write into req_cons could happen before
> > actually doing the memcpy, and so another processor could try to reuse
> > the same request slot.
> 
> req_cons is not shared with the frontend driver,

Aow, right, I had forgotten that on the frontend side we are using the
rsp index, so that there is no more in-flight requests than ring slots.

Samuel