On 30/11/15 12:02, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Zoltan Kiss [mailto:zoltan.k...@linaro.org]
Sent: Thursday, November 26, 2015 4:35 PM
To: Savolainen, Petri (Nokia - FI/Espoo); lng-odp@lists.linaro.org
Cc: bill.fischo...@linaro.org; mike.hol...@linaro.org
Subject: Re: [API-NEXT PATCH] api: init: allow implementation to use
private ways for its own configuration
On 26/11/15 09:25, Savolainen, Petri (Nokia - FI/Espoo) wrote:
-----Original Message-----
From: EXT Zoltan Kiss [mailto:zoltan.k...@linaro.org]
Sent: Tuesday, November 24, 2015 4:45 PM
To: lng-odp@lists.linaro.org
Cc: bill.fischo...@linaro.org; Savolainen, Petri (Nokia - FI/Espoo);
mike.hol...@linaro.org
Subject: Re: [API-NEXT PATCH] api: init: allow implementation to use
private ways for its own configuration
Could anyone review this? Petri?
On 18/11/15 16:22, Zoltan Kiss wrote:
This could help the existing configuration methods to be used if the
application prefers that. The platform_params should always supersede
that
though.
Signed-off-by: Zoltan Kiss <zoltan.k...@linaro.org>
diff --git a/include/odp/api/init.h b/include/odp/api/init.h
index 737ff6d..24f4f3a 100644
--- a/include/odp/api/init.h
+++ b/include/odp/api/init.h
@@ -141,6 +141,9 @@ typedef struct odp_platform_init_t {
*
* This function must be called once before calling any other ODP
API
* functions.
+ * The underlying implementation may have another way to get
configuration
+ * related to platform_params (e.g. environmental variable,
configuration
+ * file), but if the application passes it, it should always
supersede.
*
* @param params Those parameters that are interpreted by
the
ODP API.
* Use NULL to set all parameters to their
defaults.
Which configuration should supersede which?
"but if the application passes it, it should always supersede."
As this is in the comment of odp_platform_init_t, sed
s/"it"/"odp_platform_init_t"/g
+ * The underlying implementation may have another way to get configuration
+ * related to platform_params (e.g. environmental variable, configuration
+ * file), but if the application passes it, it should always supersede.
It's not clear from this sentence if application:
- passes "another way" (e.g. config file) which supersedes "platform_params" ,or
- passes "platform_params" which supersedes "another way"
Both can be understood as "it".
Ok, I'll rephrase it as "but if the application passes platform_params"
Isn't it that way around that config files and env params are usually
used to override the hard coded configuration. So, you'd build your
application with a default config, but would use extra methods to override
the hard coded default config. If hard coded config all ways overrides,
you cannot try other configs without rebuilding (which may not be even
possible if you just have received the app binary).
Yes, but noone talks about hardcoded configuration here. Every sensible
application should have a sensible way to get configured, including ODP
platform parameters of the user's choice to be passed to the actual
platform. Of course the application can choose to detect the platform
runtime and apply a default if nothing else configured to it, but it
would be still more relevant than some platform defaults.
For example, if an (3rd party) application has been built before a new
(implementation specific) parameter was introduced, the app would init all
params to defaults (with odp_xxx_params_init()) and set those that it cares
about. Any new params would be in defaults unless you rebuild the app, which
may not be always possible.
This is an example of a badly written application. As I said above,
platform_params shouldn't be used for passing hardcoded default parameters.
I can add a sentence to remind people about having a sensible way of
configuration, and not trying to figure out the impossible at the time
of development.
Another example, optimal configuration is not known at build time. The system
is composed from multiple ODP apps, any single developer does not know which
ODP configuration is optimal or even possible when apps are integrated together.
Yes, that's why your app should have a way to get its configuration.
E.g. command line options, a config file. OVS has OVSDB, at the moment
you can also store a string called odp_platform_params there, which will
be passed to the platform in platform_params.
If there is a platform which needs something else than a string, then
OVS can provide a way for that. In any case, that's the implementation's
internal detail, the application may or may not know how to handle that,
and how to handle if it changes.
But it's not ODP's business, that's why we are providing only an opaque
type here. This patch just tries to encourage apps and platforms to pass
it here, because IF the app is able to configure the platform (e.g. OVS
at the moment is able to pass a string given by the user), then it's
better to pass the config here than forcing the user to use another way
(e.g. OVS used a gross hack in the init script to get the config from
OVSDB and set the env var)
It may vary per parameter and application, which parameters are possible
to override (without application 100% controlling it).
I'm not sure I understand that. A parameter is something which is
configurable, by definition. In other words, it's possible to override
it. "always supersede" implies to me that, but you can suggest a better
phrasing.
For example, maximum number of CPUs may be hard coded in an app,
I think that's the applications trouble, it has to handle that.
but any CPU MHz goes.
int my_cpu[32];
// crash if plat_config.max_cpu > 32
my_cpu[odp_cpu_id()].foo = 1;
// any value is OK
printf("CPU MHz %u", odp_cpu_mhz());
// Fill defaults (from config file)
// max_cpu = 64, max_mhz = 1000 (on this SoC)
plat_xyz_config_init_params(&plat_config);
Again, I strongly oppose this way. It doesn't make any sense. Any decent
applications should get the platform parameters through it's own config
from the user, and not try to hardcode these things.
plat_config.max_cpu = 32;
plat_config.max_mhz = 500; // 500MHz was max on the old SoC, could be
overridden with some plat specific way
Can we say anything else than platform configuration is platform
specific
Even platform config is platform specific, it also seems obvious to me.
and may include usage of platform_params ?
My aim here is that if the app provide platform_params, then the
platform MUST use that instead of other possible configuration ways (if
any). So the application can enforce (if it wants) config, which is
probably coming from the user through some way of app configuration.
I think the key here is that the plat_xyz_config_init_params(&plat_config)
reads the config file, so that those things application does not set in
platform_paras, can be still modified through a config file. But currently
plat_xyz_config_init_params() is platform specific (may not even exist).
-Petri
-Petri
_______________________________________________
lng-odp mailing list
lng-odp@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/lng-odp
