From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f46.google.com (mail-wm0-f46.google.com [74.125.82.46]) by dpdk.org (Postfix) with ESMTP id 9A61BD018 for ; Tue, 14 Mar 2017 09:56:34 +0100 (CET) Received: by mail-wm0-f46.google.com with SMTP id v186so58341270wmd.0 for ; Tue, 14 Mar 2017 01:56:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=pbFaH4SPy+7DspsD9hoeBu/EgePKKyBMVawyzBpBtk4=; b=G+ZPiYdSp1xfh4N1VsgOWdxLofW3mJuoJMiFsT8r3nUiDmCFx4PO7xJ7Gycgrkp6/Q vPnK9TxOqZaqmtD2/1CXqKR+mgJ4Rn74MLdDaRHeCuB0U/EAXipaTjvWIUJw8Vbr9dEQ lZ3ycTqHfGUx8hfgl5wH4xrC1fEhYDNgcmr7dKO6Vw51U3AOYLoIY2MdXieDoj9LzEJ/ A8DHfFn5V0Rwt9iIFiz5qj9kMyeC1Ptreq3uDluC4a7VqdWURWQulxK5EwKbgWUsWM1j VmkZTIPs0KC8KWsNiEatCmH4T3JH6LudPJuIsM/FjnSwt7gg3ZuA3NGNpFkmXnraGIs5 QhIA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=pbFaH4SPy+7DspsD9hoeBu/EgePKKyBMVawyzBpBtk4=; b=px8/njP8RaX9BvlZZnrzo4yqmBgObAkaw9tD9qUdbYptIKGFWOEBjsDUwjh5nl5YvM KApueH2Wj7wi4Xi4ShWDHZDf5310UQmjrNUHTkqabg51W8hk3CyvODLfpoQm7gDmVpsj pdZLuniSwTF1S7FxvOycNiFshMGwWjmiOu0SSvVXTKXV98U+Il9jmc8zZicdcsgbYPhD tE8Nj0aYzc1X8jwMpPc3Eh4bnBv0s+7GsyqQDvnnGR8Fhxo2Gfz2sgHnYe5lQOs7ouho BmOjZFzXPmbRH2GYK/uqFwxG/PEb41l23Ur4Z5jkwVt/JWnWpObKUxxAVftdjJWS1bjZ LjSQ== X-Gm-Message-State: AFeK/H0k5WHXSlvCWsZaGenJ/MFvMC8kebTpXQqOoqzEb64IAgFivnWRC2DosbTVrTP5+qpA X-Received: by 10.28.8.147 with SMTP id 141mr14317157wmi.43.1489481793935; Tue, 14 Mar 2017 01:56:33 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id 53sm18237045wrt.52.2017.03.14.01.56.33 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 14 Mar 2017 01:56:33 -0700 (PDT) Date: Tue, 14 Mar 2017 09:56:31 +0100 From: Olivier Matz To: Bruce Richardson Cc: dev@dpdk.org Message-ID: <20170314095631.3d93623c@platinum> In-Reply-To: <20170308120654.GA286404@bricha3-MOBL3.ger.corp.intel.com> References: <20170223172407.27664-1-bruce.richardson@intel.com> <20170223172407.27664-13-bruce.richardson@intel.com> <20170308114906.6101f90b@glumotte.dev.6wind.com> <20170308120654.GA286404@bricha3-MOBL3.ger.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v1 12/14] ring: separate out head index manipulation for enq/deq X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Mar 2017 08:56:34 -0000 On Wed, 8 Mar 2017 12:06:54 +0000, Bruce Richardson wrote: > On Wed, Mar 08, 2017 at 11:49:06AM +0100, Olivier MATZ wrote: > > On Thu, 23 Feb 2017 17:24:05 +0000, Bruce Richardson wrote: > > > We can write a single common function for head manipulation for enq > > > and a common one for deq, allowing us to have a single worker function > > > for enq and deq, rather than two of each. Update all other inline > > > functions to use the new functions. > > > > > > Signed-off-by: Bruce Richardson > > > --- > > > lib/librte_ring/rte_ring.c | 4 +- > > > lib/librte_ring/rte_ring.h | 328 ++++++++++++++++++++------------------------- > > > 2 files changed, 149 insertions(+), 183 deletions(-) > > > > > > > [...] > > > > > +static inline __attribute__((always_inline)) unsigned int > > > +__rte_ring_do_enqueue(struct rte_ring *r, void * const *obj_table, > > > + unsigned int n, enum rte_ring_queue_behavior behavior, > > > + int is_sp, unsigned int *free_space) > > > { > > > - uint32_t prod_head, cons_tail; > > > - uint32_t prod_next, free_entries; > > > - uint32_t mask = r->mask; > > > - > > > - prod_head = r->prod.head; > > > - cons_tail = r->cons.tail; > > > - /* The subtraction is done between two unsigned 32bits value > > > - * (the result is always modulo 32 bits even if we have > > > - * prod_head > cons_tail). So 'free_entries' is always between 0 > > > - * and size(ring)-1. */ > > > - free_entries = mask + cons_tail - prod_head; > > > - > > > - /* check that we have enough room in ring */ > > > - if (unlikely(n > free_entries)) > > > - n = (behavior == RTE_RING_QUEUE_FIXED) ? 0 : free_entries; > > > + uint32_t prod_head, prod_next; > > > + uint32_t free_entries; > > > > > > + n = __rte_ring_move_prod_head(r, is_sp, n, behavior, > > > + &prod_head, &prod_next, &free_entries); > > > if (n == 0) > > > goto end; > > > > > > - > > > - prod_next = prod_head + n; > > > - r->prod.head = prod_next; > > > - > > > - /* write entries in ring */ > > > ENQUEUE_PTRS(); > > > rte_smp_wmb(); > > > > > > + /* > > > + * If there are other enqueues in progress that preceded us, > > > + * we need to wait for them to complete > > > + */ > > > + while (unlikely(r->prod.tail != prod_head)) > > > + rte_pause(); > > > + > > > > I'd say this part should not be done in case is_sp == 1. > > Since it is sometimes a constant arg in an inline func, it may be better > > to add the if (is_sp == 0). > > > > [...] > > > > Yes, it's an unnecessary check. However, having it in place for the sp > case made no performance difference in my test, so I decided to keep > the code shorter by avoiding an additional branch. If there is a > performance hit I'll remove it, but I would rather not add more branches > to the code in the absense of a real impact to not having them. Ok. Maybe it's worth checking the numbers given by the unit test. Olivier