From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zoltan Kiss Subject: Re: [PATCH RFC] xen/public/ring.h: simplify RING_HAS_UNCONSUMED_REQUESTS() Date: Mon, 24 Mar 2014 12:23:33 +0000 Message-ID: <533023C5.8070704@citrix.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> <532FEF1602000078000012A1@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" 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 1WS3ul-0000uN-QR for xen-devel@lists.xenproject.org; Mon, 24 Mar 2014 12:23:40 +0000 In-Reply-To: <532FEF1602000078000012A1@nat28.tlf.novell.com> 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 Deegan Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , freebsd-xen@freebsd.org, Alan Somers , Paul Durrant , David Vrabel , xen-devel@lists.xenproject.org, Wei Liu , Boris Ostrovsky , roger.pau@citrix.com, Manuel Bouyer , John Suykerbuyk List-Id: xen-devel@lists.xenproject.org On 24/03/14 07:38, Jan Beulich wrote: >>>> 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. It's not "may", it is a "will". In case of Linux netback for sure, but I think it's reasonable for any backend to rely on the fact that the ring macros protect them from abusive frontends. Protecting a backend to read requests from the [rsp_prod_pvt, req_cons] range is a sensible thing to do in the ring macros. Also, RING_FINAL_CHECK_FOR_REQUESTS relies on this macro, removing this protection may cause other issues, e.g. netback keeps the NAPI instance spinning while it's not consuming any requests. Zoli