[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC] xen/public/ring.h: simplify RING_HAS_UNCONSUMED_REQUESTS()

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 24 March 2014 07:39
> To: Zoltan Kiss; Tim (Xen.org)
> Cc: David Vrabel; Ian Campbell; Paul Durrant; Roger Pau Monne; Wei Liu;
> Stefano Stabellini; freebsd-xen@xxxxxxxxxxx; xen-
> devel@xxxxxxxxxxxxxxxxxxxx; Manuel Bouyer; Boris Ostrovsky; Konrad
> Rzeszutek Wilk; Alan Somers; John Suykerbuyk; Keir (Xen.org)
> Subject: Re: [PATCH RFC] xen/public/ring.h: simplify
> >>> On 22.03.14 at 18:14, <tim@xxxxxxx> wrote:
> > At 14:18 +0000 on 22 Mar (1395494283), Zoltan Kiss wrote:
> >> I think I might have an explanation why do we need this, see this mailing:
> >>
> >> https://lkml.org/lkml/2014/3/20/710
> >> https://lkml.org/lkml/2014/3/21/111
> >> https://lkml.org/lkml/2014/3/21/390
> >
> > Quoting from the third of these:
> >
> > | But consuming overrunning requests after rsp_prod_pvt is a problem:
> > | - NAPI instance races with dealloc thread over the slots. The first
> > | reads them as requests, the second writes them as responses
> > | - the NAPI instance overwrites used pending slots as well, so skb frag
> > | release go wrong etc.
> >
> > OK, so the backend needs to be careful not to follow the frontend into
> > overrun, not because of the ring itself being corrupted but because it
> > will mess up the backend's internal bookkeeping.
> With s/will/may/ I'm not sure that's a reason to withdraw the patch:
> The generic macros in ring.h imo shouldn't dictate any particular
> protection policy beyond protecting the ring itself. I.e. I'd think if
> netback need protection beyond the one provided by ring.h macros,
> it should take care to implement them itself.
> Yet of course I can see that weakening the protection we have had
> in place for so many years may result in very undesirable fallout.

But these are, of course, macros and so the protection is baked into any old 
code. I'm still in favour of changing the macro in the canonical header and 
adding a comment to point out that older versions of the macro had the extra 


> Jan

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.