Re: [dpdk-dev] [PATCH v5 18/19] ring: add sched_yield to avoid spin forever

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Feb 12, 2015 at 01:08:43PM +0000, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: Liang, Cunming
> > Sent: Thursday, February 12, 2015 1:05 PM
> > To: Olivier MATZ; dev@dpdk.org
> > Cc: Ananyev, Konstantin
> > Subject: RE: [PATCH v5 18/19] ring: add sched_yield to avoid spin forever
> > 
> > Hi,
> > 
> > > -----Original Message-----
> > > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > > Sent: Thursday, February 12, 2015 7:16 PM
> > > To: Liang, Cunming; dev@dpdk.org
> > > Cc: Ananyev, Konstantin
> > > Subject: Re: [PATCH v5 18/19] ring: add sched_yield to avoid spin forever
> > >
> > > Hi,
> > >
> > > On 02/12/2015 09:16 AM, Cunming Liang wrote:
> > > > Add a sched_yield() syscall if the thread spins for too long, waiting other thread
> > > to finish its operations on the ring.
> > > > That gives pre-empted thread a chance to proceed and finish with ring
> > > enqnue/dequeue operation.
> > > > The purpose is to reduce contention on the ring. By ring_perf_test, it doesn't
> > > shows additional perf penalty.
> > > >
> > > > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > > > ---
> > > >   v5 changes:
> > > >     add RTE_RING_PAUSE_REP to config file
> > > >
> > > >   v4 changes:
> > > >     update and add more comments on sched_yield()
> > > >
> > > >   v3 changes:
> > > >     new patch adding sched_yield() in rte_ring to avoid long spin
> > > >
> > > >   config/common_bsdapp       |  1 +
> > > >   config/common_linuxapp     |  1 +
> > > >   lib/librte_ring/rte_ring.h | 31 +++++++++++++++++++++++++++----
> > > >   3 files changed, 29 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/config/common_bsdapp b/config/common_bsdapp
> > > > index 57bacb8..52c5143 100644
> > > > --- a/config/common_bsdapp
> > > > +++ b/config/common_bsdapp
> > > > @@ -234,6 +234,7 @@ CONFIG_RTE_PMD_PACKET_PREFETCH=y
> > > >   CONFIG_RTE_LIBRTE_RING=y
> > > >   CONFIG_RTE_LIBRTE_RING_DEBUG=n
> > > >   CONFIG_RTE_RING_SPLIT_PROD_CONS=n
> > > > +CONFIG_RTE_RING_PAUSE_REP=n
> > >
> > > Maybe it's better to use CONFIG_RTE_RING_PAUSE_REP=0 instead?
> > > If I understand well, it has to be set to an integer value to
> > > enable it, am I correct?
> > [LCM] If RTE_RING_PAUSE_REP=N (no define), by default will use 0. If it's set to 'y'(=1), will issue yield in the most frequent rate.
> > It also can set as integer to assign any number. All cases works for this configure.
> > One point is in configure file, just demonstrate the default way to use it.
> > It can't prevent to use anything unexpected. Except we rule the n & y illegal for this option.
> > The meaningful value of it can write in the doc.
> 
> I also think that it is better avoid 'y' and 'n' (though as you said it would work), but use integers for all cases.
> Less confusion for the users.
> Konstantin 
> 
+1
Also expand "REP" to "REP_COUNT" to make it clear that it's a numeric value.

/Bruce