On 15 February 2017 at 09:34, Savolainen, Petri (Nokia - FI/Espoo)
<petri.savolai...@nokia-bell-labs.com> wrote:
>
>
>> -----Original Message-----
>> From: Christophe Milard [mailto:christophe.mil...@linaro.org]
>> Sent: Monday, February 13, 2017 5:41 PM
>> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-
>> labs.com>
>> Cc: Mike Holmes <mike.hol...@linaro.org>; lng-odp <lng-
>> o...@lists.linaro.org>
>> Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to
>> linux helpers
>>
>> This is getting very confusing now: why helper/<OS>?  do we have
>> odp/<OS>?: shouldn't  these be symetric?
>
>
> Application (like OFP today) which wants to use e.g. Linux pthread helpers 
> include:
>
> #include <odp_api.h>
> #include <odp/helper/linux/pthread.h>
>
> odph_linux_pthread_create()
> ...
> odph_linux_pthread_join()
>
>
> It explicitly uses Linux pthreads through a helper function. There's no need 
> to provide these helpers and functions on a non-Linux implementation (because 
> there is no Linux).
>
> Linux helpers are totally different thing than ODP implementation internal 
> directory structure.

Different: Yes. But related:
We are back, sadly, to my first question: "what is a platform".
To that, you, Petri, answered: "platform == implementation". And I agree.

Now, can we agree that the above platform definition implies:
different OS => different platform? Or are you saying that a OSE ODP
and linux ODP are the same implementation???.

If we can agree that, for ODP implementation, different OSes means
different platforms, do you feel comfortable calling these variation
differently for the helpers?

What if some helper function requires to be different for a given OS?
Say for instance, some company wants to implement ODP threads using
libdill or libmill? (on linux). Or some helper function would depend
on memory available on a given HW?

Yes, ODP implementation and helpers are 2 differents things, but they
are facing the same issue: They try to implement a given interface
(the ODP API  for ODP and the helper API for the helpers) on different
HW, OS, ... I'd say on different platform. I cannot see why helpers
variants would only be limited to OSes.

If we can agree on the above, we should agree that a common solution
to tackle the same problem would be nice.
On the ODP implementation side, we agreed that a platform was just a
way to diverge for the main delivery.
I still think it makes sense to have the same approach for the helpers:
If we deliver ODP implementation "linux-gen" and "ODP-dpdk" we should
deliver 2 variant of helper for these as well (even if they happen to
be the same).

A constructor willing to have his own ODP (whatever the reason) would
have the framework to diverge: a divergence is a new platform. Both
for helpers and ODP
The interesting question is how do we write the code so that these
divergence would be easy to do and so that maximum code can be reuse.
That is where the next challenge is.  But first we need to agree on
what a platform is and how we handle helpers.

Christophe.

