On 2018-03-29 14:38, Van Haaren, Harry wrote:
From: dev [mailto:[email protected]] On Behalf Of Mattias Rönnblom Sent: Wednesday, March 28, 2018 10:17 AM To: [email protected] Cc: Mattias Rönnblom <[email protected]> Subject: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ringrte_event_ring enqueue and dequeue tail updates were hardcoded for a SC/SP configuration. Signed-off-by: Mattias Rönnblom <[email protected]> ---Hi Mattias, Thanks for your patch, a few notes; 1) You can CC the maintainer of a section - the event rings falls under the eventdev library, and Jerin Jacob is the maintainer. 2) In DPDK we note patches that are "Fixes", so we can track what commit they fix. Fixes patches are also often candidates for backporting. Details on getting the fixes line here: https://dpdk.org/doc/guides/contributing/patches.html#commit-messages-body For this patch, the following is the fixline: Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") Cc: [email protected]
Thanks for your help. I'll resubmit and try to get the administrative details correct.
lib/librte_eventdev/rte_event_ring.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/librte_eventdev/rte_event_ring.h b/lib/librte_eventdev/rte_event_ring.h index 29d4228..07b4559 100644 --- a/lib/librte_eventdev/rte_event_ring.h +++ b/lib/librte_eventdev/rte_event_ring.h @@ -99,7 +99,7 @@ rte_event_ring_enqueue_burst(struct rte_event_ring *r, ENQUEUE_PTRS(&r->r, &r[1], prod_head, events, n, struct rte_event); - update_tail(&r->r.prod, prod_head, prod_next, 1, 1); + update_tail(&r->r.prod, prod_head, prod_next, 1, r->r.prod.single); end: if (free_space != NULL) *free_space = free_entries - n; @@ -140,7 +140,7 @@ rte_event_ring_dequeue_burst(struct rte_event_ring *r, DEQUEUE_PTRS(&r->r, &r[1], cons_head, events, n, struct rte_event); - update_tail(&r->r.cons, cons_head, cons_next, 1, 0); + update_tail(&r->r.cons, cons_head, cons_next, 1, r->r.cons.single);The signature of update_tail() is as follows: static __rte_always_inline void update_tail(struct rte_ring_headtail *ht, uint32_t old_val, uint32_t new_val, uint32_t single, uint32_t enqueue) It seems to be that the 2nd last parameter sets the single/multi producer?
Yes, you are right. I ported this fix from my 17.08 tree, and update_tail() has changed since then - something I failed to notice.
I reproduced the issue on 17.08, and even on a machine with a strong memory model, the event ring doesn't work correctly in a MP/SC configuration. With the fix, it at least passes my tests on 17.08. I didn't try MP/MC, and I also didn't try anything beyond building on HEAD.
Regards,
Mattias
