[v2] eal/windows: ensure all enabled CPUs are counted
Checks
Commit Message
From: Narcisa Vasile <navasile@microsoft.com>
rte_cpuset_t describes a set of CPUs by using an array of masks
named '_bits'. Each element in the '_bits' array represents
a bit mask, with each bit corresponding to a CPU.
The maximum number of CPUs is given by 'CPU_SETSIZE'.
The number of bit masks is computed using '_NUM_SETS(CPU_SETSIZE)'.
count_cpu() should count the number of CPUs enabled in the set 's'.
Currently, it iterates through the number of masks in the
set 's', instead of iterating through all the bits in all the masks.
For example, if '_NUM_SETS(CPU_SETSIZE)' returns 2,
which means there are 2 bit masks: _bits[0] and _bits[1],
count_cpu() would only check if CPUs '0' and '1' are enabled.
The correct behavior is to iterate through all the CPUs in the set and
count the ones that are enabled.
This patch fixes count_cpu() to ensure all the bits in all the masks
are checked to compute the correct number of CPUs enabled in 's'.
Fixes: e8428a9d89f1 ("eal/windows: add some basic functions and macros")
Cc: pallavi.kadam@intel.com
Cc: stable@dpdk.org
Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
---
v2:
* Fix commit message.
lib/eal/windows/include/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
2021-06-29 18:56 (UTC-0700), Narcisa Ana Maria Vasile:
> From: Narcisa Vasile <navasile@microsoft.com>
>
> rte_cpuset_t describes a set of CPUs by using an array of masks
> named '_bits'. Each element in the '_bits' array represents
> a bit mask, with each bit corresponding to a CPU.
> The maximum number of CPUs is given by 'CPU_SETSIZE'.
> The number of bit masks is computed using '_NUM_SETS(CPU_SETSIZE)'.
>
> count_cpu() should count the number of CPUs enabled in the set 's'.
> Currently, it iterates through the number of masks in the
> set 's', instead of iterating through all the bits in all the masks.
> For example, if '_NUM_SETS(CPU_SETSIZE)' returns 2,
> which means there are 2 bit masks: _bits[0] and _bits[1],
> count_cpu() would only check if CPUs '0' and '1' are enabled.
> The correct behavior is to iterate through all the CPUs in the set and
> count the ones that are enabled.
>
> This patch fixes count_cpu() to ensure all the bits in all the masks
> are checked to compute the correct number of CPUs enabled in 's'.
>
> Fixes: e8428a9d89f1 ("eal/windows: add some basic functions and macros")
> Cc: pallavi.kadam@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
> v2:
> * Fix commit message.
>
> 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;
Sorry, I lost track of this patch.
Great and thorough explanation, but I don't think it needs to be so long.
Suggestion:
On Windows, -l/--lcores EAL option was unable to process CPU sets
containing CPUs other than 0 and 1, because CPU_COUNT() macro
only checked these CPUs in the set. Fix CPU_COUNT() by enumerating
all possible CPU indices.
Whatever message is taken in the end,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
On 6/29/2021 6:56 PM, Narcisa Ana Maria Vasile wrote:
> From: Narcisa Vasile <navasile@microsoft.com>
>
> rte_cpuset_t describes a set of CPUs by using an array of masks
> named '_bits'. Each element in the '_bits' array represents
> a bit mask, with each bit corresponding to a CPU.
> The maximum number of CPUs is given by 'CPU_SETSIZE'.
> The number of bit masks is computed using '_NUM_SETS(CPU_SETSIZE)'.
>
> count_cpu() should count the number of CPUs enabled in the set 's'.
> Currently, it iterates through the number of masks in the
> set 's', instead of iterating through all the bits in all the masks.
> For example, if '_NUM_SETS(CPU_SETSIZE)' returns 2,
> which means there are 2 bit masks: _bits[0] and _bits[1],
> count_cpu() would only check if CPUs '0' and '1' are enabled.
> The correct behavior is to iterate through all the CPUs in the set and
> count the ones that are enabled.
>
> This patch fixes count_cpu() to ensure all the bits in all the masks
> are checked to compute the correct number of CPUs enabled in 's'.
>
> Fixes: e8428a9d89f1 ("eal/windows: add some basic functions and macros")
> Cc: pallavi.kadam@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Narcisa Vasile <navasile@microsoft.com>
> ---
Acked-by: Pallavi Kadam <pallavi.kadam@intel.com>
@@ -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;