From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: Re: [PATCH RFC] xen/public/ring.h: simplify RING_HAS_UNCONSUMED_REQUESTS() Date: Mon, 24 Mar 2014 09:39:37 +0000 Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD0296A52@AMSPEX01CL01.citrite.net> References: <5318987C.3030303@citrix.com> <1394121221.13270.10.camel@hastur.hellion.org.uk> <5318A2D8.3090808@citrix.com> <20140306173057.GK11475@deinos.phlegethon.org> <20140313163806.GB41479@deinos.phlegethon.org> <532D9B9B.20705@schaman.hu> <20140322171451.GA78509@deinos.phlegethon.org> <532FEF1602000078000012A1@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WS1M4-0006c7-Sm for xen-devel@lists.xenproject.org; Mon, 24 Mar 2014 09:39:41 +0000 In-Reply-To: <532FEF1602000078000012A1@nat28.tlf.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Zoltan Kiss , "Tim (Xen.org)" Cc: Wei Liu , Ian Campbell , "Keir (Xen.org)" , "freebsd-xen@freebsd.org" , Alan Somers , Manuel Bouyer , Stefano Stabellini , David Vrabel , "xen-devel@lists.xenproject.org" , Boris Ostrovsky , John Suykerbuyk , Roger Pau Monne List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > 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@freebsd.org; xen- > devel@lists.xenproject.org; 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, 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