>
>
>>
>> My view is getting clearer and clearer:
>> On the test side, was have made tests for {OS, HW}= {linux/PC} and
>> given the possibility to diverge from these (for any  reason, e.g.
>> difference of HW or OS) with the use of "platform":
>> If a test defined on the common validation test is inappropriate for a
>> ODP developer (whatever the reason), that developer can overwrite this
>> test (or part of it, such as its setup) with an appropriate one on the
>> platform side.
>> Kallray, AFAIK, if using the platform trick for both OS and HW divergence.
>
> If the system under test is not a Linux system. You cannot run linux helper 
> code there and thus you just skip the linux helper tests.
>
>
>>
>> Why not the same approach on the helper side? We give helpers for
>> {linux/PC} and provide a way to redefine these function for those who
>> need to,  on the platform side (e.g. using a weak symbol on the common
>> helpers and letting those who need define the same function as
>> "strong" on the helper platform side?... or any other better trick).
>
> Why a vendor which do not support Linux would rewrite e.g. 
> odph_linux_pthread_create() to emulate Linux pthreads? If the system is not 
> Linux, there is no point to run Linux system calls there and application 
> would not call odph_linux_pthread_create() there.
>
> Remember, helper code is application code. Application would not call Linux 
> system calls in an non-Linux system. So, no need for a non-Linux based ODP 
> implementation to emulate any Linux system calls.
>
>
>>
>> Then, any implementer knows where to place their hacks: on the
>> "platform side". Maybe not best, but at least consistent and clear.
>>
>> That would just give helpers, with different platform (for any reason)
>> implementations.
>>
>> The linux-specific helper would of course not fit on the common helper
>> part as they are linux specific and cannot be implementation on
>> different OS (platform). Once again, I don't understand why we needed
>> to do wrappers around well known Linux system calls: IMHO, helpers
>> should contains functions that we expect to be delivered on any
>> implementation (OS/HW) helpers, so these linux wrappers don't fit
>> there.
>
> Linux helpers can be used in Linux based systems - may be 90 - 95% of all 
> data plane systems today support or can support Linux.
>
> Non-Linux systems do not need to run Linux applications (helpers).
>
>
>> If we really want to keep that (I'd vote against), let's create
>> another directory name for what it is: "linux-wrappers"... and
>> separate them from function we'd expect to see everywhere (the rest of
>> the helpers, even if different platform dependent implementations for
>> those might exist)
>
> This patch uses odp/helper/linux/xx.h which is pretty explicit about the 
> dependency to Linux. Anything under odp/helper/linux is Linux dependent and 
> would not build/run on non-Linux systems. Clear?
>
>
> -Petri
>
>
>
>
>
>>
>> Christophe
>>
>> On 13 February 2017 at 16:09, Savolainen, Petri (Nokia - FI/Espoo)
>> <petri.savolai...@nokia-bell-labs.com> wrote:
>> >
>> >
>> > From: Mike Holmes [mailto:mike.hol...@linaro.org]
>> > Sent: Monday, February 13, 2017 5:02 PM
>> > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia-bell-
>> labs.com>
>> > Cc: lng-odp <lng-odp@lists.linaro.org>
>> > Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn
>> to linux helpers
>> >
>> >
>> >
>> > On 13 February 2017 at 09:41, Savolainen, Petri (Nokia - FI/Espoo)
>> <mailto:petri.savolai...@nokia-bell-labs.com> wrote:
>> >
>> >
>> > From: Mike Holmes [mailto:mailto:mike.hol...@linaro.org]
>> > Sent: Friday, February 10, 2017 5:02 PM
>> > To: Petri Savolainen <mailto:petri.savolai...@linaro.org>
>> > Cc: lng-odp <mailto:lng-odp@lists.linaro.org>
>> > Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn
>> to linux helpers
>> >
>> >
>> >
>> > On 3 February 2017 at 06:23, Petri Savolainen
>> <mailto:mailto:petri.savolai...@linaro.org> wrote:
>> > There's no platform specific helpers. Helpers may depend on
>> > Linux and make it easier to do common series of Linux system
>> > calls. These kind of helpers are grouped into helper/linux
>> > directory.
>> >
>> > Use --enable-helper-linux configuration option to enable
>> > support for Linux helpers.
>> >
>> > Signed-off-by: Petri Savolainen
>> <mailto:mailto:petri.savolai...@linaro.org>
>> >
>> > Reviewed-by: Mike Holmes <mailto:mailto:mike.hol...@linaro.org>
>> >
>> > required 3 way apply
>> >
>> >
>> >
>> > This should be merged. It moves linux dependent helper code under linux
>> folder. If there would be helper code dependent on some other OS, that
>> would go to their own helper/OS_foo/ folder. All Linux based ODP
>> implementations can run this helper code - there is no need to have N
>> different linux helpers for N different Linux based implementations.
>> >
>> >
>> > Also, the build is currently broken in master for --enable-helper-extn.
>> E.g. OFP cannot be built against it.
>> >
>> > This is not broken IMHO, it is by design, OFP depends on the helpers, we
>> need helpers to either be support for apps that we maintain and don't use
>> (current case) or we enforce  OFP and others to use the abstract API that
>> all our own executable use. Maintaining code we don't use is dangerous,
>> you should have to choose to do it, but I like the rename, that is fine
>> and is progress, hence the review.
>> >
>> > I think splitting out all of all the executables  will make this much
>> cleaner.
>> > All ODP upstream executables will use the abstract API and be portable
>> to any platform and OS. Meanwhile the odp-Linux repo can have a Linux
>> specific apis such as the legacy one OFP is using and that is just that
>> implementations choice, as we have said there is no requirement to use
>> helpers, or linux, and OFP might want to attach itself to that,  but the
>> tests and examples must remain agnostic until we state we are dropping
>> that goal.
>> >
>> >
>> > Please try:
>> > ./configure --enable-helper-extn
>> > make
>> >
>> > It does not compile, due to:
>> >
>> > platform/linux-generic/thread.c:282:12: error:
>> \u2018odph_linux_thread_create\u2019 defined but not used [-Werror=unused-
>> function]
>> >  static int odph_linux_thread_create(odph_odpthread_t *thread_tbl,
>> >
>> >
>> > My patch fixes this also.
>> >
>> >
>> > -Petri
>> >
>> >

Reply via email to