From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH RFC] xen/public/ring.h: simplify RING_HAS_UNCONSUMED_REQUESTS() Date: Mon, 24 Mar 2014 07:38:46 +0000 Message-ID: <532FEF1602000078000012A1@nat28.tlf.novell.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WRzTA-000097-V1 for xen-devel@lists.xenproject.org; Mon, 24 Mar 2014 07:38:53 +0000 In-Reply-To: <20140322171451.GA78509@deinos.phlegethon.org> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Zoltan Kiss , Tim Deegan Cc: Wei Liu , Ian Campbell , Stefano Stabellini , Keir Fraser , freebsd-xen@freebsd.org, Alan Somers , Paul Durrant , David Vrabel , xen-devel@lists.xenproject.org, Boris Ostrovsky , John Suykerbuyk , Manuel Bouyer , roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org >>> 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. Jan