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.
