* [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked
@ 2021-06-25 0:27 Narcisa Ana Maria Vasile
2021-06-25 8:36 ` Dmitry Kozlyuk
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
0 siblings, 2 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2021-06-25 0:27 UTC (permalink / raw)
To: dev, thomas, dmitry.kozliuk, ocardona, pallavi.kadam, talshn, dmitrym
Cc: Narcisa 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;
--
2.31.0.vfs.0.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked
2021-06-25 0:27 [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked Narcisa Ana Maria Vasile
@ 2021-06-25 8:36 ` Dmitry Kozlyuk
2021-06-30 2:02 ` Narcisa Ana Maria Vasile
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
1 sibling, 1 reply; 9+ messages in thread
From: Dmitry Kozlyuk @ 2021-06-25 8:36 UTC (permalink / raw)
To: Narcisa Ana Maria Vasile
Cc: dev, thomas, ocardona, pallavi.kadam, talshn, dmitrym, Narcisa Vasile
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted
2021-06-25 0:27 [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked Narcisa Ana Maria Vasile
2021-06-25 8:36 ` Dmitry Kozlyuk
@ 2021-06-30 1:56 ` Narcisa Ana Maria Vasile
2021-08-04 14:56 ` Dmitry Kozlyuk
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2021-06-30 1:56 UTC (permalink / raw)
To: dev, thomas, dmitry.kozliuk, ocardona, pallavi.kadam, talshn, dmitrym
Cc: Narcisa Vasile, stable
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;
--
2.31.0.vfs.0.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked
2021-06-25 8:36 ` Dmitry Kozlyuk
@ 2021-06-30 2:02 ` Narcisa Ana Maria Vasile
0 siblings, 0 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2021-06-30 2:02 UTC (permalink / raw)
To: Dmitry Kozlyuk
Cc: dev, thomas, ocardona, pallavi.kadam, talshn, dmitrym, Narcisa Vasile
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!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
@ 2021-08-04 14:56 ` Dmitry Kozlyuk
2021-08-04 20:40 ` Kadam, Pallavi
2021-08-18 13:46 ` [dpdk-dev] [PATCH v3] " Narcisa Ana Maria Vasile
2 siblings, 0 replies; 9+ messages in thread
From: Dmitry Kozlyuk @ 2021-08-04 14:56 UTC (permalink / raw)
To: Narcisa Ana Maria Vasile, talshn
Cc: dev, thomas, ocardona, pallavi.kadam, dmitrym, Narcisa Vasile, stable
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
2021-08-04 14:56 ` Dmitry Kozlyuk
@ 2021-08-04 20:40 ` Kadam, Pallavi
2021-08-18 13:46 ` [dpdk-dev] [PATCH v3] " Narcisa Ana Maria Vasile
2 siblings, 0 replies; 9+ messages in thread
From: Kadam, Pallavi @ 2021-08-04 20:40 UTC (permalink / raw)
To: Narcisa Ana Maria Vasile, dev, thomas, dmitry.kozliuk, ocardona,
talshn, dmitrym
Cc: Narcisa Vasile, stable
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>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [dpdk-dev] [PATCH v3] eal/windows: ensure all enabled CPUs are counted
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
2021-08-04 14:56 ` Dmitry Kozlyuk
2021-08-04 20:40 ` Kadam, Pallavi
@ 2021-08-18 13:46 ` Narcisa Ana Maria Vasile
2021-10-08 23:56 ` Narcisa Ana Maria Vasile
2021-10-11 18:48 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2 siblings, 2 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2021-08-18 13:46 UTC (permalink / raw)
To: dev, thomas, dmitry.kozliuk, khot, navasile, dmitrym, roretzla,
talshn, ocardona
Cc: bruce.richardson, david.marchand, pallavi.kadam, stable
From: Narcisa Vasile <navasile@microsoft.com>
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.
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: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Pallavi Kadam <pallavi.kadam@intel.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;
--
2.31.0.vfs.0.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [PATCH v3] eal/windows: ensure all enabled CPUs are counted
2021-08-18 13:46 ` [dpdk-dev] [PATCH v3] " Narcisa Ana Maria Vasile
@ 2021-10-08 23:56 ` Narcisa Ana Maria Vasile
2021-10-11 18:48 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
1 sibling, 0 replies; 9+ messages in thread
From: Narcisa Ana Maria Vasile @ 2021-10-08 23:56 UTC (permalink / raw)
To: dev, thomas, dmitry.kozliuk, khot, navasile, dmitrym, roretzla,
talshn, ocardona
Cc: bruce.richardson, david.marchand, pallavi.kadam, stable
On Wed, Aug 18, 2021 at 06:46:00AM -0700, Narcisa Ana Maria Vasile wrote:
> From: Narcisa Vasile <navasile@microsoft.com>
>
> 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.
>
> 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: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Pallavi Kadam <pallavi.kadam@intel.com>
> ---
Can this be merged? This small fix will solve the issues related to threads
affinity on Windows. Thanks!
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH v3] eal/windows: ensure all enabled CPUs are counted
2021-08-18 13:46 ` [dpdk-dev] [PATCH v3] " Narcisa Ana Maria Vasile
2021-10-08 23:56 ` Narcisa Ana Maria Vasile
@ 2021-10-11 18:48 ` Thomas Monjalon
1 sibling, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2021-10-11 18:48 UTC (permalink / raw)
To: navasile
Cc: dev, dmitry.kozliuk, khot, dmitrym, roretzla, talshn, ocardona,
stable, bruce.richardson, david.marchand, pallavi.kadam, stable
18/08/2021 15:46, Narcisa Ana Maria Vasile:
> From: Narcisa Vasile <navasile@microsoft.com>
>
> 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.
>
> 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: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Pallavi Kadam <pallavi.kadam@intel.com>
Applied, thanks.
Sorry for the delay.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-10-11 18:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25 0:27 [dpdk-dev] [PATCH] eal/windows: ensure all the CPUs in the set are checked Narcisa Ana Maria Vasile
2021-06-25 8:36 ` Dmitry Kozlyuk
2021-06-30 2:02 ` Narcisa Ana Maria Vasile
2021-06-30 1:56 ` [dpdk-dev] [PATCH v2] eal/windows: ensure all enabled CPUs are counted Narcisa Ana Maria Vasile
2021-08-04 14:56 ` Dmitry Kozlyuk
2021-08-04 20:40 ` Kadam, Pallavi
2021-08-18 13:46 ` [dpdk-dev] [PATCH v3] " Narcisa Ana Maria Vasile
2021-10-08 23:56 ` Narcisa Ana Maria Vasile
2021-10-11 18:48 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
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).