DPDK patches and discussions
 help / color / mirror / Atom feed
From: Phil Yang <phil.yang@arm.com>
To: dev@dpdk.org
Cc: nd@arm.com, jerin.jacob@caviumnetworks.com,
	kkokkilagadda@caviumnetworks.com, Honnappa.Nagarahalli@arm.com,
	Gavin.Hu@arm.com
Subject: [dpdk-dev] [PATCH v2 2/3] kni: fix kni fifo synchronization
Date: Wed, 19 Sep 2018 21:42:39 +0800	[thread overview]
Message-ID: <1537364560-4124-2-git-send-email-phil.yang@arm.com> (raw)
In-Reply-To: <1537364560-4124-1-git-send-email-phil.yang@arm.com>

With existing code in kni_fifo_put, rx_q values are not being updated
before updating fifo_write. While reading rx_q in kni_net_rx_normal,
This is causing the sync issue on other core. The same situation happens
in kni_fifo_get as well.

So syncing the values by adding C11 atomic memory barriers to make sure
the values being synced before updating fifo_write and fifo_read.

Fixes: 3fc5ca2 ("kni: initial import")
Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Reviewed-by: Gavin Hu <Gavin.Hu@arm.com>
---
 .../linuxapp/eal/include/exec-env/rte_kni_common.h |  5 ++++
 lib/librte_kni/rte_kni_fifo.h                      | 30 +++++++++++++++++++++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
