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

On Thu, Feb 12, 2015 at 01:08:43PM +0000, Ananyev, Konstantin wrote:

quoted

-----Original Message-----
From: Liang, Cunming
Sent: Thursday, February 12, 2015 1:05 PM
To: Olivier MATZ; dev-VfR2kkLFssw@public.gmane.org
Cc: Ananyev, Konstantin
Subject: RE: [PATCH v5 18/19] ring: add sched_yield to avoid spin forever

Hi,

quoted

-----Original Message-----
From: Olivier MATZ [mailto:olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org]
Sent: Thursday, February 12, 2015 7:16 PM
To: Liang, Cunming; dev-VfR2kkLFssw@public.gmane.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:

quoted

Add a sched_yield() syscall if the thread spins for too long, waiting other thread

to finish its operations on the ring.

quoted

That gives pre-empted thread a chance to proceed and finish with ring

enqnue/dequeue operation.

quoted

The purpose is to reduce contention on the ring. By ring_perf_test, it doesn't

shows additional perf penalty.

quoted

Signed-off-by: Cunming Liang <redacted>
---
  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