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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----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
> RING_HAS_UNCONSUMED_REQUESTS()
> 
> >>> 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 
check.

  Paul

> Jan


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