diff mbox

[API-NEXT,v4,09/10] linux-generic: cpumask: add API odp_cpumask_available()

Message ID 1439278439-11386-10-git-send-email-hongbo.zhang@freescale.com
State New
Headers show

Commit Message

hongbo.zhang@freescale.com Aug. 11, 2015, 7:33 a.m. UTC 
From: Hongbo Zhang <hongbo.zhang@linaro.org>

In some cases, odp_cpu_count() isn't enough to show all the available
CPUs, so a new API odp_cpumask_available() is introduced, which returns
all the worker and control CPUs.

Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
---
 include/odp/api/cpumask.h            | 10 ++++++++++
 platform/linux-generic/odp_cpumask.c | 11 +++++++++++
 2 files changed, 21 insertions(+)

Comments

Savolainen, Petri (Nokia - FI/Espoo) Sept. 3, 2015, 2:02 p.m. UTC | #1 
> -----Original Message-----
> From: ext hongbo.zhang@freescale.com
> [mailto:hongbo.zhang@freescale.com]
> Sent: Tuesday, August 11, 2015 10:34 AM
> To: lng-odp@lists.linaro.org
> Cc: mike.holmes@linaro.org; stuart.haslam@arm.com; Savolainen, Petri
> (Nokia - FI/Espoo); petri.savolainen@linaro.org; Hongbo Zhang
> Subject: [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add API
> odp_cpumask_available()
> 
> From: Hongbo Zhang <hongbo.zhang@linaro.org>
> 
> In some cases, odp_cpu_count() isn't enough to show all the available
> CPUs, so a new API odp_cpumask_available() is introduced, which returns
> all the worker and control CPUs.
> 
> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> ---
>  include/odp/api/cpumask.h            | 10 ++++++++++
>  platform/linux-generic/odp_cpumask.c | 11 +++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
> index 2ad7fea..eef6404 100644
> --- a/include/odp/api/cpumask.h
> +++ b/include/odp/api/cpumask.h
> @@ -218,6 +218,16 @@ int odp_cpumask_def_worker(odp_cpumask_t *mask,
> int num);
>  int odp_cpumask_def_control(odp_cpumask_t *mask, int num);
> 
>  /**
> + * Report all the available CPUs
> + *
> + * All the available CPUs include both worker CPUs and control CPUs
> + *
> + * @param[out] mask    CPU mask to hold all available CPUs
> + * @return cpu number of all available CPUs
> + */
> +int odp_cpumask_available(odp_cpumask_t *mask);
> +
> +/**
>   * @}
>   */


This could be even more explicit

int odp_cpumask_all_available(odp_cpumask_t *mask);

Also in theory, there can be more CPUs available for ODP than the default (recommended) CPU masks return. E.g. dual threading could functionally work (every CPU in the mask), but ODP implementation recommends to run only one worker thread per physical CPU (every second CPU in the mask).


"All available CPUs include both default worker and control CPUs, and potentially additional CPUs"


-Petri
Bill Fischofer Sept. 4, 2015, 2:02 a.m. UTC | #2 
Perhaps odp_cpumask_max() might be a little less of a mouthful?

On Thu, Sep 3, 2015 at 9:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <
petri.savolainen@nokia.com> wrote:

>
>
> > -----Original Message-----
> > From: ext hongbo.zhang@freescale.com
> > [mailto:hongbo.zhang@freescale.com]
> > Sent: Tuesday, August 11, 2015 10:34 AM
> > To: lng-odp@lists.linaro.org
> > Cc: mike.holmes@linaro.org; stuart.haslam@arm.com; Savolainen, Petri
> > (Nokia - FI/Espoo); petri.savolainen@linaro.org; Hongbo Zhang
> > Subject: [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add API
> > odp_cpumask_available()
> >
> > From: Hongbo Zhang <hongbo.zhang@linaro.org>
> >
> > In some cases, odp_cpu_count() isn't enough to show all the available
> > CPUs, so a new API odp_cpumask_available() is introduced, which returns
> > all the worker and control CPUs.
> >
> > Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
> > ---
> >  include/odp/api/cpumask.h            | 10 ++++++++++
> >  platform/linux-generic/odp_cpumask.c | 11 +++++++++++
> >  2 files changed, 21 insertions(+)
> >
> > diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
> > index 2ad7fea..eef6404 100644
> > --- a/include/odp/api/cpumask.h
> > +++ b/include/odp/api/cpumask.h
> > @@ -218,6 +218,16 @@ int odp_cpumask_def_worker(odp_cpumask_t *mask,
> > int num);
> >  int odp_cpumask_def_control(odp_cpumask_t *mask, int num);
> >
> >  /**
> > + * Report all the available CPUs
> > + *
> > + * All the available CPUs include both worker CPUs and control CPUs
> > + *
> > + * @param[out] mask    CPU mask to hold all available CPUs
> > + * @return cpu number of all available CPUs
> > + */
> > +int odp_cpumask_available(odp_cpumask_t *mask);
> > +
> > +/**
> >   * @}
> >   */
>
>
> This could be even more explicit
>
> int odp_cpumask_all_available(odp_cpumask_t *mask);
>
> Also in theory, there can be more CPUs available for ODP than the default
> (recommended) CPU masks return. E.g. dual threading could functionally work
> (every CPU in the mask), but ODP implementation recommends to run only one
> worker thread per physical CPU (every second CPU in the mask).
>
>
> "All available CPUs include both default worker and control CPUs, and
> potentially additional CPUs"
>
>
> -Petri
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Savolainen, Petri (Nokia - FI/Espoo) Sept. 4, 2015, 7:44 a.m. UTC | #3 
Cpu mask API mainly manipulate the mask. So, odp_cpumask_max can be mixed to  return “max” or “last” cpu ID. Same problem with plain odp_cpumask_all – it’s too close to “set all cpu IDs in the mask”.

I think these “extra” functions which return system preferences (default workers, controllers, all available for ODP, all CPUs on the device (also those unavailable), …)  can be a bit longer and thus more descriptive.

-Petri


From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org]

Sent: Friday, September 04, 2015 5:03 AM
To: Savolainen, Petri (Nokia - FI/Espoo)
Cc: ext hongbo.zhang@freescale.com; lng-odp@lists.linaro.org; stuart.haslam@arm.com
Subject: Re: [lng-odp] [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add API odp_cpumask_available()

Perhaps odp_cpumask_max() might be a little less of a mouthful?

On Thu, Sep 3, 2015 at 9:02 AM, Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com<mailto:petri.savolainen@nokia.com>> wrote:


> -----Original Message-----

> From: ext hongbo.zhang@freescale.com<mailto:hongbo.zhang@freescale.com>

> [mailto:hongbo.zhang@freescale.com<mailto:hongbo.zhang@freescale.com>]

> Sent: Tuesday, August 11, 2015 10:34 AM

> To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>

> Cc: mike.holmes@linaro.org<mailto:mike.holmes@linaro.org>; stuart.haslam@arm.com<mailto:stuart.haslam@arm.com>; Savolainen, Petri

> (Nokia - FI/Espoo); petri.savolainen@linaro.org<mailto:petri.savolainen@linaro.org>; Hongbo Zhang

> Subject: [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add API

> odp_cpumask_available()

>

> From: Hongbo Zhang <hongbo.zhang@linaro.org<mailto:hongbo.zhang@linaro.org>>

>

> In some cases, odp_cpu_count() isn't enough to show all the available

> CPUs, so a new API odp_cpumask_available() is introduced, which returns

> all the worker and control CPUs.

>

> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org<mailto:hongbo.zhang@linaro.org>>

> ---

>  include/odp/api/cpumask.h            | 10 ++++++++++

>  platform/linux-generic/odp_cpumask.c | 11 +++++++++++

>  2 files changed, 21 insertions(+)

>

> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h

> index 2ad7fea..eef6404 100644

> --- a/include/odp/api/cpumask.h

> +++ b/include/odp/api/cpumask.h

> @@ -218,6 +218,16 @@ int odp_cpumask_def_worker(odp_cpumask_t *mask,

> int num);

>  int odp_cpumask_def_control(odp_cpumask_t *mask, int num);

>

>  /**

> + * Report all the available CPUs

> + *

> + * All the available CPUs include both worker CPUs and control CPUs

> + *

> + * @param[out] mask    CPU mask to hold all available CPUs

> + * @return cpu number of all available CPUs

> + */

> +int odp_cpumask_available(odp_cpumask_t *mask);

> +

> +/**

>   * @}

>   */


This could be even more explicit

int odp_cpumask_all_available(odp_cpumask_t *mask);

Also in theory, there can be more CPUs available for ODP than the default (recommended) CPU masks return. E.g. dual threading could functionally work (every CPU in the mask), but ODP implementation recommends to run only one worker thread per physical CPU (every second CPU in the mask).


"All available CPUs include both default worker and control CPUs, and potentially additional CPUs"


-Petri


_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>
https://lists.linaro.org/mailman/listinfo/lng-odp
Hongbo Zhang Sept. 8, 2015, 11:21 a.m. UTC | #4 
On 4 September 2015 at 15:44, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
> Cpu mask API mainly manipulate the mask. So, odp_cpumask_max can be mixed to
> return “max” or “last” cpu ID. Same problem with plain odp_cpumask_all –
> it’s too close to “set all cpu IDs in the mask”.
>
>
>
> I think these “extra” functions which return system preferences (default
> workers, controllers, all available for ODP, all CPUs on the device (also
> those unavailable), …)  can be a bit longer and thus more descriptive.
>
Yes "all available" seems more descriptive.

But let us think in another way, Stuart suggested me to introduce such
a function, further thought would be that this function can replace
the current odp_cpu_count(), I didn't take the further steps into this
patch set till now.

Currently the only user of this new API is the validation code, I use
this API to iterate each CPU, so is it important enough to introduce
such a new API?

>
>
> -Petri
>
>
>
>
>
> From: ext Bill Fischofer [mailto:bill.fischofer@linaro.org]
> Sent: Friday, September 04, 2015 5:03 AM
> To: Savolainen, Petri (Nokia - FI/Espoo)
> Cc: ext hongbo.zhang@freescale.com; lng-odp@lists.linaro.org;
> stuart.haslam@arm.com
> Subject: Re: [lng-odp] [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add
> API odp_cpumask_available()
>
>
>
> Perhaps odp_cpumask_max() might be a little less of a mouthful?
>
>
>
> On Thu, Sep 3, 2015 at 9:02 AM, Savolainen, Petri (Nokia - FI/Espoo)
> <petri.savolainen@nokia.com> wrote:
>
>
>
>> -----Original Message-----
>> From: ext hongbo.zhang@freescale.com
>> [mailto:hongbo.zhang@freescale.com]
>> Sent: Tuesday, August 11, 2015 10:34 AM
>> To: lng-odp@lists.linaro.org
>> Cc: mike.holmes@linaro.org; stuart.haslam@arm.com; Savolainen, Petri
>> (Nokia - FI/Espoo); petri.savolainen@linaro.org; Hongbo Zhang
>> Subject: [API-NEXT PATCH v4 09/10] linux-generic: cpumask: add API
>> odp_cpumask_available()
>>
>> From: Hongbo Zhang <hongbo.zhang@linaro.org>
>>
>> In some cases, odp_cpu_count() isn't enough to show all the available
>> CPUs, so a new API odp_cpumask_available() is introduced, which returns
>> all the worker and control CPUs.
>>
>> Signed-off-by: Hongbo Zhang <hongbo.zhang@linaro.org>
>> ---
>>  include/odp/api/cpumask.h            | 10 ++++++++++
>>  platform/linux-generic/odp_cpumask.c | 11 +++++++++++
>>  2 files changed, 21 insertions(+)
>>
>> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
>> index 2ad7fea..eef6404 100644
>> --- a/include/odp/api/cpumask.h
>> +++ b/include/odp/api/cpumask.h
>> @@ -218,6 +218,16 @@ int odp_cpumask_def_worker(odp_cpumask_t *mask,
>> int num);
>>  int odp_cpumask_def_control(odp_cpumask_t *mask, int num);
>>
>>  /**
>> + * Report all the available CPUs
>> + *
>> + * All the available CPUs include both worker CPUs and control CPUs
>> + *
>> + * @param[out] mask    CPU mask to hold all available CPUs
>> + * @return cpu number of all available CPUs
>> + */
>> +int odp_cpumask_available(odp_cpumask_t *mask);
>> +
>> +/**
>>   * @}
>>   */
>
> This could be even more explicit
>
> int odp_cpumask_all_available(odp_cpumask_t *mask);
>
> Also in theory, there can be more CPUs available for ODP than the default
> (recommended) CPU masks return. E.g. dual threading could functionally work
> (every CPU in the mask), but ODP implementation recommends to run only one
> worker thread per physical CPU (every second CPU in the mask).
>
>
> "All available CPUs include both default worker and control CPUs, and
> potentially additional CPUs"
>
>
> -Petri
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>
Savolainen, Petri (Nokia - FI/Espoo) Sept. 9, 2015, 12:20 p.m. UTC | #5 
> -----Original Message-----

> From: ext Hongbo Zhang [mailto:hongbo.zhang@linaro.org]

> Sent: Tuesday, September 08, 2015 2:21 PM

> To: Savolainen, Petri (Nokia - FI/Espoo)

> Cc: ext Bill Fischofer; ext hongbo.zhang@freescale.com;

> stuart.haslam@arm.com; lng-odp@lists.linaro.org

> Subject: Re: [lng-odp] [API-NEXT PATCH v4 09/10] linux-generic:

> cpumask: add API odp_cpumask_available()

> 

> On 4 September 2015 at 15:44, Savolainen, Petri (Nokia - FI/Espoo)

> <petri.savolainen@nokia.com> wrote:

> > Cpu mask API mainly manipulate the mask. So, odp_cpumask_max can be

> mixed to

> > return “max” or “last” cpu ID. Same problem with plain

> odp_cpumask_all –

> > it’s too close to “set all cpu IDs in the mask”.

> >

> >

> >

> > I think these “extra” functions which return system preferences

> (default

> > workers, controllers, all available for ODP, all CPUs on the device

> (also

> > those unavailable), …)  can be a bit longer and thus more

> descriptive.

> >

> Yes "all available" seems more descriptive.

> 

> But let us think in another way, Stuart suggested me to introduce such

> a function, further thought would be that this function can replace

> the current odp_cpu_count(), I didn't take the further steps into this

> patch set till now.

> 

> Currently the only user of this new API is the validation code, I use

> this API to iterate each CPU, so is it important enough to introduce

> such a new API?


It could replace cpu_count(). The mask and the count are related. User should use both when iterating through CPUs. A  count without the mask builds expectation that available CPU IDs are from 0 to count - 1, which may not be the case.

User needs cpu count/mask for e.g. allocating resources per CPU and iterating those.

-Petri
Hongbo Zhang Sept. 10, 2015, 6:45 a.m. UTC | #6 
On 9 September 2015 at 20:20, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolainen@nokia.com> wrote:
>
>
>> -----Original Message-----
>> From: ext Hongbo Zhang [mailto:hongbo.zhang@linaro.org]
>> Sent: Tuesday, September 08, 2015 2:21 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo)
>> Cc: ext Bill Fischofer; ext hongbo.zhang@freescale.com;
>> stuart.haslam@arm.com; lng-odp@lists.linaro.org
>> Subject: Re: [lng-odp] [API-NEXT PATCH v4 09/10] linux-generic:
>> cpumask: add API odp_cpumask_available()
>>
>> On 4 September 2015 at 15:44, Savolainen, Petri (Nokia - FI/Espoo)
>> <petri.savolainen@nokia.com> wrote:
>> > Cpu mask API mainly manipulate the mask. So, odp_cpumask_max can be
>> mixed to
>> > return “max” or “last” cpu ID. Same problem with plain
>> odp_cpumask_all –
>> > it’s too close to “set all cpu IDs in the mask”.
>> >
>> >
>> >
>> > I think these “extra” functions which return system preferences
>> (default
>> > workers, controllers, all available for ODP, all CPUs on the device
>> (also
>> > those unavailable), …)  can be a bit longer and thus more
>> descriptive.
>> >
>> Yes "all available" seems more descriptive.
>>
>> But let us think in another way, Stuart suggested me to introduce such
>> a function, further thought would be that this function can replace
>> the current odp_cpu_count(), I didn't take the further steps into this
>> patch set till now.
>>
>> Currently the only user of this new API is the validation code, I use
>> this API to iterate each CPU, so is it important enough to introduce
>> such a new API?
>
> It could replace cpu_count(). The mask and the count are related. User should use both when iterating through CPUs. A  count without the mask builds expectation that available CPU IDs are from 0 to count - 1, which may not be the case.
>
Oh, yes, preparing the v5 patch according to the comments.

> User needs cpu count/mask for e.g. allocating resources per CPU and iterating those.
>
> -Petri
>
>
diff mbox

Patch

diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h
index 2ad7fea..eef6404 100644
--- a/include/odp/api/cpumask.h
+++ b/include/odp/api/cpumask.h
@@ -218,6 +218,16 @@  int odp_cpumask_def_worker(odp_cpumask_t *mask, int num);
 int odp_cpumask_def_control(odp_cpumask_t *mask, int num);
 
 /**
+ * Report all the available CPUs
+ *
+ * All the available CPUs include both worker CPUs and control CPUs
+ *
+ * @param[out] mask    CPU mask to hold all available CPUs
+ * @return cpu number of all available CPUs
+ */
+int odp_cpumask_available(odp_cpumask_t *mask);
+
+/**
  * @}
  */
 
diff --git a/platform/linux-generic/odp_cpumask.c b/platform/linux-generic/odp_cpumask.c
index c28153b..bb37cb8 100644
--- a/platform/linux-generic/odp_cpumask.c
+++ b/platform/linux-generic/odp_cpumask.c
@@ -243,3 +243,14 @@  int odp_cpumask_def_control(odp_cpumask_t *mask, int num ODP_UNUSED)
 	odp_cpumask_set(mask, 0);
 	return 1;
 }
+
+int odp_cpumask_available(odp_cpumask_t *mask)
+{
+	odp_cpumask_t mask_work, mask_ctrl;
+
+	odp_cpumask_def_worker(&mask_work, 0);
+	odp_cpumask_def_control(&mask_ctrl, 0);
+	odp_cpumask_or(mask, &mask_work, &mask_ctrl);
+
+	return odp_cpumask_count(mask);
+}