DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stanislaw Kardach <kda@semihalf.com>
To: David Hunt <david.hunt@intel.com>, dev@dpdk.org
Cc: upstream@semihalf.com, Stanislaw Kardach <kda@semihalf.com>,
	l.wojciechow@partner.samsung.com,
	David Marchand <david.marchand@redhat.com>
Subject: [dpdk-dev] [RFC PATCH] test/distributor: fix burst flush on worker quit
Date: Mon, 26 Apr 2021 18:33:10 +0200	[thread overview]
Message-ID: <20210426163310.1043438-1-kda@semihalf.com> (raw)

While working on RISC-V port I have encountered a situation where worker
threads get stuck in the rte_distributor_return_pkt() function in the
burst test.
After investigation some of the threads enter this function with
flag RTE_DISTRIB_GET_BUF set in the d->retptr64[0]. At the same time
main thread has already passed rte_distributor_process() so nobody will
clear this flag and hence workers can't return.

What I've noticed is that adding a flush just after the last _process(),
similarly to how quit_workers() function is written in the
test_distributor.c fixes the issue.
Additionally the issue disappears when I remove the rdtsc delay code
inside the rte_distributor_request_pkt().
However I can't get this to reproduce on x86 (even with SIMD forced
off) and with artificial delays, which is why I wonder whether I'm not
actually hiding some other issue.

Looking at the implementation of the distributor, it is based on
__atomic_* builtins and the only platform related bit in the fast-path
is the rte_rdtsc() and rte_pause(). There may be some issues in the
toolchain (I've tried so far with the Ubuntu one - 10.2.0-8ubuntu1).
I should add that all unit tests for distributor are passing so either
there's some coverage corner case or the implementation works on RISC-V.
As for RDTSC I'm using a sleep-stable time counter with 1MHz frequency
and switching to high resolution cycle counter also removes the issue
but that's the same as removing the rdtsc delay as mentioned above.

I'd love to hear from You if this fix makes any sense.

While modifying this test, I've also pulled in a fix from
test_distributor.c which ensures that each thread gets his own wakeup
packet as it's possible that when sending a burst of packets, they won't
be spread over all the workers.

Signed-off-by: Stanislaw Kardach <kda@semihalf.com>
Fixes: 7c3287a10535 ("test/distributor: add performance test for burst mode")
Cc: david.hunt@intel.com
Cc: l.wojciechow@partner.samsung.com
Cc: David Marchand <david.marchand@redhat.com>
---
 app/test/test_distributor_perf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/test/test_distributor_perf.c b/app/test/test_distributor_perf.c
index b25f79a34..fdbeae6d2 100644
--- a/app/test/test_distributor_perf.c
+++ b/app/test/test_distributor_perf.c
@@ -188,13 +188,15 @@ quit_workers(struct rte_distributor *d, struct rte_mempool *p)
 	rte_mempool_get_bulk(p, (void *)bufs, num_workers);
 
 	quit = 1;
-	for (i = 0; i < num_workers; i++)
+	for (i = 0; i < num_workers; i++) {
 		bufs[i]->hash.usr = i << 1;
-	rte_distributor_process(d, bufs, num_workers);
+		rte_distributor_process(d, &bufs[i], 1);
+	}
 
 	rte_mempool_put_bulk(p, (void *)bufs, num_workers);
 
 	rte_distributor_process(d, NULL, 0);
+	rte_distributor_flush(d);
 	rte_eal_mp_wait_lcore();
 	quit = 0;
 	worker_idx = 0;
-- 
2.27.0


             reply	other threads:[~2021-04-26 16:33 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20210426163404eucas1p13be6ae1bbcc4b947599cb606befd5370@eucas1p1.samsung.com>
2021-04-26 16:33 ` Stanislaw Kardach [this message]
2021-04-28  7:46   ` Lukasz Wojciechowski
2021-04-28 12:50     ` David Hunt
2021-04-28 12:53       ` Stanisław Kardach
2021-04-28 13:03         ` David Hunt
2021-04-28 13:11   ` David Marchand
2021-04-28 13:22     ` Stanisław Kardach
2021-04-28 14:25   ` [dpdk-dev] [PATCH v2 0/2] test/distributor: perf burst mode quit fixes Stanislaw Kardach
2021-04-28 14:25     ` [dpdk-dev] [PATCH v2 1/2] test/distributor: fix worker notification in burst Stanislaw Kardach
2021-04-28 14:25     ` [dpdk-dev] [PATCH v2 2/2] test/distributor: fix burst flush on worker quit Stanislaw Kardach
2021-05-05 14:54     ` [dpdk-dev] [PATCH v2 0/2] test/distributor: perf burst mode quit fixes David Marchand

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=20210426163310.1043438-1-kda@semihalf.com \
    --to=kda@semihalf.com \
    --cc=david.hunt@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=l.wojciechow@partner.samsung.com \
    --cc=upstream@semihalf.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).