DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Olivier MATZ <olivier.matz@6wind.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v1 12/14] ring: separate out head index manipulation for enq/deq
Date: Wed, 8 Mar 2017 12:06:54 +0000	[thread overview]
Message-ID: <20170308120654.GA286404@bricha3-MOBL3.ger.corp.intel.com> (raw)
In-Reply-To: <20170308114906.6101f90b@glumotte.dev.6wind.com>

On Wed, Mar 08, 2017 at 11:49:06AM +0100, Olivier MATZ wrote:
> On Thu, 23 Feb 2017 17:24:05 +0000, Bruce Richardson <bruce.richardson@intel.com> 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 <bruce.richardson@intel.com>
> > ---
> >  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.

Regards,
/Bruce

  reply	other threads:[~2017-03-08 12:06 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-23 17:23 [dpdk-dev] [PATCH v1 00/14] refactor and cleanup of rte_ring Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 01/14] ring: remove split cacheline build setting Bruce Richardson
2017-02-28 11:35   ` Jerin Jacob
2017-02-28 11:57     ` Bruce Richardson
2017-02-28 12:08       ` Jerin Jacob
2017-02-28 13:52         ` Bruce Richardson
2017-02-28 17:54           ` Jerin Jacob
2017-03-01  9:47             ` Bruce Richardson
2017-03-01 10:17               ` Olivier Matz
2017-03-01 10:42                 ` Bruce Richardson
2017-03-01 11:06                   ` Olivier Matz
2017-03-01 11:19                     ` Jerin Jacob
2017-03-01 12:12                       ` Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 02/14] ring: create common structure for prod and cons metadata Bruce Richardson
2017-03-01 10:22   ` Olivier Matz
2017-03-01 10:33     ` Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 03/14] ring: eliminate duplication of size and mask fields Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 04/14] ring: remove debug setting Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 05/14] ring: remove the yield when waiting for tail update Bruce Richardson
2017-02-23 17:23 ` [dpdk-dev] [PATCH v1 06/14] ring: remove watermark support Bruce Richardson
2017-03-01 10:34   ` Olivier Matz
2017-03-01 10:43     ` Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 07/14] ring: make bulk and burst fn return vals consistent Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 08/14] ring: allow enqueue fns to return free space value Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 09/14] ring: allow dequeue fns to return remaining entry count Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 10/14] examples/quota_watermark: use ring space for watermarks Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 11/14] ring: reduce scope of local variables Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 12/14] ring: separate out head index manipulation for enq/deq Bruce Richardson
2017-03-08 10:49   ` Olivier MATZ
2017-03-08 12:06     ` Bruce Richardson [this message]
2017-03-14  8:56       ` Olivier Matz
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 13/14] ring: create common function for updating tail idx Bruce Richardson
2017-02-23 17:24 ` [dpdk-dev] [PATCH v1 14/14] ring: make ring struct and enq/deq macros type agnostic Bruce Richardson
2017-03-07 11:32 ` [dpdk-dev] [PATCH v2 00/14] refactor and cleanup of rte_ring Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 01/14] ring: remove split cacheline build setting Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 02/14] ring: create common structure for prod and cons metadata Bruce Richardson
2017-03-15 14:01     ` Thomas Monjalon
2017-03-22 16:38       ` Bruce Richardson
2017-03-24 14:55       ` Bruce Richardson
2017-03-24 16:41         ` Olivier Matz
2017-03-24 16:57           ` Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 03/14] ring: eliminate duplication of size and mask fields Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 04/14] ring: remove debug setting Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 05/14] ring: remove the yield when waiting for tail update Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 06/14] ring: remove watermark support Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 07/14] ring: make bulk and burst fn return vals consistent Bruce Richardson
2017-03-08 10:22     ` Olivier MATZ
2017-03-08 12:08       ` Bruce Richardson
2017-03-14  8:56         ` Olivier Matz
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 08/14] ring: allow enqueue fns to return free space value Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 09/14] ring: allow dequeue fns to return remaining entry count Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 10/14] examples/quota_watermark: use ring space for watermarks Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 11/14] ring: reduce scope of local variables Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 12/14] ring: separate out head index manipulation for enq/deq Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 13/14] ring: create common function for updating tail idx Bruce Richardson
2017-03-07 11:32   ` [dpdk-dev] [PATCH v2 14/14] ring: make ring struct and enq/deq macros type agnostic Bruce Richardson
2017-03-14  8:59   ` [dpdk-dev] [PATCH v2 00/14] refactor and cleanup of rte_ring Olivier Matz
2017-03-24 17:09   ` [dpdk-dev] [PATCH v3 " Bruce Richardson
2017-03-24 17:09     ` [dpdk-dev] [PATCH v3 01/14] ring: remove split cacheline build setting Bruce Richardson
2017-03-24 17:09     ` [dpdk-dev] [PATCH v3 02/14] ring: create common structure for prod and cons metadata Bruce Richardson
2017-03-27  7:20       ` Olivier Matz
2017-03-24 17:09     ` [dpdk-dev] [PATCH v3 03/14] ring: eliminate duplication of size and mask fields Bruce Richardson
2017-03-27  9:52       ` Thomas Monjalon
2017-03-27 10:13         ` Bruce Richardson
2017-03-27 10:15         ` Bruce Richardson
2017-03-27 13:13           ` Thomas Monjalon
2017-03-27 14:57             ` Bruce Richardson
2017-03-24 17:09     ` [dpdk-dev] [PATCH v3 04/14] ring: remove debug setting Bruce Richardson
2017-03-24 17:09     ` [dpdk-dev] [PATCH v3 05/14] ring: remove the yield when waiting for tail update Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 06/14] ring: remove watermark support Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 07/14] ring: make bulk and burst fn return vals consistent Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 08/14] ring: allow enqueue fns to return free space value Bruce Richardson
2017-03-28  7:12       ` Thomas Monjalon
2017-03-28  8:16         ` Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 09/14] ring: allow dequeue fns to return remaining entry count Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 10/14] examples/quota_watermark: use ring space for watermarks Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 11/14] ring: reduce scope of local variables Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 12/14] ring: separate out head index manipulation for enq/deq Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 13/14] ring: create common function for updating tail idx Bruce Richardson
2017-03-24 17:10     ` [dpdk-dev] [PATCH v3 14/14] ring: make ring struct and enq/deq macros type agnostic Bruce Richardson
2017-03-28 20:35     ` [dpdk-dev] [PATCH v4 00/14] refactor and cleanup of rte_ring Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 01/14] ring: remove split cacheline build setting Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 02/14] ring: create common structure for prod and cons metadata Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 03/14] ring: eliminate duplication of size and mask fields Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 04/14] ring: remove debug setting Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 05/14] ring: remove the yield when waiting for tail update Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 06/14] ring: remove watermark support Bruce Richardson
2017-03-28 20:35       ` [dpdk-dev] [PATCH v4 07/14] ring: make bulk and burst fn return vals consistent Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 08/14] ring: allow enqueue fns to return free space value Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 09/14] ring: allow dequeue fns to return remaining entry count Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 10/14] examples/quota_watermark: use ring space for watermarks Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 11/14] ring: reduce scope of local variables Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 12/14] ring: separate out head index manipulation for enq/deq Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 13/14] ring: create common function for updating tail idx Bruce Richardson
2017-03-28 20:36       ` [dpdk-dev] [PATCH v4 14/14] ring: make ring struct and enq/deq macros type agnostic Bruce Richardson
2017-03-29  2:47       ` [dpdk-dev] [PATCH v4 00/14] refactor and cleanup of rte_ring Yuanhan Liu
2017-03-29 13:09       ` [dpdk-dev] [PATCH v5 " Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 01/14] ring: remove split cacheline build setting Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 02/14] ring: create common structure for prod and cons metadata Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 03/14] ring: eliminate duplication of size and mask fields Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 04/14] ring: remove debug setting Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 05/14] ring: remove the yield when waiting for tail update Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 06/14] ring: remove watermark support Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 07/14] ring: make bulk and burst fn return vals consistent Bruce Richardson
2017-04-13  6:42           ` Wang, Zhihong
2017-04-13  8:33             ` Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 08/14] ring: allow enqueue fns to return free space value Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 09/14] ring: allow dequeue fns to return remaining entry count Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 10/14] examples/quota_watermark: use ring space for watermarks Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 11/14] ring: reduce scope of local variables Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 12/14] ring: separate out head index manipulation for enq/deq Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 13/14] ring: create common function for updating tail idx Bruce Richardson
2017-03-29 13:09         ` [dpdk-dev] [PATCH v5 14/14] ring: make ring struct and enq/deq macros type agnostic Bruce Richardson
2017-03-29 20:33         ` [dpdk-dev] [PATCH v5 00/14] refactor and cleanup of rte_ring Thomas Monjalon
2017-03-31 14:37         ` Ferruh Yigit
2017-04-03 17:55           ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170308120654.GA286404@bricha3-MOBL3.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).