| Message ID | 20170421131134.27992-2-petri.savolainen@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Use HW time counter | expand |
On 04/21 16:11:27, Petri Savolainen wrote: > This information specifies the system where ODP application > is running for debugging purposes. > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > --- > include/odp/api/spec/system_info.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/odp/api/spec/system_info.h b/include/odp/api/spec/system_info.h > index 0bb4f1f1..ca4dcdc7 100644 > --- a/include/odp/api/spec/system_info.h > +++ b/include/odp/api/spec/system_info.h > @@ -45,6 +45,15 @@ uint64_t odp_sys_page_size(void); > int odp_sys_cache_line_size(void); > > /** > + * Print system info I would advise that APIs return information that can be printed by the application or used otherwise. An API like this indicates that the implementation itself would be sending something to stdout; that can be done by the application. > + * Print out implementation defined information about the system. This > + * information is intended for debugging purposes and may contain e.g. > + * information about CPUs, memory and other HW configuration. > + */ > +void odp_sys_info_print(void); > + > +/** > * @} > */ > > -- > 2.11.0 >
On Fri, Apr 21, 2017 at 11:42 AM, Brian Brooks <brian.brooks@arm.com> wrote: > On 04/21 16:11:27, Petri Savolainen wrote: > > This information specifies the system where ODP application > > is running for debugging purposes. > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > > --- > > include/odp/api/spec/system_info.h | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/include/odp/api/spec/system_info.h > b/include/odp/api/spec/system_info.h > > index 0bb4f1f1..ca4dcdc7 100644 > > --- a/include/odp/api/spec/system_info.h > > +++ b/include/odp/api/spec/system_info.h > > @@ -45,6 +45,15 @@ uint64_t odp_sys_page_size(void); > > int odp_sys_cache_line_size(void); > > > > /** > > + * Print system info > > I would advise that APIs return information that can be printed by > the application or used otherwise. An API like this indicates that > the implementation itself would be sending something to stdout; that > can be done by the application. > There is ample precedent in ODP for including APIs specifically for application debugging, e.g., odp_pool_print(), odp_buffer_print(), odp_packet_print(), so I don't see this as any different. There's a difference between APIs for debugging vs. logging. A debugging API would not be used in a production setting whereas a logging API would. For logging APIs we have the ability for the application to provide an overriding output function to allow it to consolidate or otherwise handle the processing of this output. > > > + * Print out implementation defined information about the system. This > > + * information is intended for debugging purposes and may contain e.g. > > + * information about CPUs, memory and other HW configuration. > > + */ > > +void odp_sys_info_print(void); > > + > > +/** > > * @} > > */ > > > > -- > > 2.11.0 > > >
> -----Original Message----- > From: Brian Brooks [mailto:brian.brooks@arm.com] > Sent: Friday, April 21, 2017 7:42 PM > To: Petri Savolainen <petri.savolainen@linaro.org> > Cc: lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [API-NEXT PATCH 1/8] api: system: added system info > print > > On 04/21 16:11:27, Petri Savolainen wrote: > > This information specifies the system where ODP application > > is running for debugging purposes. > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > > --- > > include/odp/api/spec/system_info.h | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/include/odp/api/spec/system_info.h > b/include/odp/api/spec/system_info.h > > index 0bb4f1f1..ca4dcdc7 100644 > > --- a/include/odp/api/spec/system_info.h > > +++ b/include/odp/api/spec/system_info.h > > @@ -45,6 +45,15 @@ uint64_t odp_sys_page_size(void); > > int odp_sys_cache_line_size(void); > > > > /** > > + * Print system info > > I would advise that APIs return information that can be printed by > the application or used otherwise. An API like this indicates that > the implementation itself would be sending something to stdout; that > can be done by the application. > As Bill said, this is similar to pool/packet/buffer_print() functions we already have. All these are easy to include into application code, especially when debugging. When a user hits a problem, an implementer would first ask to call this and send back the output. Output can be very implementation specific and thus useful in finding system configuration problems. E.g. an ARM SoC implementation could include ARM Foo-Bar Bus frequency or QoS settings in there, without a need from ODP API or the user to know anything about it. In this patch set, I output a lot of x86 CPU flags through this - without application needing to know if it's x86, how many flags there are, or how long string would be needed to output all that. -Petri > > + * Print out implementation defined information about the system. This > > + * information is intended for debugging purposes and may contain e.g. > > + * information about CPUs, memory and other HW configuration. > > + */ > > +void odp_sys_info_print(void); > > + > > +/** > > * @} > > */ > > > > -- > > 2.11.0 > >
On 04/21 14:26:55, Bill Fischofer wrote: > On Fri, Apr 21, 2017 at 11:42 AM, Brian Brooks <brian.brooks@arm.com> wrote: > > > On 04/21 16:11:27, Petri Savolainen wrote: > > > This information specifies the system where ODP application > > > is running for debugging purposes. > > > > > > Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> > > > --- > > > include/odp/api/spec/system_info.h | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/include/odp/api/spec/system_info.h > > b/include/odp/api/spec/system_info.h > > > index 0bb4f1f1..ca4dcdc7 100644 > > > --- a/include/odp/api/spec/system_info.h > > > +++ b/include/odp/api/spec/system_info.h > > > @@ -45,6 +45,15 @@ uint64_t odp_sys_page_size(void); > > > int odp_sys_cache_line_size(void); > > > > > > /** > > > + * Print system info > > > > I would advise that APIs return information that can be printed by > > the application or used otherwise. An API like this indicates that > > the implementation itself would be sending something to stdout; that > > can be done by the application. > > > > There is ample precedent in ODP for including APIs specifically for > application debugging, e.g., odp_pool_print(), odp_buffer_print(), > odp_packet_print(), so I don't see this as any different. There's a > difference between APIs for debugging vs. logging. A debugging API would > not be used in a production setting whereas a logging API would. For > logging APIs we have the ability for the application to provide an > overriding output function to allow it to consolidate or otherwise handle > the processing of this output. That was a recommendation along the lines of best practices rather than questioning what the current precedent is in the code today. > > > > > + * Print out implementation defined information about the system. This > > > + * information is intended for debugging purposes and may contain e.g. > > > + * information about CPUs, memory and other HW configuration. > > > + */ > > > +void odp_sys_info_print(void); > > > + > > > +/** > > > * @} > > > */ > > > > > > -- > > > 2.11.0 > > > > >
diff --git a/include/odp/api/spec/system_info.h b/include/odp/api/spec/system_info.h index 0bb4f1f1..ca4dcdc7 100644 --- a/include/odp/api/spec/system_info.h +++ b/include/odp/api/spec/system_info.h @@ -45,6 +45,15 @@ uint64_t odp_sys_page_size(void); int odp_sys_cache_line_size(void); /** + * Print system info + * + * Print out implementation defined information about the system. This + * information is intended for debugging purposes and may contain e.g. + * information about CPUs, memory and other HW configuration. + */ +void odp_sys_info_print(void); + +/** * @} */
This information specifies the system where ODP application is running for debugging purposes. Signed-off-by: Petri Savolainen <petri.savolainen@linaro.org> --- include/odp/api/spec/system_info.h | 9 +++++++++ 1 file changed, 9 insertions(+) -- 2.11.0