eal/windows: ensure all the CPUs in the set are checked

Message ID 1624580843-4521-1-git-send-email-navasile@linux.microsoft.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series eal/windows: ensure all the CPUs in the set are checked |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Functional success Functional Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/github-robot success github build: passed

Commit Message

Narcisa Ana Maria Vasile June 25, 2021, 12:27 a.m. UTC
  From: Narcisa Vasile <navasile@microsoft.com>

Fix count_cpu() to ensure it iterates through all the CPUs in a set.
count_cpu() iterates through the CPUs in the set 's' and counts the
selected ones.

Previously, it was incorrectly using the number of CPUSETS to iterate
through the CPUs.

Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
 lib/eal/windows/include/sched.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Dmitry Kozlyuk June 25, 2021, 8:36 a.m. UTC | #1
2021-06-24 17:27 (UTC-0700), Narcisa Ana Maria Vasile:
> From: Narcisa Vasile <navasile@microsoft.com>
> 
> Fix count_cpu() to ensure it iterates through all the CPUs in a set.
> count_cpu() iterates through the CPUs in the set 's' and counts the
> selected ones.
> 
> Previously, it was incorrectly using the number of CPUSETS to iterate
> through the CPUs.
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
>  lib/eal/windows/include/sched.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
> index ff572b5dcb..bc31cc8465 100644
> --- a/lib/eal/windows/include/sched.h
> +++ b/lib/eal/windows/include/sched.h
> @@ -49,7 +49,7 @@ count_cpu(rte_cpuset_t *s)
>  	unsigned int _i;
>  	int count = 0;
>  
> -	for (_i = 0; _i < _NUM_SETS(CPU_SETSIZE); _i++)
> +	for (_i = 0; _i < CPU_SETSIZE; _i++)
>  		if (CPU_ISSET(_i, s) != 0LL)
>  			count++;
>  	return count;

Hi Naty,

Thank you for the fix, but we also need a proper commit message:

https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body

Specifically, please, describe what was the observable issue (usually first
comes what was wrong, then how it is fixed now) and add "Fixes" tag and Cc.
Also, "number of CPUSETS" sounds unclear, as there's no "CPUSET".
Suggestion: "number of bitset limbs" or maybe if you describe what was
wrong with the result you won't need to describe its reason precisely at all.
  
Narcisa Ana Maria Vasile June 30, 2021, 2:02 a.m. UTC | #2
On Fri, Jun 25, 2021 at 11:36:21AM +0300, Dmitry Kozlyuk wrote:
> 2021-06-24 17:27 (UTC-0700), Narcisa Ana Maria Vasile:
> > From: Narcisa Vasile <navasile@microsoft.com>
> > 
> > Fix count_cpu() to ensure it iterates through all the CPUs in a set.
> > count_cpu() iterates through the CPUs in the set 's' and counts the
> > selected ones.
> > 
> > Previously, it was incorrectly using the number of CPUSETS to iterate
> > through the CPUs.
> >
> > Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> > ---
> >  lib/eal/windows/include/sched.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Hi Naty,
> 
> Thank you for the fix, but we also need a proper commit message:
> 
> https://doc.dpdk.org/guides/contributing/patches.html#commit-messages-body
> 
> Specifically, please, describe what was the observable issue (usually first
> comes what was wrong, then how it is fixed now) and add "Fixes" tag and Cc.
> Also, "number of CPUSETS" sounds unclear, as there's no "CPUSET".
> Suggestion: "number of bitset limbs" or maybe if you describe what was
> wrong with the result you won't need to describe its reason precisely at all.

Ah, I've mixed some terminology here..
Thank you Dmitry for the feedback!
  

Patch

diff --git a/lib/eal/windows/include/sched.h b/lib/eal/windows/include/sched.h
index ff572b5dcb..bc31cc8465 100644
--- a/lib/eal/windows/include/sched.h
+++ b/lib/eal/windows/include/sched.h
@@ -49,7 +49,7 @@  count_cpu(rte_cpuset_t *s)
 	unsigned int _i;
 	int count = 0;
 
-	for (_i = 0; _i < _NUM_SETS(CPU_SETSIZE); _i++)
+	for (_i = 0; _i < CPU_SETSIZE; _i++)
 		if (CPU_ISSET(_i, s) != 0LL)
 			count++;
 	return count;