| Message ID | 1446042764-12311-1-git-send-email-anders.roxell@linaro.org |
|---|---|
| State | Accepted |
| Commit | d6d3dea2463c9d33765023197f96a0be4b01df94 |
| Headers | show |
On 10/28/2015 17:32, Anders Roxell wrote: > Applications that use an installed ODP will get the following > compile error: > .../odp_install/include/odp/plat/cpumask_types.h:26:33: fatal error: > odp_config_internal.h: No such file or directory > #include <odp_config_internal.h> > > Defines a new public API macro ODP_CPUMASK_SIZE that is the maximum > number of CPUs that can be accessed in this system. > > Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=1864 > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- > include/odp/api/cpumask.h | 6 ++++++ > platform/linux-generic/include/odp/plat/cpumask_types.h | 5 +++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h > index 4835a6c..7480132 100644 > --- a/include/odp/api/cpumask.h > +++ b/include/odp/api/cpumask.h > @@ -26,6 +26,12 @@ extern "C" { > */ > > /** > + * @def ODP_CPUMASK_SIZE > + * Maximum cpumask size, this definition limits the number of individual CPUs > + * that can be accessed in this system. > + */ > + > +/** > * @def ODP_CPUMASK_STR_SIZE > * Minimum size of output buffer for odp_cpumask_to_str() > */ > diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h b/platform/linux-generic/include/odp/plat/cpumask_types.h > index affe59b..1beaa1d 100644 > --- a/platform/linux-generic/include/odp/plat/cpumask_types.h > +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h > @@ -23,12 +23,13 @@ extern "C" { > */ > > #include <odp/std_types.h> > -#include <odp_config_internal.h> > + > +#define ODP_CPUMASK_SIZE 1024 > > /** > * Minimum size of output buffer for odp_cpumask_to_str() > */ > -#define ODP_CPUMASK_STR_SIZE ((_ODP_INTERNAL_MAX_THREADS + 3) / 4 + 3) > +#define ODP_CPUMASK_STR_SIZE ((ODP_CPUMASK_SIZE + 3) / 4 + 3) > it was 128 you changed it to 1024. linux-generic will create array[128] you will refer to indexes up to 1024? From first quick look that patch does not looks good. ODP_STATIC_ASSERT is needed to keep correlation with CPUMASK. Maxim. > /** > * CPU mask
On 28 October 2015 at 16:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 10/28/2015 17:32, Anders Roxell wrote: >> >> Applications that use an installed ODP will get the following >> compile error: >> .../odp_install/include/odp/plat/cpumask_types.h:26:33: fatal error: >> odp_config_internal.h: No such file or directory >> #include <odp_config_internal.h> >> >> Defines a new public API macro ODP_CPUMASK_SIZE that is the maximum >> number of CPUs that can be accessed in this system. >> >> Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=1864 >> >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >> --- >> include/odp/api/cpumask.h | 6 ++++++ >> platform/linux-generic/include/odp/plat/cpumask_types.h | 5 +++-- >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h >> index 4835a6c..7480132 100644 >> --- a/include/odp/api/cpumask.h >> +++ b/include/odp/api/cpumask.h >> @@ -26,6 +26,12 @@ extern "C" { >> */ >> /** >> + * @def ODP_CPUMASK_SIZE >> + * Maximum cpumask size, this definition limits the number of individual >> CPUs >> + * that can be accessed in this system. >> + */ >> + >> +/** >> * @def ODP_CPUMASK_STR_SIZE >> * Minimum size of output buffer for odp_cpumask_to_str() >> */ >> diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h >> b/platform/linux-generic/include/odp/plat/cpumask_types.h >> index affe59b..1beaa1d 100644 >> --- a/platform/linux-generic/include/odp/plat/cpumask_types.h >> +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h >> @@ -23,12 +23,13 @@ extern "C" { >> */ >> #include <odp/std_types.h> >> -#include <odp_config_internal.h> >> + >> +#define ODP_CPUMASK_SIZE 1024 >> /** >> * Minimum size of output buffer for odp_cpumask_to_str() >> */ >> -#define ODP_CPUMASK_STR_SIZE ((_ODP_INTERNAL_MAX_THREADS + 3) / 4 + 3) >> +#define ODP_CPUMASK_STR_SIZE ((ODP_CPUMASK_SIZE + 3) / 4 + 3) >> > > it was 128 you changed it to 1024. linux-generic will create array[128] you > will refer to indexes up to 1024? In discussions with Mike and Petri there seams to be no correlation between threads and CPUs. You can have less threads than you have CPUs, exactly the same number or many threads per CPU. Obviously we know if we have more threads per CPU the performance will not be great, but we shouldn't stop the implementations of ODP doing this. > From first quick look that patch does not looks good. ODP_STATIC_ASSERT is > needed to keep correlation with CPUMASK. So we only need to test that the ODP_CPUMASK_SIZE can be accommodated by the platform its on. +/** @internal Compile time assert */ +_ODP_STATIC_ASSERT(CPU_SETSIZE >= ODP_CPUMASK_SIZE, + "ODP_CPUMASK_SIZE__SIZE_ERROR"); + Cheers, Anders
On 10/29/2015 00:24, Anders Roxell wrote: > On 28 October 2015 at 16:55, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: >> On 10/28/2015 17:32, Anders Roxell wrote: >>> Applications that use an installed ODP will get the following >>> compile error: >>> .../odp_install/include/odp/plat/cpumask_types.h:26:33: fatal error: >>> odp_config_internal.h: No such file or directory >>> #include <odp_config_internal.h> >>> >>> Defines a new public API macro ODP_CPUMASK_SIZE that is the maximum >>> number of CPUs that can be accessed in this system. >>> >>> Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=1864 >>> >>> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >>> --- >>> include/odp/api/cpumask.h | 6 ++++++ >>> platform/linux-generic/include/odp/plat/cpumask_types.h | 5 +++-- >>> 2 files changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h >>> index 4835a6c..7480132 100644 >>> --- a/include/odp/api/cpumask.h >>> +++ b/include/odp/api/cpumask.h >>> @@ -26,6 +26,12 @@ extern "C" { >>> */ >>> /** >>> + * @def ODP_CPUMASK_SIZE >>> + * Maximum cpumask size, this definition limits the number of individual >>> CPUs >>> + * that can be accessed in this system. >>> + */ >>> + >>> +/** >>> * @def ODP_CPUMASK_STR_SIZE >>> * Minimum size of output buffer for odp_cpumask_to_str() >>> */ >>> diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h >>> b/platform/linux-generic/include/odp/plat/cpumask_types.h >>> index affe59b..1beaa1d 100644 >>> --- a/platform/linux-generic/include/odp/plat/cpumask_types.h >>> +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h >>> @@ -23,12 +23,13 @@ extern "C" { >>> */ >>> #include <odp/std_types.h> >>> -#include <odp_config_internal.h> >>> + >>> +#define ODP_CPUMASK_SIZE 1024 >>> /** >>> * Minimum size of output buffer for odp_cpumask_to_str() >>> */ >>> -#define ODP_CPUMASK_STR_SIZE ((_ODP_INTERNAL_MAX_THREADS + 3) / 4 + 3) >>> +#define ODP_CPUMASK_STR_SIZE ((ODP_CPUMASK_SIZE + 3) / 4 + 3) >>> >> it was 128 you changed it to 1024. linux-generic will create array[128] you >> will refer to indexes up to 1024? > In discussions with Mike and Petri there seams to be no correlation > between threads and CPUs. > You can have less threads than you have CPUs, exactly the same number > or many threads per CPU. > Obviously we know if we have more threads per CPU the performance will > not be great, but we shouldn't > stop the implementations of ODP doing this. yes, agree with you, I just forgot about running several thread on one cpu :) >> From first quick look that patch does not looks good. ODP_STATIC_ASSERT is >> needed to keep correlation with CPUMASK. > So we only need to test that the ODP_CPUMASK_SIZE can be accommodated > by the platform its on. > > +/** @internal Compile time assert */ > +_ODP_STATIC_ASSERT(CPU_SETSIZE >= ODP_CPUMASK_SIZE, > + "ODP_CPUMASK_SIZE__SIZE_ERROR"); > + yes. Maxim. > > Cheers, > Anders
Merged to api-next. Needed to do the same for rwlock_recursive_types.h. Will merge patches to master soon. Maxim. On 10/29/2015 10:32, Savolainen, Petri (Nokia - FI/Espoo) wrote: > Both patches: > > Reviewed-by: Petri Savolainen <petri.savolainen@nokia.com> > > >> -----Original Message----- >> From: EXT Anders Roxell [mailto:anders.roxell@linaro.org] >> Sent: Wednesday, October 28, 2015 4:33 PM >> To: Orpana, Pasi (Nokia - FI/Espoo); Wallen, Carl (Nokia - FI/Espoo) >> Cc: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo); Anders >> Roxell >> Subject: [PATCH 1/2] api: cpumask: don't use platform internal defines >> >> Applications that use an installed ODP will get the following >> compile error: >> .../odp_install/include/odp/plat/cpumask_types.h:26:33: fatal error: >> odp_config_internal.h: No such file or directory >> #include <odp_config_internal.h> >> >> Defines a new public API macro ODP_CPUMASK_SIZE that is the maximum >> number of CPUs that can be accessed in this system. >> >> Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=1864 >> >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >> --- >> include/odp/api/cpumask.h | 6 ++++++ >> platform/linux-generic/include/odp/plat/cpumask_types.h | 5 +++-- >> 2 files changed, 9 insertions(+), 2 deletions(-) >> >> diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h >> index 4835a6c..7480132 100644 >> --- a/include/odp/api/cpumask.h >> +++ b/include/odp/api/cpumask.h >> @@ -26,6 +26,12 @@ extern "C" { >> */ >> >> /** >> + * @def ODP_CPUMASK_SIZE >> + * Maximum cpumask size, this definition limits the number of individual >> CPUs >> + * that can be accessed in this system. >> + */ >> + >> +/** >> * @def ODP_CPUMASK_STR_SIZE >> * Minimum size of output buffer for odp_cpumask_to_str() >> */ >> diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h >> b/platform/linux-generic/include/odp/plat/cpumask_types.h >> index affe59b..1beaa1d 100644 >> --- a/platform/linux-generic/include/odp/plat/cpumask_types.h >> +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h >> @@ -23,12 +23,13 @@ extern "C" { >> */ >> >> #include <odp/std_types.h> >> -#include <odp_config_internal.h> >> + >> +#define ODP_CPUMASK_SIZE 1024 >> >> /** >> * Minimum size of output buffer for odp_cpumask_to_str() >> */ >> -#define ODP_CPUMASK_STR_SIZE ((_ODP_INTERNAL_MAX_THREADS + 3) / 4 + 3) >> +#define ODP_CPUMASK_STR_SIZE ((ODP_CPUMASK_SIZE + 3) / 4 + 3) >> >> /** >> * CPU mask >> -- >> 2.1.4 > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/include/odp/api/cpumask.h b/include/odp/api/cpumask.h index 4835a6c..7480132 100644 --- a/include/odp/api/cpumask.h +++ b/include/odp/api/cpumask.h @@ -26,6 +26,12 @@ extern "C" { */ /** + * @def ODP_CPUMASK_SIZE + * Maximum cpumask size, this definition limits the number of individual CPUs + * that can be accessed in this system. + */ + +/** * @def ODP_CPUMASK_STR_SIZE * Minimum size of output buffer for odp_cpumask_to_str() */ diff --git a/platform/linux-generic/include/odp/plat/cpumask_types.h b/platform/linux-generic/include/odp/plat/cpumask_types.h index affe59b..1beaa1d 100644 --- a/platform/linux-generic/include/odp/plat/cpumask_types.h +++ b/platform/linux-generic/include/odp/plat/cpumask_types.h @@ -23,12 +23,13 @@ extern "C" { */ #include <odp/std_types.h> -#include <odp_config_internal.h> + +#define ODP_CPUMASK_SIZE 1024 /** * Minimum size of output buffer for odp_cpumask_to_str() */ -#define ODP_CPUMASK_STR_SIZE ((_ODP_INTERNAL_MAX_THREADS + 3) / 4 + 3) +#define ODP_CPUMASK_STR_SIZE ((ODP_CPUMASK_SIZE + 3) / 4 + 3) /** * CPU mask
Applications that use an installed ODP will get the following compile error: .../odp_install/include/odp/plat/cpumask_types.h:26:33: fatal error: odp_config_internal.h: No such file or directory #include <odp_config_internal.h> Defines a new public API macro ODP_CPUMASK_SIZE that is the maximum number of CPUs that can be accessed in this system. Fixes bug: https://bugs.linaro.org/show_bug.cgi?id=1864 Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- include/odp/api/cpumask.h | 6 ++++++ platform/linux-generic/include/odp/plat/cpumask_types.h | 5 +++-- 2 files changed, 9 insertions(+), 2 deletions(-)