ok, thanks. On 17 February 2017 at 12:39, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia-bell-labs.com> wrote:
> > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > Maxim > > Uvarov > > Sent: Thursday, February 16, 2017 10:55 PM > > To: lng-odp@lists.linaro.org > > Subject: Re: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn to > > linux helpers > > > > On 02/16/17 11:51, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > > > > >> -----Original Message----- > > >> From: Savolainen, Petri (Nokia - FI/Espoo) > > >> Sent: Monday, February 13, 2017 5:10 PM > > >> To: 'Mike Holmes' <mike.hol...@linaro.org> > > >> Cc: lng-odp <lng-odp@lists.linaro.org> > > >> Subject: RE: [lng-odp] [PATCH 1/2] helper: linux: renamed threads_extn > > to > > >> linux helpers > > >> > > >> > > >> > > >> 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 > > >> > > > > > > Could we merge this clean up patch. It does not belong to discussions > > about: > > > * what is platform > > > * cloud profile > > > * abstract threads > > > > > > All those can be discussed and done on other mail threads and patches. > > This patch just explicitly defines that pthread/process functions depend > > on Linux. Application is free to use those or not. Similarly to e.g. > > cuckoo hash helper, an application is free to use cuckoo or not. > > > > > > Currently the code in the repo (breaks the build and) hints that > > pthread/process functions do not depend on Linux, but on an ODP > > implementation which is wrong. Abstract thread helper may depend on > > implementation, but these pthread/process functions here are not abstract > > - they use Linux. > > > > > > -Petri > > > > > > > Ok, I applied 1/2. 2/2 has rejects. Can you please rebase it? > > > > Maxim. > > 2/2 is not needed anymore. It fixed missing doxygen documentation, but > those were fixed since by other patch. > > -Petri > >
