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
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-Jul-18 19:33 UTC
Re: [Xen-devel] barriers before {req/rsp}_cons = cons?
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-Jul-18 19:34 UTC
Re: [Xen-devel] barriers before {req/rsp}_cons = cons?
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-Jul-21 10:08 UTC
Re: [Xen-devel] barriers before {req/rsp}_cons = cons?
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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel