diff mbox series

lib/ring: fix the bug of HTS/RTS bulk enqueue

Message ID 20200610033918.45834-1-feifei.wang2@arm.com (mailing list archive)
State Superseded, archived
Headers
Series lib/ring: fix the bug of HTS/RTS bulk enqueue |

Checks

Context Check Description
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Feifei Wang June 10, 2020, 3:39 a.m. UTC 
  Directly call "_rte_ring_do_enqueue_elem" function can result in the
process skips the "switch" order and cannot call the APIs of RTS and HTS
mode in "rte_ring_enqueue_bulk_elem" function.

To fix it, remove the unnecessary function and make
"_rte_ring_do_enqueue_elem" consistent with "_rte_ring_do_dequeue_elem".

Fixes: e6ba4731c0f3 ("ring: introduce RTS ring mode")
Cc: stable@dpdk.org

Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 lib/librte_ring/rte_ring_elem.h | 3 ---
 1 file changed, 3 deletions(-)

Comments

Honnappa Nagarahalli June 10, 2020, 3:56 a.m. UTC | #1 
Hi Feifei,
	Thanks for the patch. Few nits inline.

> -----Original Message-----
> From: Feifei Wang <feifei.wang2@arm.com>
> Sent: Tuesday, June 9, 2020 10:39 PM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> Ananyev <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>; stable@dpdk.org
> Subject: [PATCH] lib/ring: fix the bug of HTS/RTS bulk enqueue
> 
> Directly call "_rte_ring_do_enqueue_elem" function can result in the process
> skips the "switch" order and cannot call the APIs of RTS and HTS mode in
> "rte_ring_enqueue_bulk_elem" function.
may be " lib/ring: fix bulk enqueue API to allow HTS/RTS modes"?

> 
> To fix it, remove the unnecessary function and make
> "_rte_ring_do_enqueue_elem" consistent with
> "_rte_ring_do_dequeue_elem".
How about something like:
Remove the unwanted call to "_rte_ring_do_enqueue_elem" to allow for correct handling of RTS/HTS modes.

