DPDK patches and discussions
 help / color / mirror / Atom feed
* [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).