| Message ID | 1403027720-9738-10-git-send-email-taras.kondratiuk@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > --- > include/odp_config.h | 11 +++--- > platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items and number of config items to build the libodp.{a,so} > 2 files changed, 50 insertions(+), 5 deletions(-) > create mode 100644 platform/linux-generic/include/plat/odp_config.h > > diff --git a/include/odp_config.h b/include/odp_config.h > index 7a15ff9..e6d0cae 100644 > --- a/include/odp_config.h > +++ b/include/odp_config.h > @@ -18,31 +18,32 @@ > extern "C" { > #endif > > +#include <plat/odp_config.h> > > /** > * Maximum number of threads > */ > -#define ODP_CONFIG_MAX_THREADS 128 > +#define ODP_CONFIG_MAX_THREADS PLAT_ODP_CONFIG_MAX_THREADS > > /** > * Maximum number of buffer pools > */ > -#define ODP_CONFIG_BUFFER_POOLS 16 > +#define ODP_CONFIG_BUFFER_POOLS PLAT_ODP_CONFIG_BUFFER_POOLS > > /** > * Maximum number of queues > */ > -#define ODP_CONFIG_QUEUES 1024 > +#define ODP_CONFIG_QUEUES PLAT_ODP_CONFIG_QUEUES > > /** > * Number of scheduling priorities > */ > -#define ODP_CONFIG_SCHED_PRIOS 8 > +#define ODP_CONFIG_SCHED_PRIOS PLAT_ODP_CONFIG_SCHED_PRIOS > > /** > * Maximum number of packet IO resources > */ > -#define ODP_CONFIG_PKTIO_ENTRIES 64 > +#define ODP_CONFIG_PKTIO_ENTRIES PLAT_ODP_CONFIG_PKTIO_ENTRIES > > #ifdef __cplusplus > } > diff --git a/platform/linux-generic/include/plat/odp_config.h b/platform/linux-generic/include/plat/odp_config.h > new file mode 100644 > index 0000000..7230925 > --- /dev/null > +++ b/platform/linux-generic/include/plat/odp_config.h > @@ -0,0 +1,44 @@ > +/* Copyright (c) 2013, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > + > +/** > + * @file > + * > + * ODP configuration > + */ > + > +#ifndef ODP_CONFIG_H_ > +#error This file should be included only into corresponding top level header > +#else > + > +/** > + * Maximum number of threads > + */ > +#define PLAT_ODP_CONFIG_MAX_THREADS 128 > + > +/** > + * Maximum number of buffer pools > + */ > +#define PLAT_ODP_CONFIG_BUFFER_POOLS 16 > + > +/** > + * Maximum number of queues > + */ > +#define PLAT_ODP_CONFIG_QUEUES 1024 > + > +/** > + * Number of scheduling priorities > + */ > +#define PLAT_ODP_CONFIG_SCHED_PRIOS 8 > + > +/** > + * Maximum number of packet IO resources > + */ > +#define PLAT_ODP_CONFIG_PKTIO_ENTRIES 64 > + > + > +#endif > -- > 1.7.9.5 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
On 06/18/2014 12:58 PM, Jerin Jacob wrote: > On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: >> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >> --- >> include/odp_config.h | 11 +++--- >> platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ > > All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. > So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) It is just a coincidence that all parameters here are platform-specific. Some configuration functions may be added later. > More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items > and number of config items to build the libodp.{a,so} IMO some of these values are public. Application may want, for example, to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any additional configuration parameters can be added to a platform config header if needed.
On Wed, Jun 18, 2014 at 01:37:59PM +0300, Taras Kondratiuk wrote: > On 06/18/2014 12:58 PM, Jerin Jacob wrote: > > On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: > >> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > >> --- > >> include/odp_config.h | 11 +++--- > >> platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ > > > > All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. > > So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) > > It is just a coincidence that all parameters here are platform-specific. > Some configuration functions may be added later. > I was under the impression that, the value for such configuration always comes from the platform as we are including only plat/odp_config.h and if we try to find a default value for any configuration item that works well across all platform then it may not be a configuration entry. Anyway, if you foresee any such global configuration in future then lets keep the indirection. > > More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items > > and number of config items to build the libodp.{a,so} > > IMO some of these values are public. Application may want, for example, > to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any > additional configuration parameters can be added to a platform config > header if needed. IMO Interface between ODP and apps better to be an API, just like ODP system info APIs(example: odp_sys_core_count). > > -- > Taras Kondratiuk
On 06/18/2014 03:29 PM, Jerin Jacob wrote: > On Wed, Jun 18, 2014 at 01:37:59PM +0300, Taras Kondratiuk wrote: >> On 06/18/2014 12:58 PM, Jerin Jacob wrote: >>> On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: >>>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >>>> --- >>>> include/odp_config.h | 11 +++--- >>>> platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ >>> >>> All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. >>> So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) >> >> It is just a coincidence that all parameters here are platform-specific. >> Some configuration functions may be added later. >> > > I was under the impression that, the value for such configuration always > comes from the platform as we are including only plat/odp_config.h and > if we try to find a default value for any configuration item that works > well across all platform then it may not be a configuration entry. > > Anyway, if you foresee any such global configuration in future then lets keep the > indirection. > >>> More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items >>> and number of config items to build the libodp.{a,so} >> >> IMO some of these values are public. Application may want, for example, >> to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any >> additional configuration parameters can be added to a platform config >> header if needed. > > IMO Interface between ODP and apps better to be an API, just like ODP system info APIs(example: odp_sys_core_count). Agree, but in this case a function prototypes like odp_max_thread_count() will anyway go to odp_config.h, so no sense to remove it. We need to agree on a basic approach to more forward. These details can be polished later.
On 06/18/2014 03:29 PM, Jerin Jacob wrote: > On Wed, Jun 18, 2014 at 01:37:59PM +0300, Taras Kondratiuk wrote: >> On 06/18/2014 12:58 PM, Jerin Jacob wrote: >>> On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: >>>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> >>>> --- >>>> include/odp_config.h | 11 +++--- >>>> platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ >>> >>> All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. >>> So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) >> >> It is just a coincidence that all parameters here are platform-specific. >> Some configuration functions may be added later. >> > > I was under the impression that, the value for such configuration always > comes from the platform as we are including only plat/odp_config.h and > if we try to find a default value for any configuration item that works > well across all platform then it may not be a configuration entry. > > Anyway, if you foresee any such global configuration in future then lets keep the > indirection. > >>> More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items >>> and number of config items to build the libodp.{a,so} >> >> IMO some of these values are public. Application may want, for example, >> to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any >> additional configuration parameters can be added to a platform config >> header if needed. > > IMO Interface between ODP and apps better to be an API, just like ODP system info APIs(example: odp_sys_core_count). Agree, but in this case a function prototypes like odp_max_thread_count() will anyway go to odp_config.h, so no sense to remove it. We need to agree on a basic approach to more forward. These details can be polished later.
On Wed, Jun 18, 2014 at 03:48:34PM +0300, Taras Kondratiuk wrote: > On 06/18/2014 03:29 PM, Jerin Jacob wrote: > > On Wed, Jun 18, 2014 at 01:37:59PM +0300, Taras Kondratiuk wrote: > >> On 06/18/2014 12:58 PM, Jerin Jacob wrote: > >>> On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: > >>>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > >>>> --- > >>>> include/odp_config.h | 11 +++--- > >>>> platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ > >>> > >>> All the parameters in include/odp_config.h are derived from the include/plat/odp_config.h file. > >>> So instead of keeping additional indirection, can we move the config file to per platform(platform/xxxx/include/odp_config.h) > >> > >> It is just a coincidence that all parameters here are platform-specific. > >> Some configuration functions may be added later. > >> > > > > I was under the impression that, the value for such configuration always > > comes from the platform as we are including only plat/odp_config.h and > > if we try to find a default value for any configuration item that works > > well across all platform then it may not be a configuration entry. > > > > Anyway, if you foresee any such global configuration in future then lets keep the > > indirection. > > > >>> More over, I think config file should not be part of ODP public header file as each platform will have its on constrains on each config items > >>> and number of config items to build the libodp.{a,so} > >> > >> IMO some of these values are public. Application may want, for example, > >> to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any > >> additional configuration parameters can be added to a platform config > >> header if needed. > > > > IMO Interface between ODP and apps better to be an API, just like ODP system info APIs(example: odp_sys_core_count). > > Agree, but in this case a function prototypes like > odp_max_thread_count() will anyway go to odp_config.h, so no sense to > remove it. We need to agree on a basic approach to more forward. These > details can be polished later. I agree and I liked your approach towards introducing indirection to enable platform abstraction. I was trying to see is it really required for odp_config.h file or not(as its different from all other header file)?. If you think its really required then please keep the indirection for odp_config.h. Thanks, Jerin > > -- > Taras Kondratiuk
> -----Original Message----- > From: lng-odp-bounces@lists.linaro.org [mailto:lng-odp- > bounces@lists.linaro.org] On Behalf Of ext Jerin Jacob > Sent: Wednesday, June 18, 2014 4:17 PM > To: Taras Kondratiuk > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v2 09/18] Split out platform-specific > part of odp_config.h > > On Wed, Jun 18, 2014 at 03:48:34PM +0300, Taras Kondratiuk wrote: > > On 06/18/2014 03:29 PM, Jerin Jacob wrote: > > > On Wed, Jun 18, 2014 at 01:37:59PM +0300, Taras Kondratiuk wrote: > > >> On 06/18/2014 12:58 PM, Jerin Jacob wrote: > > >>> On Tue, Jun 17, 2014 at 08:55:11PM +0300, Taras Kondratiuk wrote: > > >>>> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > > >>>> --- > > >>>> include/odp_config.h | 11 +++--- > > >>>> platform/linux-generic/include/plat/odp_config.h | 44 > ++++++++++++++++++++++ > > >>> > > >>> All the parameters in include/odp_config.h are derived from the > include/plat/odp_config.h file. > > >>> So instead of keeping additional indirection, can we move the > config file to per platform(platform/xxxx/include/odp_config.h) > > >> > > >> It is just a coincidence that all parameters here are platform- > specific. > > >> Some configuration functions may be added later. > > >> > > > > > > I was under the impression that, the value for such configuration > always > > > comes from the platform as we are including only plat/odp_config.h > and > > > if we try to find a default value for any configuration item that > works > > > well across all platform then it may not be a configuration entry. > > > > > > Anyway, if you foresee any such global configuration in future then > lets keep the > > > indirection. > > > > > >>> More over, I think config file should not be part of ODP public > header file as each platform will have its on constrains on each config > items > > >>> and number of config items to build the libodp.{a,so} > > >> > > >> IMO some of these values are public. Application may want, for > example, > > >> to know ODP_CONFIG_MAX_THREADS value to adjust its logic. Any > > >> additional configuration parameters can be added to a platform > config > > >> header if needed. > > > > > > IMO Interface between ODP and apps better to be an API, just like > ODP system info APIs(example: odp_sys_core_count). > > > > Agree, but in this case a function prototypes like > > odp_max_thread_count() will anyway go to odp_config.h, so no sense to > > remove it. We need to agree on a basic approach to more forward. > These > > details can be polished later. > > I agree and I liked your approach towards introducing indirection to > enable platform abstraction. > I was trying to see is it really required for odp_config.h file or > not(as its different from all other header file)?. > If you think its really required then please keep the indirection for > odp_config.h. > A set of defines (like ODP_CONFIG_MAX_THREADS) are needed for controlling size of application data structures (e.g. an array of per thread statistics, per queue context data, ...). Also user should be able to modify some of these (to optimize memory ODP implementation memory usage, etc). Those should be collected into a common header file (odp_config.h, which would be overridable on application level ?) - Petri
diff --git a/include/odp_config.h b/include/odp_config.h index 7a15ff9..e6d0cae 100644 --- a/include/odp_config.h +++ b/include/odp_config.h @@ -18,31 +18,32 @@ extern "C" { #endif +#include <plat/odp_config.h> /** * Maximum number of threads */ -#define ODP_CONFIG_MAX_THREADS 128 +#define ODP_CONFIG_MAX_THREADS PLAT_ODP_CONFIG_MAX_THREADS /** * Maximum number of buffer pools */ -#define ODP_CONFIG_BUFFER_POOLS 16 +#define ODP_CONFIG_BUFFER_POOLS PLAT_ODP_CONFIG_BUFFER_POOLS /** * Maximum number of queues */ -#define ODP_CONFIG_QUEUES 1024 +#define ODP_CONFIG_QUEUES PLAT_ODP_CONFIG_QUEUES /** * Number of scheduling priorities */ -#define ODP_CONFIG_SCHED_PRIOS 8 +#define ODP_CONFIG_SCHED_PRIOS PLAT_ODP_CONFIG_SCHED_PRIOS /** * Maximum number of packet IO resources */ -#define ODP_CONFIG_PKTIO_ENTRIES 64 +#define ODP_CONFIG_PKTIO_ENTRIES PLAT_ODP_CONFIG_PKTIO_ENTRIES #ifdef __cplusplus } diff --git a/platform/linux-generic/include/plat/odp_config.h b/platform/linux-generic/include/plat/odp_config.h new file mode 100644 index 0000000..7230925 --- /dev/null +++ b/platform/linux-generic/include/plat/odp_config.h @@ -0,0 +1,44 @@ +/* Copyright (c) 2013, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + + +/** + * @file + * + * ODP configuration + */ + +#ifndef ODP_CONFIG_H_ +#error This file should be included only into corresponding top level header +#else + +/** + * Maximum number of threads + */ +#define PLAT_ODP_CONFIG_MAX_THREADS 128 + +/** + * Maximum number of buffer pools + */ +#define PLAT_ODP_CONFIG_BUFFER_POOLS 16 + +/** + * Maximum number of queues + */ +#define PLAT_ODP_CONFIG_QUEUES 1024 + +/** + * Number of scheduling priorities + */ +#define PLAT_ODP_CONFIG_SCHED_PRIOS 8 + +/** + * Maximum number of packet IO resources + */ +#define PLAT_ODP_CONFIG_PKTIO_ENTRIES 64 + + +#endif
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> --- include/odp_config.h | 11 +++--- platform/linux-generic/include/plat/odp_config.h | 44 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 platform/linux-generic/include/plat/odp_config.h