On Fri, Jul 29, 2016 at 2:40 AM, Christophe Milard <
[email protected]> wrote:

> On 29 July 2016 at 00:26, Bill Fischofer <[email protected]>
> wrote:
> > General comments:
> >
> > 1. How is this hooked in to the rest of doxygen? After applying it I see
> the
> > source file but I don't see how it gets included and formatted by doxygen
> > anywhere.
>
> You can generate the docs with:
> $ ./bootstrap; ./configure  --enable-user-guides && make -j 8
> $ make doxygen-doc
> Since Mike's patch (793c4b15275fd7e6c30bf348f236e778b9bfee02),
> this also generates platform specific API guide, which is a copy of
> the "public" specification, with possible add-ons specific to the
> given platform.
> The resulting platform specific API doc can be viewed with:
> $ firefox doc/platform-api-guide/output/html/index.html
>
>
OK, I see that now.  It wasn't clear from the patch where this would show
up.


> >
> > 2. Why is this in platform/linux-generic/include/odp/api ? Everything
> else
> > in that directory are .h files.
>
> Because these are platform specific add-ons (as these changes were
> already nacked when suggested on the public API docs, the reason being
> that all platforms may not want these restrictions).
> It is the first time linux-gen documentation add-ons are written,
> hence this new file.
> The file has to be under:
> $(SRCDIR)/platform/$(WITH_PLATFORM)/include/odp
> as this is where it will be searched for (see
> doc/platform-api-guide/Doxyfile in Mike's patch). Of course, it could
> get its own directory, but as long as it is unclear how this
> platform-specific docs will grow, if felt a bit overkill. It is easy
> to change that later if the need comes.
>

OK, but I agree a separate platform-specific doc directory would seem to be
a better way to keep things organized.


>
> >
> > On Thu, Jul 28, 2016 at 6:59 AM, Christophe Milard
> > <[email protected]> wrote:
> >>
> >> Defining an odp thread for the linux generic implementation of ODP.
> >>
> >> Signed-off-by: Christophe Milard <[email protected]>
> >> ---
> >>  .../include/odp/api/platform_specific.dox          | 46
> >> ++++++++++++++++++++++
> >>  1 file changed, 46 insertions(+)
> >>  create mode 100644
> >> platform/linux-generic/include/odp/api/platform_specific.dox
> >>
> >> diff --git
> a/platform/linux-generic/include/odp/api/platform_specific.dox
> >> b/platform/linux-generic/include/odp/api/platform_specific.dox
> >> new file mode 100644
> >> index 0000000..e116ec6
> >> --- /dev/null
> >> +++ b/platform/linux-generic/include/odp/api/platform_specific.dox
> >> @@ -0,0 +1,46 @@
> >> +/* Copyright (c) 2016, Linaro Limited
> >> + * All rights reserved
> >> + *
> >> + * SPDX-License-Identifier:     BSD-3-Clause
> >> + */
> >> +
> >> +/**
> >> +* @file platform_specific.dox
> >> +* extra linux-generic documentation
> >> +*/
> >> +
> >> +/** @addtogroup odp_thread
> >> + *  @par ODP thread
> >> + *  In this ODP implementation an odp thread is either:
> >> + *  - a linux process descendant (or same as) the odp instantiation
> >> process.
> >> + *  - a pthread 'member' of a linux process descendant (or same as) the
> >> odp
> >> + *  instantiation process.
> >> + */
> >> +
> >> +/**
> >> + *  @fn odp_init_local(odp_instance_t instance, odp_thread_type_t
> >> thr_type)
> >> + *  @note              In this ODP implementation odpthreads have to be
> >> + *                     processes descendant of (or same as) the ODP
> >> + *                     instantiation process, or pthreads 'member' of
> >> such
> >> + *                     processes.
> >> + *  @note              As ODP instantiation processes cannot be
> >> descendants
> >> + *                     of each others, the instance parameter provided
> >> + *                     to odp_init_local() is actually fully defined by
> >> these
> >> + *                     requirements: It has to be the value returned by
> >> the
> >> + *                     unique call to odp_init_global() made by one
> >> single
> >> + *                     acsendant of the current process.
> >
> >
> > I'm not sure what the purpose of these notes are. Isn't it sufficient to
> say
> > that in this implementation odp_init_global() can only be called once per
> > root process?  Everything else implies this and the elaborations are
> > unnecessary if this restriction is simply plainly stated.
>
> Assuming what you call "root process" is what is defined as "ODP
> instantiation process" there are a few problems with your single
> statement:
>
> 1) It is kind of recursive, as a linux process becomes a
> "instantiation process" once it has called odp_global_init()!.
> Literally, Your sentence would actually mean that a linux process can
> call odp_global_init() twice!: Once to become a "Instantiation process
> aka 'root_process' ", and then once more!
>
> 2)Your sentence does not cover descendance: saying that the
> instantiation process is not allowed to perform any further call to
> odp_global_init() once the first call (making it an ODP instantiation
> process) is done does not say anything about its children. With your
> single statement nothing prevents a child process to try to create an
> odp sub-instance by calling odp_global_init().
>
> 3) your sentence does not force odp-threads to be descendant of the
> instantiation process... and this is probaly very unclear to a user
> that sees an "odp_instance_t  instance" parameter to odp_init_local():
> We have agreed that, on the linux generic implementation, ODP threads
> have to be descendant of the instantiation process and we also have
> agreed on the fact that instantiation processes cannot be descendant
> of each-other. These assumptions actually make the "instance "
> parameter of odp_init_local() completely useless for this
> implementation! So these assumptions must be very well stated as the
> prototype defined in the public API alone can lead to erroneous
> conclusion for this platform (if there is an instance in
> odp_init_local(), it then makes sence to assume that any process can
> attach to any instance! why would you have this parameter oherwise?)
>
> So even if the formulation I had sounds somehow heavy, I still think
> it is more accurate...
>

OK, thanks for the explanation.


>
> Christophe
>
> >
> >>
> >> + */
> >> +
> >> +/**
> >> + *  @fn odp_init_global(odp_instance_t *instance,
> >> + *                  const odp_init_t *params,
> >> + *                  const odp_platform_init_t *platform_params)
> >> + *  @note      This ODP implementation supports mupliple instances of
> ODP
> >> + *             (i.e. multiple call to odp_init_global()) with the
> >> following
> >> + *             restrictions:
> >> + *  @note       -Different ODP instances cannot share the same
> >> instantiation
> >> + *             process. In other words, a single process may
> >> + *             only call odp_init_global() once.
> >> + *  @note      -Different ODP instantiation processes cannot be
> >> descendant of
> >> + *             each other.
> >>
> >> + */
> >> --
> >> 2.7.4
> >>
> >
>

Reply via email to