diff mbox series

[v2,1/4] hash: fix unnecessary pause

Message ID 1540404570-102126-2-git-send-email-yipeng1.wang@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series hash: improve multiple places |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wang, Yipeng1 Oct. 24, 2018, 6:09 p.m. UTC 
  There is a rte_pause in hash table reset function.
Since the loop is not a polling loop on shared
data structure, the rte_pause is not needed.

Fixes: b26473ff8f4a ("hash: add reset function")
Cc: stable@dpdk.org

Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
---
 lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Mattias Rönnblom Oct. 25, 2018, 6:31 a.m. UTC | #1 
On 2018-10-24 20:09, Yipeng Wang wrote:
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared
> data structure, the rte_pause is not needed.
> 

I'm guessing the <rte_pause.h> include is longer needed.
Bruce Richardson Oct. 25, 2018, 9:30 a.m. UTC | #2 
On Thu, Oct 25, 2018 at 08:31:16AM +0200, Mattias Rönnblom wrote:
> On 2018-10-24 20:09, Yipeng Wang wrote:
> > There is a rte_pause in hash table reset function.
> > Since the loop is not a polling loop on shared
> > data structure, the rte_pause is not needed.
> > 
> 
> I'm guessing the <rte_pause.h> include is longer needed.

Good point, otherwise:

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Honnappa Nagarahalli Oct. 26, 2018, 12:24 a.m. UTC | #3 
> 
> There is a rte_pause in hash table reset function.
> Since the loop is not a polling loop on shared data structure, the rte_pause
> is not needed.
> 
> Fixes: b26473ff8f4a ("hash: add reset function")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yipeng Wang <yipeng1.wang@intel.com>
> ---
>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> b/lib/librte_hash/rte_cuckoo_hash.c
> index 0648a22..4a2647e 100644
> --- a/lib/librte_hash/rte_cuckoo_hash.c
> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> 
>  	/* clear the free ring */
>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> -		rte_pause();
> +		continue;
Minor comment: 'continue' can be removed.

> 
>  	/* clear free extendable bucket ring and memory */
>  	if (h->ext_table_support) {
>  		memset(h->buckets_ext, 0, h->num_buckets *
>  						sizeof(struct
> rte_hash_bucket));
>  		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
> -			rte_pause();
> +			continue;
>  	}
> 
>  	/* Repopulate the free slots ring. Entry zero is reserved for key
> misses */
> --
> 2.7.4
Wang, Yipeng1 Oct. 26, 2018, 2:04 a.m. UTC | #4 
>-----Original Message-----
>From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
>> ---
>>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
>> b/lib/librte_hash/rte_cuckoo_hash.c
>> index 0648a22..4a2647e 100644
>> --- a/lib/librte_hash/rte_cuckoo_hash.c
>> +++ b/lib/librte_hash/rte_cuckoo_hash.c
>> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
>>
>>  	/* clear the free ring */
>>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
>> -		rte_pause();
>> +		continue;
>Minor comment: 'continue' can be removed.
>
[Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
Anyone know a common way to do it in DPDK?
Bruce Richardson Oct. 26, 2018, 11:09 a.m. UTC | #5 
On Fri, Oct 26, 2018 at 03:04:12AM +0100, Wang, Yipeng1 wrote:
> >-----Original Message-----
> >From: Honnappa Nagarahalli [mailto:Honnappa.Nagarahalli@arm.com]
> >> ---
> >>  lib/librte_hash/rte_cuckoo_hash.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/lib/librte_hash/rte_cuckoo_hash.c
> >> b/lib/librte_hash/rte_cuckoo_hash.c
> >> index 0648a22..4a2647e 100644
> >> --- a/lib/librte_hash/rte_cuckoo_hash.c
> >> +++ b/lib/librte_hash/rte_cuckoo_hash.c
> >> @@ -574,14 +574,14 @@ rte_hash_reset(struct rte_hash *h)
> >>
> >>  	/* clear the free ring */
> >>  	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
> >> -		rte_pause();
> >> +		continue;
> >Minor comment: 'continue' can be removed.
> >
> [Wang, Yipeng]  I did not find a pretty way to do it without coding style warning, e.g. add semicolon in same line...
> Anyone know a common way to do it in DPDK?
> 
Semi-colon on a new line, possibly with comment should work ok. However,
the continue is harmless, so I'm ok with it as-is.
diff mbox series

Patch

diff --git a/lib/librte_hash/rte_cuckoo_hash.c b/lib/librte_hash/rte_cuckoo_hash.c
index 0648a22..4a2647e 100644
--- a/lib/librte_hash/rte_cuckoo_hash.c
+++ b/lib/librte_hash/rte_cuckoo_hash.c
@@ -574,14 +574,14 @@  rte_hash_reset(struct rte_hash *h)
 
 	/* clear the free ring */
 	while (rte_ring_dequeue(h->free_slots, &ptr) == 0)
-		rte_pause();
+		continue;
 
 	/* clear free extendable bucket ring and memory */
 	if (h->ext_table_support) {
 		memset(h->buckets_ext, 0, h->num_buckets *
 						sizeof(struct rte_hash_bucket));
 		while (rte_ring_dequeue(h->free_ext_bkts, &ptr) == 0)
-			rte_pause();
+			continue;
 	}
 
 	/* Repopulate the free slots ring. Entry zero is reserved for key misses */