> 
> Fixes: e6ba4731c0f3 ("ring: introduce RTS ring mode")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/librte_ring/rte_ring_elem.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/lib/librte_ring/rte_ring_elem.h b/lib/librte_ring/rte_ring_elem.h
> index a5a4c46f9..469aee7da 100644
> --- a/lib/librte_ring/rte_ring_elem.h
> +++ b/lib/librte_ring/rte_ring_elem.h
> @@ -578,9 +578,6 @@ static __rte_always_inline unsigned int
> rte_ring_enqueue_bulk_elem(struct rte_ring *r, const void *obj_table,
>  		unsigned int esize, unsigned int n, unsigned int *free_space)  {
> -	return __rte_ring_do_enqueue_elem(r, obj_table, esize, n,
> -			RTE_RING_QUEUE_FIXED, r->prod.sync_type,
> free_space);
> -
>  	switch (r->prod.sync_type) {
>  	case RTE_RING_SYNC_MT:
>  		return rte_ring_mp_enqueue_bulk_elem(r, obj_table, esize, n,
Otherwise,
Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

> --
> 2.17.1
Feifei Wang June 10, 2020, 6:56 a.m. UTC | #2 
> -----邮件原件-----
> 发件人: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> 发送时间: 2020年6月10日 11:57
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; Konstantin Ananyev
> <konstantin.ananyev@intel.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>; stable@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> 主题: RE: [PATCH] lib/ring: fix the bug of HTS/RTS bulk enqueue
>
> Hi Feifei,
> Thanks for the patch. Few nits inline.
>
> > -----Original Message-----
> > From: Feifei Wang <feifei.wang2@arm.com>
> > Sent: Tuesday, June 9, 2020 10:39 PM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Konstantin
> > Ananyev <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; nd <nd@arm.com>; Feifei Wang
> <Feifei.Wang2@arm.com>;
> > stable@dpdk.org
> > Subject: [PATCH] lib/ring: fix the bug of HTS/RTS bulk enqueue
> >
> > Directly call "_rte_ring_do_enqueue_elem" function can result in the
> > process skips the "switch" order and cannot call the APIs of RTS and
> > HTS mode in "rte_ring_enqueue_bulk_elem" function.
> may be " lib/ring: fix bulk enqueue API to allow HTS/RTS modes"?
>
> >
> > To fix it, remove the unnecessary function and make
> > "_rte_ring_do_enqueue_elem" consistent with
> > "_rte_ring_do_dequeue_elem".
> How about something like:
> Remove the unwanted call to "_rte_ring_do_enqueue_elem" to allow for
> correct handling of RTS/HTS modes.
>
That's all right. This is more clearly.
> >
> > Fixes: e6ba4731c0f3 ("ring: introduce RTS ring mode")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/librte_ring/rte_ring_elem.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/lib/librte_ring/rte_ring_elem.h
> > b/lib/librte_ring/rte_ring_elem.h index a5a4c46f9..469aee7da 100644
> > --- a/lib/librte_ring/rte_ring_elem.h
> > +++ b/lib/librte_ring/rte_ring_elem.h
> > @@ -578,9 +578,6 @@ static __rte_always_inline unsigned int
> > rte_ring_enqueue_bulk_elem(struct rte_ring *r, const void *obj_table,
> >  unsigned int esize, unsigned int n, unsigned int *free_space)
> {
> > -return __rte_ring_do_enqueue_elem(r, obj_table, esize, n,
> > -RTE_RING_QUEUE_FIXED, r->prod.sync_type,
> > free_space);
> > -
> >  switch (r->prod.sync_type) {
> >  case RTE_RING_SYNC_MT:
> >  return rte_ring_mp_enqueue_bulk_elem(r, obj_table, esize,
> n,
> Otherwise,
> Acked-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>
> > --
> > 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Ananyev, Konstantin June 10, 2020, 10:17 a.m. UTC | #3 
> Directly call "_rte_ring_do_enqueue_elem" function can result in the
> process skips the "switch" order and cannot call the APIs of RTS and HTS
> mode in "rte_ring_enqueue_bulk_elem" function.
> 
> To fix it, remove the unnecessary function and make
> "_rte_ring_do_enqueue_elem" consistent with "_rte_ring_do_dequeue_elem".
> 
> Fixes: e6ba4731c0f3 ("ring: introduce RTS ring mode")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  lib/librte_ring/rte_ring_elem.h | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/lib/librte_ring/rte_ring_elem.h b/lib/librte_ring/rte_ring_elem.h
> index a5a4c46f9..469aee7da 100644
> --- a/lib/librte_ring/rte_ring_elem.h
> +++ b/lib/librte_ring/rte_ring_elem.h
> @@ -578,9 +578,6 @@ static __rte_always_inline unsigned int
>  rte_ring_enqueue_bulk_elem(struct rte_ring *r, const void *obj_table,
>  		unsigned int esize, unsigned int n, unsigned int *free_space)
>  {
> -	return __rte_ring_do_enqueue_elem(r, obj_table, esize, n,
> -			RTE_RING_QUEUE_FIXED, r->prod.sync_type, free_space);
> -
>  	switch (r->prod.sync_type) {
>  	case RTE_RING_SYNC_MT:
>  		return rte_ring_mp_enqueue_bulk_elem(r, obj_table, esize, n,
> --

Ouch, that was a nasty one from me.
Thanks for catching and fixing.
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> 2.17.1
Feifei Wang June 12, 2020, 5:02 a.m. UTC | #4 
> -----邮件原件-----
> 发件人: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> 发送时间: 2020年6月10日 18:17
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; stable@dpdk.org
> 主题: RE: [PATCH] lib/ring: fix the bug of HTS/RTS bulk enqueue
>
>
> > Directly call "_rte_ring_do_enqueue_elem" function can result in the
> > process skips the "switch" order and cannot call the APIs of RTS and
> > HTS mode in "rte_ring_enqueue_bulk_elem" function.
> >
> > To fix it, remove the unnecessary function and make
> > "_rte_ring_do_enqueue_elem" consistent with
> "_rte_ring_do_dequeue_elem".
> >
> > Fixes: e6ba4731c0f3 ("ring: introduce RTS ring mode")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/librte_ring/rte_ring_elem.h | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/lib/librte_ring/rte_ring_elem.h
> > b/lib/librte_ring/rte_ring_elem.h index a5a4c46f9..469aee7da 100644
> > --- a/lib/librte_ring/rte_ring_elem.h
> > +++ b/lib/librte_ring/rte_ring_elem.h
> > @@ -578,9 +578,6 @@ static __rte_always_inline unsigned int
> > rte_ring_enqueue_bulk_elem(struct rte_ring *r, const void *obj_table,
> >  unsigned int esize, unsigned int n, unsigned int *free_space)
> {
> > -return __rte_ring_do_enqueue_elem(r, obj_table, esize, n,
> > -RTE_RING_QUEUE_FIXED, r->prod.sync_type,
> free_space);
> > -
> >  switch (r->prod.sync_type) {
> >  case RTE_RING_SYNC_MT:
> >  return rte_ring_mp_enqueue_bulk_elem(r, obj_table, esize,
> n,
> > --
>
> Ouch, that was a nasty one from me.
> Thanks for catching and fixing.
That's all right. It is admirable to integrate RTS mode. And I think I need a lot of things to learn from you.
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
> > 2.17.1

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
diff mbox series

Patch

diff --git a/lib/librte_ring/rte_ring_elem.h b/lib/librte_ring/rte_ring_elem.h
index a5a4c46f9..469aee7da 100644
--- a/lib/librte_ring/rte_ring_elem.h
+++ b/lib/librte_ring/rte_ring_elem.h
@@ -578,9 +578,6 @@  static __rte_always_inline unsigned int
 rte_ring_enqueue_bulk_elem(struct rte_ring *r, const void *obj_table,
 		unsigned int esize, unsigned int n, unsigned int *free_space)
 {
-	return __rte_ring_do_enqueue_elem(r, obj_table, esize, n,
-			RTE_RING_QUEUE_FIXED, r->prod.sync_type, free_space);
-
 	switch (r->prod.sync_type) {
 	case RTE_RING_SYNC_MT:
 		return rte_ring_mp_enqueue_bulk_elem(r, obj_table, esize, n,