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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.03.14 at 10:39, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----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.

Right, but the reduced protection won't be noticeable on a re-build,
and syncing up header files may be a purely mechanical (perhaps even
automated) operation.

> 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.

I too am in favor of changing the canonical header; I only wanted to
point out that this isn't without risk of regressions.

Jan


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