* [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring @ 2018-03-28 9:16 Mattias Rönnblom 2018-03-29 12:38 ` Van Haaren, Harry 2018-04-03 8:19 ` Mattias Rönnblom 0 siblings, 2 replies; 9+ messages in thread From: Mattias Rönnblom @ 2018-03-28 9:16 UTC (permalink / raw) To: dev; +Cc: Mattias Rönnblom rte_event_ring enqueue and dequeue tail updates were hardcoded for a SC/SP configuration. Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> --- 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); end: if (available != NULL) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-03-28 9:16 [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring Mattias Rönnblom @ 2018-03-29 12:38 ` Van Haaren, Harry 2018-04-02 2:55 ` Jerin Jacob 2018-04-03 8:14 ` Mattias Rönnblom 2018-04-03 8:19 ` Mattias Rönnblom 1 sibling, 2 replies; 9+ messages in thread From: Van Haaren, Harry @ 2018-03-29 12:38 UTC (permalink / raw) To: Mattias Rönnblom, dev > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mattias Rönnblom > Sent: Wednesday, March 28, 2018 10:17 AM > To: dev@dpdk.org > Cc: Mattias Rönnblom <hofors@lysator.liu.se> > Subject: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in > rte_event_ring > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > SC/SP configuration. > > Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> > --- 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: bruce.richardson@intel.com > 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? The value that has been changed in this patch is the enqueue or dequeue parameter, which changes the type of memory barrier that is used.. Please review this code, and you can post a V2 of the patch in reply to this one (see contrib guidelines above for details). Regards, -Harry ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-03-29 12:38 ` Van Haaren, Harry @ 2018-04-02 2:55 ` Jerin Jacob 2018-04-03 8:14 ` Mattias Rönnblom 1 sibling, 0 replies; 9+ messages in thread From: Jerin Jacob @ 2018-04-02 2:55 UTC (permalink / raw) To: Van Haaren, Harry; +Cc: Mattias Rönnblom, dev -----Original Message----- > Date: Thu, 29 Mar 2018 12:38:01 +0000 > From: "Van Haaren, Harry" <harry.van.haaren@intel.com> > To: Mattias Rönnblom <hofors@lysator.liu.se>, "dev@dpdk.org" <dev@dpdk.org> > Subject: Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates > in rte_event_ring > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mattias Rönnblom > > Sent: Wednesday, March 28, 2018 10:17 AM > > To: dev@dpdk.org > > Cc: Mattias Rönnblom <hofors@lysator.liu.se> > > Subject: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in > > rte_event_ring > > > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > > SC/SP configuration. > > > > Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> > > --- > > 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: bruce.richardson@intel.com > Hi Mattias, Could you please the v2 version addressing Harry's comments. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-03-29 12:38 ` Van Haaren, Harry 2018-04-02 2:55 ` Jerin Jacob @ 2018-04-03 8:14 ` Mattias Rönnblom 1 sibling, 0 replies; 9+ messages in thread From: Mattias Rönnblom @ 2018-04-03 8:14 UTC (permalink / raw) To: Van Haaren, Harry, dev On 2018-03-29 14:38, Van Haaren, Harry wrote: >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mattias Rönnblom >> Sent: Wednesday, March 28, 2018 10:17 AM >> To: dev@dpdk.org >> Cc: Mattias Rönnblom <hofors@lysator.liu.se> >> Subject: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in >> rte_event_ring >> >> rte_event_ring enqueue and dequeue tail updates were hardcoded for a >> SC/SP configuration. >> >> Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> >> --- > > 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: bruce.richardson@intel.com > > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-03-28 9:16 [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring Mattias Rönnblom 2018-03-29 12:38 ` Van Haaren, Harry @ 2018-04-03 8:19 ` Mattias Rönnblom 2018-04-04 0:52 ` Jerin Jacob 1 sibling, 1 reply; 9+ messages in thread From: Mattias Rönnblom @ 2018-04-03 8:19 UTC (permalink / raw) To: dev; +Cc: jerin.jacob, Mattias Rönnblom, bruce.richardson rte_event_ring enqueue and dequeue tail updates were hardcoded for a SC/SP configuration. Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") Cc: bruce.richardson@intel.com Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> v2: * Fixed issue with update_tail() parameter order. --- 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..827a320 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, r->r.prod.single, 1); 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, r->r.cons.single, 0); end: if (available != NULL) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-04-03 8:19 ` Mattias Rönnblom @ 2018-04-04 0:52 ` Jerin Jacob 2018-04-04 8:52 ` Bruce Richardson 0 siblings, 1 reply; 9+ messages in thread From: Jerin Jacob @ 2018-04-04 0:52 UTC (permalink / raw) To: Mattias Rönnblom; +Cc: dev, bruce.richardson -----Original Message----- > Date: Tue, 3 Apr 2018 10:19:03 +0200 > From: Mattias Rönnblom <hofors@lysator.liu.se> > To: dev@dpdk.org > CC: jerin.jacob@caviumnetworks.com, Mattias Rönnblom > <hofors@lysator.liu.se>, bruce.richardson@intel.com > Subject: [PATCH] eventdev: fix incorrect MP/MC tail updates in > rte_event_ring > X-Mailer: git-send-email 2.7.4 > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > SC/SP configuration. > > Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") > Cc: bruce.richardson@intel.com Bruce, As an original author of this file, Can you review this change? > > Signed-off-by: Mattias Rönnblom <hofors@lysator.liu.se> > > v2: > * Fixed issue with update_tail() parameter order. > --- > 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..827a320 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, r->r.prod.single, 1); > 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, r->r.cons.single, 0); > > end: > if (available != NULL) > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-04-04 0:52 ` Jerin Jacob @ 2018-04-04 8:52 ` Bruce Richardson 2018-04-04 9:46 ` Jerin Jacob 2018-04-04 10:36 ` Van Haaren, Harry 0 siblings, 2 replies; 9+ messages in thread From: Bruce Richardson @ 2018-04-04 8:52 UTC (permalink / raw) To: Jerin Jacob; +Cc: Mattias Rönnblom, dev On Wed, Apr 04, 2018 at 06:22:26AM +0530, Jerin Jacob wrote: > -----Original Message----- > > Date: Tue, 3 Apr 2018 10:19:03 +0200 > > From: Mattias Rönnblom <hofors@lysator.liu.se> > > To: dev@dpdk.org > > CC: jerin.jacob@caviumnetworks.com, Mattias Rönnblom > > <hofors@lysator.liu.se>, bruce.richardson@intel.com > > Subject: [PATCH] eventdev: fix incorrect MP/MC tail updates in > > rte_event_ring > > X-Mailer: git-send-email 2.7.4 > > > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > > SC/SP configuration. > > > > Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") > > Cc: bruce.richardson@intel.com > > Bruce, > > As an original author of this file, Can you review this change? > Yes, LTGM Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-04-04 8:52 ` Bruce Richardson @ 2018-04-04 9:46 ` Jerin Jacob 2018-04-04 10:36 ` Van Haaren, Harry 1 sibling, 0 replies; 9+ messages in thread From: Jerin Jacob @ 2018-04-04 9:46 UTC (permalink / raw) To: Bruce Richardson; +Cc: Mattias Rönnblom, dev -----Original Message----- > Date: Wed, 4 Apr 2018 09:52:18 +0100 > From: Bruce Richardson <bruce.richardson@intel.com> > To: Jerin Jacob <jerin.jacob@caviumnetworks.com> > CC: Mattias Rönnblom <hofors@lysator.liu.se>, dev@dpdk.org > Subject: Re: [PATCH] eventdev: fix incorrect MP/MC tail updates in > rte_event_ring > User-Agent: Mutt/1.9.4 (2018-02-28) > > On Wed, Apr 04, 2018 at 06:22:26AM +0530, Jerin Jacob wrote: > > -----Original Message----- > > > Date: Tue, 3 Apr 2018 10:19:03 +0200 > > > From: Mattias Rönnblom <hofors@lysator.liu.se> > > > To: dev@dpdk.org > > > CC: jerin.jacob@caviumnetworks.com, Mattias Rönnblom > > > <hofors@lysator.liu.se>, bruce.richardson@intel.com > > > Subject: [PATCH] eventdev: fix incorrect MP/MC tail updates in > > > rte_event_ring > > > X-Mailer: git-send-email 2.7.4 > > > > > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > > > SC/SP configuration. > > > > > > Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") > > > Cc: bruce.richardson@intel.com > > > > Bruce, > > > > As an original author of this file, Can you review this change? > > > > Yes, LTGM > > Acked-by: Bruce Richardson <bruce.richardson@intel.com> Cc: stable@dpdk.org Applied to dpdk-next-eventdev/master. Thanks. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring 2018-04-04 8:52 ` Bruce Richardson 2018-04-04 9:46 ` Jerin Jacob @ 2018-04-04 10:36 ` Van Haaren, Harry 1 sibling, 0 replies; 9+ messages in thread From: Van Haaren, Harry @ 2018-04-04 10:36 UTC (permalink / raw) To: Mattias Rönnblom; +Cc: dev, Richardson, Bruce, Jerin Jacob > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > Sent: Wednesday, April 4, 2018 9:52 AM > To: Jerin Jacob <jerin.jacob@caviumnetworks.com> > Cc: Mattias Rönnblom <hofors@lysator.liu.se>; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in > rte_event_ring > > On Wed, Apr 04, 2018 at 06:22:26AM +0530, Jerin Jacob wrote: > > -----Original Message----- > > > Date: Tue, 3 Apr 2018 10:19:03 +0200 > > > From: Mattias Rönnblom <hofors@lysator.liu.se> > > > To: dev@dpdk.org > > > CC: jerin.jacob@caviumnetworks.com, Mattias Rönnblom > > > <hofors@lysator.liu.se>, bruce.richardson@intel.com > > > Subject: [PATCH] eventdev: fix incorrect MP/MC tail updates in > > > rte_event_ring > > > X-Mailer: git-send-email 2.7.4 > > > > > > rte_event_ring enqueue and dequeue tail updates were hardcoded for a > > > SC/SP configuration. > > > > > > Fixes: dc39e2f359b5 ("eventdev: add ring structure for events") > > > Cc: bruce.richardson@intel.com Hi Mattias, I think this is a v2 of the patch here: http://dpdk.org/dev/patchwork/patch/36604/ When doing the git-send-email, we should pass the -v2 parameter, so the subject says [PATCH v2]. It is helpful to CC the people who replied to the V1 of the patch, so they are aware of the re-submit. Bruce has Acked this patch, so it can be merged now - above is just for future reference. Regards, -Harry <snip> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-04 10:36 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-28 9:16 [dpdk-dev] [PATCH] eventdev: fix incorrect MP/MC tail updates in rte_event_ring Mattias Rönnblom 2018-03-29 12:38 ` Van Haaren, Harry 2018-04-02 2:55 ` Jerin Jacob 2018-04-03 8:14 ` Mattias Rönnblom 2018-04-03 8:19 ` Mattias Rönnblom 2018-04-04 0:52 ` Jerin Jacob 2018-04-04 8:52 ` Bruce Richardson 2018-04-04 9:46 ` Jerin Jacob 2018-04-04 10:36 ` Van Haaren, Harry
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).