Thanks, fixed both issues in v3.

- Petri


From: ext Christophe Milard [mailto:[email protected]]
Sent: Tuesday, June 09, 2015 9:30 AM
To: Savolainen, Petri (Nokia - FI/Espoo)
Cc: LNG ODP Mailman List
Subject: Re: [lng-odp] [API-NEXT PATCH v2 1/3] api: cpumask: added 
cpumask_setall



On 5 June 2015 at 12:48, Petri Savolainen 
<[email protected]<mailto:[email protected]>> wrote:
The call sets all possible CPUs in the mask. It's system specific which
CPUs are actually available to the application.

Signed-off-by: Petri Savolainen 
<[email protected]<mailto:[email protected]>>
---
 include/odp/api/cpumask.h            | 15 +++++++++++++--
 platform/linux-generic/odp_cpumask.c |  8 ++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
index dbac7b9..217bb5b 100644
--- a/include/odp/api/cpumask.h
+++ b/include/odp/api/cpumask.h
@@ -65,6 +65,17 @@ void odp_cpumask_zero(odp_cpumask_t *mask);
 void odp_cpumask_set(odp_cpumask_t *mask, int cpu);

 /**
+ * Set all CPUs in mask
+ *
+ * Set all possible CPUs in the mask. All CPUs from 0 to odp_cpumask_count()
+ * minus one are set. It's system specific which CPUs are actually available
+ * to the application.

I am not sure what you mean by " It's system specific which CPUs are actually 
available to the application." and why this sentence is here. I would have 
expected a comment like "Set all possible CPUs in the mask. All CPUs from 0 to 
odp_cpumask_count() minus one are set, regardless of which CPUs are actually 
available to the application."

+ *
+ * @param mask  CPU mask to set
+ */
+void odp_cpumask_setall(odp_cpumask_t *mask);
+
+/**
  * Remove CPU from mask
  * @param mask  CPU mask to update
  * @param cpu   CPU number
@@ -82,7 +93,7 @@ void odp_cpumask_clr(odp_cpumask_t *mask, int cpu);
 int odp_cpumask_isset(const odp_cpumask_t *mask, int cpu);

 /**
- * Count number of CPU's in mask
+ * Count number of CPUs set in mask

Yes!

  *
  * @param mask  CPU mask
  * @return population count
@@ -120,7 +131,7 @@ void odp_cpumask_xor(odp_cpumask_t *dest, const 
odp_cpumask_t *src1,
                     const odp_cpumask_t *src2);

 /**
- * Test if two CPU masks contain the same CPU's
+ * Test if two CPU masks contain the same CPUs
  *
  * @param mask1    CPU mask 1
  * @param mask2    CPU mask 2
diff --git a/platform/linux-generic/odp_cpumask.c 
b/platform/linux-generic/odp_cpumask.c
index 0ca1071..a27e80c 100644
--- a/platform/linux-generic/odp_cpumask.c
+++ b/platform/linux-generic/odp_cpumask.c
@@ -125,6 +125,14 @@ void odp_cpumask_set(odp_cpumask_t *mask, int cpu)
        CPU_SET(cpu, &mask->set);
 }

+void odp_cpumask_setall(odp_cpumask_t *mask)
+{
+       int cpu;
+
+       for (cpu = 0; cpu < CPU_SETSIZE - 1; cpu++)

Why  cpu < CPU_SETSIZE - 1 ?
Shouldn't it be (cpu < CPU_SETSIZE) ? why not the last one?
Sorry for the delay of this review.
Christophe

+               CPU_SET(cpu, &mask->set);
+}
+
 void odp_cpumask_clr(odp_cpumask_t *mask, int cpu)
 {
        CPU_CLR(cpu, &mask->set);
--
2.4.2

_______________________________________________
lng-odp mailing list
[email protected]<mailto:[email protected]>
https://lists.linaro.org/mailman/listinfo/lng-odp

_______________________________________________
lng-odp mailing list
[email protected]
https://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to