index cfa9448..1fd713b 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_kni_common.h
@@ -54,8 +54,13 @@ struct rte_kni_request {
  * Writing should never overwrite the read position
  */
 struct rte_kni_fifo {
+#ifndef RTE_USE_C11_MEM_MODEL
 	volatile unsigned write;     /**< Next position to be written*/
 	volatile unsigned read;      /**< Next position to be read */
+#else
+	unsigned write;              /**< Next position to be written*/
+	unsigned read;               /**< Next position to be read */
+#endif
 	unsigned len;                /**< Circular buffer length */
 	unsigned elem_size;          /**< Pointer size - for 32/64 bit OS */
 	void *volatile buffer[];     /**< The buffer contains mbuf pointers */
diff --git a/lib/librte_kni/rte_kni_fifo.h b/lib/librte_kni/rte_kni_fifo.h
index ac26a8c..f4171a1 100644
--- a/lib/librte_kni/rte_kni_fifo.h
+++ b/lib/librte_kni/rte_kni_fifo.h
@@ -28,8 +28,13 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num)
 {
 	unsigned i = 0;
 	unsigned fifo_write = fifo->write;
-	unsigned fifo_read = fifo->read;
 	unsigned new_write = fifo_write;
+#ifdef RTE_USE_C11_MEM_MODEL
+	unsigned fifo_read = __atomic_load_n(&fifo->read,
+						 __ATOMIC_ACQUIRE);
+#else
+	unsigned fifo_read = fifo->read;
+#endif
 
 	for (i = 0; i < num; i++) {
 		new_write = (new_write + 1) & (fifo->len - 1);
@@ -39,7 +44,12 @@ kni_fifo_put(struct rte_kni_fifo *fifo, void **data, unsigned num)
 		fifo->buffer[fifo_write] = data[i];
 		fifo_write = new_write;
 	}
+#ifdef RTE_USE_C11_MEM_MODEL
+	__atomic_store_n(&fifo->write, fifo_write, __ATOMIC_RELEASE);
+#else
+	rte_smp_wmb();
 	fifo->write = fifo_write;
+#endif
 	return i;
 }
 
@@ -51,7 +61,12 @@ kni_fifo_get(struct rte_kni_fifo *fifo, void **data, unsigned num)
 {
 	unsigned i = 0;
 	unsigned new_read = fifo->read;
+#ifdef RTE_USE_C11_MEM_MODEL
+	unsigned fifo_write = __atomic_load_n(&fifo->write, __ATOMIC_ACQUIRE);
+#else
 	unsigned fifo_write = fifo->write;
+#endif
+
 	for (i = 0; i < num; i++) {
 		if (new_read == fifo_write)
 			break;
@@ -59,7 +74,12 @@ kni_fifo_get(struct rte_kni_fifo *fifo, void **data, unsigned num)
 		data[i] = fifo->buffer[new_read];
 		new_read = (new_read + 1) & (fifo->len - 1);
 	}
+#ifdef RTE_USE_C11_MEM_MODEL
+	__atomic_store_n(&fifo->read, new_read, __ATOMIC_RELEASE);
+#else
+	rte_smp_wmb();
 	fifo->read = new_read;
+#endif
 	return i;
 }
 
@@ -69,5 +89,13 @@ kni_fifo_get(struct rte_kni_fifo *fifo, void **data, unsigned num)
 static inline uint32_t
 kni_fifo_count(struct rte_kni_fifo *fifo)
 {
+#ifdef RTE_USE_C11_MEM_MODEL
+	unsigned fifo_write = __atomic_load_n(&fifo->write,
+						  __ATOMIC_ACQUIRE);
+	unsigned fifo_read = __atomic_load_n(&fifo->read,
+						 __ATOMIC_ACQUIRE);
+	return (fifo->len + fifo_write - fifo_read) & (fifo->len - 1);
+#else
 	return (fifo->len + fifo->write - fifo->read) & (fifo->len - 1);
+#endif
 }
-- 
2.7.4

  reply	other threads:[~2018-09-19 13:44 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-19 13:30 [dpdk-dev] [PATCH 1/3] config: use one single config option for C11 memory model Phil Yang
2018-09-19 13:30 ` [dpdk-dev] [PATCH 2/3] kni: fix kni fifo synchronization Phil Yang
2018-09-19 13:30 ` [dpdk-dev] [PATCH 3/3] kni: fix kni kernel " Phil Yang
2018-09-19 13:42 ` [dpdk-dev] [PATCH v2 1/3] config: use one single config option for C11 memory model Phil Yang
2018-09-19 13:42   ` Phil Yang [this message]
2018-09-20  8:28     ` [dpdk-dev] [PATCH v2 2/3] kni: fix kni fifo synchronization Jerin Jacob
2018-09-20 15:20       ` Honnappa Nagarahalli
2018-09-20 15:37         ` Jerin Jacob
2018-09-21  5:48           ` Honnappa Nagarahalli
2018-09-21  5:55             ` Jerin Jacob
2018-09-21  6:37               ` Honnappa Nagarahalli
2018-09-21  9:00                 ` Phil Yang (Arm Technology China)
2018-09-25  4:44                 ` Honnappa Nagarahalli
2018-09-26 11:42                 ` Ferruh Yigit
2018-09-27  9:06                   ` Phil Yang (Arm Technology China)
2018-09-26 11:45     ` Ferruh Yigit
2018-10-01  4:52       ` Honnappa Nagarahalli
2018-09-19 13:42   ` [dpdk-dev] [PATCH v2 3/3] kni: fix kni kernel " Phil Yang
2018-09-20  8:21   ` [dpdk-dev] [PATCH v2 1/3] config: use one single config option for C11 memory model Jerin Jacob
2018-10-08  9:11   ` [dpdk-dev] [PATCH v3 1/4] " Phil Yang
2018-10-08  9:11     ` [dpdk-dev] [PATCH v3 2/4] kni: fix kni fifo synchronization Phil Yang
2018-10-08 21:53       ` Stephen Hemminger
2018-10-10  9:58         ` Phil Yang (Arm Technology China)
2018-10-10 10:06           ` Gavin Hu (Arm Technology China)
2018-10-10 14:42             ` Ferruh Yigit
2018-10-08  9:11     ` [dpdk-dev] [PATCH v3 3/4] kni: fix kni kernel " Phil Yang
2018-10-08  9:11     ` [dpdk-dev] [PATCH v3 4/4] kni: introduce c11 atomic into kni " Phil Yang
2018-10-10 14:48     ` [dpdk-dev] [PATCH v3 1/4] config: use one single config option for C11 memory model Ferruh Yigit
2018-10-12  9:17       ` Phil Yang (Arm Technology China)
2018-10-26 15:56       ` 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=1537364560-4124-2-git-send-email-phil.yang@arm.com \
    --to=phil.yang@arm.com \
    --cc=Gavin.Hu@arm.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=dev@dpdk.org \
    --cc=jerin.jacob@caviumnetworks.com \
    --cc=kkokkilagadda@caviumnetworks.com \
    --cc=nd@arm.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).