In the case of S2M queues, the receiver synchronizes with the sender (i.e. informs of the packets it has received) using ring->tail. Hence, the sender does not need to update last_tail. In the case of M2S queues, the receiver uses last_tail to keep track of the descriptors it has received. The sender is not required to update the last_tail. Updating the last_tail makes it a shared variable between the transmitter and receiver affecting the performance. Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") Cc: jgrajcia@cisco.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index c1c7e9f8d..fd7dbc53e 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -568,12 +568,10 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) ring_size = 1 << mq->log2_ring_size; mask = ring_size - 1; - n_free = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE) - mq->last_tail; - mq->last_tail += n_free; - if (type == MEMIF_RING_S2M) { slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); - n_free = ring_size - slot + mq->last_tail; + n_free = ring_size - slot + + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) - slot; -- 2.17.1
For M2S rings, ring->tail is updated by the sender and eth_memif_tx function is called in the context of sending thread. The loads in the sender do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index fd7dbc53e..0d064c8fa 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -573,7 +573,13 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) n_free = ring_size - slot + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { - slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); + /* For M2S queues ring->tail is updated by the sender and + * this function is called in the context of sending thread. + * The loads in the sender do not need to synchronize with + * its own stores. Hence, the following load can be a + * relaxed load. + */ + slot = __atomic_load_n(&ring->tail, __ATOMIC_RELAXED); n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) - slot; } -- 2.17.1
For S2M rings, ring->head is updated by the sender and eth_memif_tx function is called in the context of sending thread. The loads in the sender do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 0d064c8fa..435c6345c 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -569,7 +569,13 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) mask = ring_size - 1; if (type == MEMIF_RING_S2M) { - slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + /* For S2M queues ring->head is updated by the sender and + * this function is called in the context of sending thread. + * The loads in the sender do not need to synchronize with + * its own stores. Hence, the following load can be a + * relaxed load. + */ + slot = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_free = ring_size - slot + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { -- 2.17.1
For M2S rings, ring->head is updated by the receiver and eth_memif_rx function is called in the context of receiving thread. The loads in the sender do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 435c6345c..1cbb1fd7a 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -405,7 +405,11 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) refill: if (type == MEMIF_RING_M2S) { - head = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + /* ring->head is updated by the receiver and this function + * is called in the context of receiver thread. The loads in + * the receiver do not need to synchronize with its own stores. + */ + head = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_slots = ring_size - head + mq->last_tail; while (n_slots--) { -- 2.17.1
In the case of S2M queues, the receiver synchronizes with the sender (i.e. informs of the packets it has received) using ring->tail. Hence, the sender does not need to update last_tail. In the case of M2S queues, the receiver uses last_tail to keep track of the descriptors it has received. The sender is not required to update the last_tail. Updating the last_tail makes it a shared variable between the transmitter and receiver affecting the performance. Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD") Cc: jgrajcia@cisco.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index a19c0f3e6..130099f2e 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -580,12 +580,10 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) ring_size = 1 << mq->log2_ring_size; mask = ring_size - 1; - n_free = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE) - mq->last_tail; - mq->last_tail += n_free; - if (type == MEMIF_RING_S2M) { slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); - n_free = ring_size - slot + mq->last_tail; + n_free = ring_size - slot + + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) - slot; -- 2.17.1
For M2S rings, ring->tail is updated by the sender and eth_memif_tx function is called in the context of sending thread. The loads in the sender do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 130099f2e..8bacacaa8 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -585,7 +585,13 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) n_free = ring_size - slot + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { - slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); + /* For M2S queues ring->tail is updated by the sender and + * this function is called in the context of sending thread. + * The loads in the sender do not need to synchronize with + * its own stores. Hence, the following load can be a + * relaxed load. + */ + slot = __atomic_load_n(&ring->tail, __ATOMIC_RELAXED); n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE) - slot; } -- 2.17.1
For M2S rings, ring->head is updated by the receiver and eth_memif_rx function is called in the context of receiving thread. The loads in the receiver do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 8bacacaa8..159b45180 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -410,7 +410,11 @@ eth_memif_rx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) refill: if (type == MEMIF_RING_M2S) { - head = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + /* ring->head is updated by the receiver and this function + * is called in the context of receiver thread. The loads in + * the receiver do not need to synchronize with its own stores. + */ + head = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_slots = ring_size - head + mq->last_tail; while (n_slots--) { -- 2.17.1
For S2M rings, ring->head is updated by the sender and eth_memif_tx function is called in the context of sending thread. The loads in the sender do not need to synchronize with its own stores. Fixes: a2aafb9aa651 ("net/memif: optimize with one-way barrier") Cc: phil.yang@arm.com Cc: stable@dpdk.org Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 159b45180..96db76121 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -585,7 +585,13 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) mask = ring_size - 1; if (type == MEMIF_RING_S2M) { - slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + /* For S2M queues ring->head is updated by the sender and + * this function is called in the context of sending thread. + * The loads in the sender do not need to synchronize with + * its own stores. Hence, the following load can be a + * relaxed load. + */ + slot = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_free = ring_size - slot + __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); } else { -- 2.17.1
For zero-copy M2S rings, ring->head is updated by the receiver and eth_memif_rx_zc function is called in the context of receiving thread. The loads in the receiver do not need to synchronize with its own stores. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 96db76121..404c86ae8 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -514,11 +514,11 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) /* Supply master with new buffers */ refill: - /* The ring->head acts as a guard variable between Tx and Rx - * threads, so using load-acquire pairs with store-release - * to synchronize it between threads. + /* ring->head is updated by the receiver and this function + * is called in the context of receiver thread. The loads in + * the receiver do not need to synchronize with its own stores. */ - head = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + head = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_slots = ring_size - head + mq->last_tail; if (n_slots < 32) @@ -543,6 +543,10 @@ eth_memif_rx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) (uint8_t *)proc_private->regions[d0->region]->addr; } no_free_mbufs: + /* The ring->head acts as a guard variable between Tx and Rx + * threads, so using store-release pairs with load-acquire + * in function eth_memif_tx. + */ __atomic_store_n(&ring->head, head, __ATOMIC_RELEASE); mq->n_pkts += n_rx_pkts; -- 2.17.1
eth_memif_tx_zc function is called only for S2M rings. Remove additional code for M2S rings in this function. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 404c86ae8..9babd073a 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -735,7 +735,6 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) rte_eth_devices[mq->in_port].process_private; memif_ring_t *ring = memif_get_ring_from_queue(proc_private, mq); uint16_t slot, n_free, ring_size, mask, n_tx_pkts = 0; - memif_ring_type_t type = mq->type; struct rte_eth_link link; if (unlikely((pmd->flags & ETH_MEMIF_FLAG_CONNECTED) == 0)) @@ -812,11 +811,8 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) } no_free_slots: - /* update ring pointers */ - if (type == MEMIF_RING_S2M) - __atomic_store_n(&ring->head, slot, __ATOMIC_RELEASE); - else - __atomic_store_n(&ring->tail, slot, __ATOMIC_RELEASE); + /* ring type always MEMIF_RING_S2M */ + __atomic_store_n(&ring->head, slot, __ATOMIC_RELEASE); /* Send interrupt, if enabled. */ if ((ring->flags & MEMIF_RING_FLAG_MASK_INT) == 0) { -- 2.17.1
For zero-copy S2M rings, ring->head is updated by the sender and eth_memif_tx_zc function is called in the context of sending thread. The loads in the sender do not need to synchronize with its own stores. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 9babd073a..704350022 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -752,11 +752,13 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) memif_free_stored_mbufs(proc_private, mq); /* ring type always MEMIF_RING_S2M */ - /* The ring->head acts as a guard variable between Tx and Rx - * threads, so using load-acquire pairs with store-release - * to synchronize it between threads. + /* For S2M queues ring->head is updated by the sender and + * this function is called in the context of sending thread. + * The loads in the sender do not need to synchronize with + * its own stores. Hence, the following load can be a + * relaxed load. */ - slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE); + slot = __atomic_load_n(&ring->head, __ATOMIC_RELAXED); n_free = ring_size - slot + mq->last_tail; int used_slots; @@ -812,6 +814,10 @@ eth_memif_tx_zc(void *queue, struct rte_mbuf **bufs, uint16_t nb_pkts) no_free_slots: /* ring type always MEMIF_RING_S2M */ + /* The ring->head acts as a guard variable between Tx and Rx + * threads, so using store-release pairs with load-acquire + * in function eth_memif_rx for S2M rings. + */ __atomic_store_n(&ring->head, slot, __ATOMIC_RELEASE); /* Send interrupt, if enabled. */ -- 2.17.1
load-acquire memory order has a barrier. Loading it inside the loop will result in a barrier in every iteration. Hence, load the variable once outside the loop. Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> Reviewed-by: Phil Yang <phil.yang@arm.com> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com> --- drivers/net/memif/rte_eth_memif.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/memif/rte_eth_memif.c b/drivers/net/memif/rte_eth_memif.c index 704350022..c73cde8fd 100644 --- a/drivers/net/memif/rte_eth_memif.c +++ b/drivers/net/memif/rte_eth_memif.c @@ -249,16 +249,17 @@ memif_get_buffer(struct pmd_process_private *proc_private, memif_desc_t *d) static void memif_free_stored_mbufs(struct pmd_process_private *proc_private, struct memif_queue *mq) { + uint16_t cur_tail; uint16_t mask = (1 << mq->log2_ring_size) - 1; memif_ring_t *ring = memif_get_ring_from_queue(proc_private, mq); /* FIXME: improve performance */ /* The ring->tail acts as a guard variable between Tx and Rx * threads, so using load-acquire pairs with store-release - * to synchronize it between threads. + * in function eth_memif_rx for S2M queues. */ - while (mq->last_tail != __atomic_load_n(&ring->tail, - __ATOMIC_ACQUIRE)) { + cur_tail = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE); + while (mq->last_tail != cur_tail) { RTE_MBUF_PREFETCH_TO_FREE(mq->buffers[(mq->last_tail + 1) & mask]); /* Decrement refcnt and free mbuf. (current segment) */ rte_mbuf_refcnt_update(mq->buffers[mq->last_tail & mask], -1); -- 2.17.1
On Mon, 28 Sep 2020 14:03:27 -0500
Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote:
> In the case of S2M queues, the receiver synchronizes with the sender
> (i.e. informs of the packets it has received) using ring->tail.
> Hence, the sender does not need to update last_tail.
>
> In the case of M2S queues, the receiver uses last_tail to
> keep track of the descriptors it has received. The
> sender is not required to update the last_tail. Updating
> the last_tail makes it a shared variable between the
> transmitter and receiver affecting the performance.
>
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> Cc: jgrajcia@cisco.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
This patch series will conflict with the pending master/slave patchset.
Please let the master/slave renaming go in first.
<snip>
>
> On Mon, 28 Sep 2020 14:03:27 -0500
> Honnappa Nagarahalli <honnappa.nagarahalli@arm.com> wrote:
>
> > In the case of S2M queues, the receiver synchronizes with the sender
> > (i.e. informs of the packets it has received) using ring->tail.
> > Hence, the sender does not need to update last_tail.
> >
> > In the case of M2S queues, the receiver uses last_tail to keep track
> > of the descriptors it has received. The sender is not required to
> > update the last_tail. Updating the last_tail makes it a shared
> > variable between the transmitter and receiver affecting the
> > performance.
> >
> > Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> > Cc: jgrajcia@cisco.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>
> This patch series will conflict with the pending master/slave patchset.
> Please let the master/slave renaming go in first.
+1, review can go on.
Hi Jakub,
Appreciate if you could review this series and provide any comments you might have.
Thank you,
Honnappa
> -----Original Message-----
> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Sent: Monday, September 28, 2020 2:03 PM
> To: dev@dpdk.org; Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com; ferruh.yigit@intel.com
> Cc: nd <nd@arm.com>; stable@dpdk.org
> Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in tx
> function
>
> In the case of S2M queues, the receiver synchronizes with the sender (i.e.
> informs of the packets it has received) using ring->tail.
> Hence, the sender does not need to update last_tail.
>
> In the case of M2S queues, the receiver uses last_tail to keep track of the
> descriptors it has received. The sender is not required to update the last_tail.
> Updating the last_tail makes it a shared variable between the transmitter and
> receiver affecting the performance.
>
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> Cc: jgrajcia@cisco.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
> drivers/net/memif/rte_eth_memif.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/memif/rte_eth_memif.c
> b/drivers/net/memif/rte_eth_memif.c
> index a19c0f3e6..130099f2e 100644
> --- a/drivers/net/memif/rte_eth_memif.c
> +++ b/drivers/net/memif/rte_eth_memif.c
> @@ -580,12 +580,10 @@ eth_memif_tx(void *queue, struct rte_mbuf **bufs,
> uint16_t nb_pkts)
> ring_size = 1 << mq->log2_ring_size;
> mask = ring_size - 1;
>
> - n_free = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE) - mq-
> >last_tail;
> - mq->last_tail += n_free;
> -
> if (type == MEMIF_RING_S2M) {
> slot = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE);
> - n_free = ring_size - slot + mq->last_tail;
> + n_free = ring_size - slot +
> + __atomic_load_n(&ring->tail,
> __ATOMIC_ACQUIRE);
> } else {
> slot = __atomic_load_n(&ring->tail, __ATOMIC_ACQUIRE);
> n_free = __atomic_load_n(&ring->head, __ATOMIC_ACQUIRE)
> - slot;
> --
> 2.17.1
> > -----Original Message-----
> > From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Sent: Monday, September 28, 2020 2:03 PM
> > To: dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>;
> > Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com;
> > ferruh.yigit@intel.com
> > Cc: nd <nd@arm.com>; stable@dpdk.org
> > Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in
> > tx function
> >
> > In the case of S2M queues, the receiver synchronizes with the sender (i.e.
> > informs of the packets it has received) using ring->tail.
> > Hence, the sender does not need to update last_tail.
> >
> > In the case of M2S queues, the receiver uses last_tail to keep track
> > of the descriptors it has received. The sender is not required to update the
> last_tail.
> > Updating the last_tail makes it a shared variable between the
> > transmitter and receiver affecting the performance.
Hi Honnappa,
The patch series is looking good.
Reviewed-by: Jakub Grajciar <jgrajcia@cisco.com>
On 10/9/2020 12:23 PM, Jakub Grajciar -X (jgrajcia - PANTHEON TECH SRO at Cisco)
wrote:
>>> -----Original Message-----
>>> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Sent: Monday, September 28, 2020 2:03 PM
>>> To: dev@dpdk.org; Honnappa Nagarahalli
>> <Honnappa.Nagarahalli@arm.com>;
>>> Phil Yang <Phil.Yang@arm.com>; jgrajcia@cisco.com;
>>> ferruh.yigit@intel.com
>>> Cc: nd <nd@arm.com>; stable@dpdk.org
>>> Subject: [PATCH v2 1/8] net/memif: do not update local copy of tail in
>>> tx function
>>>
>>> In the case of S2M queues, the receiver synchronizes with the sender (i.e.
>>> informs of the packets it has received) using ring->tail.
>>> Hence, the sender does not need to update last_tail.
>>>
>>> In the case of M2S queues, the receiver uses last_tail to keep track
>>> of the descriptors it has received. The sender is not required to update the
>> last_tail.
>>> Updating the last_tail makes it a shared variable between the
>>> transmitter and receiver affecting the performance.
>
> Hi Honnappa,
>
> The patch series is looking good.
>
> Reviewed-by: Jakub Grajciar <jgrajcia@cisco.com>
>
Series applied to dpdk-next-net/main, thanks.