On 05/12 13:57:52, Christophe Milard wrote: > On 12 May 2016 at 13:33, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolai...@nokia.com> wrote: > > > > > *From:* lng-odp [mailto:lng-odp-boun...@lists.linaro.org] *On Behalf Of > > *Christophe > > Milard > > *Sent:* Thursday, May 12, 2016 2:14 PM > > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com> > > *Cc:* lng-odp@lists.linaro.org > > *Subject:* Re: [lng-odp] [PATCHv6 05/38] helpers: linux: creating > > functions to handle odpthreads > > > > On 12 May 2016 at 12:28, Savolainen, Petri (Nokia - FI/Espoo) < > > petri.savolai...@nokia.com> wrote: > > > > > > > +int odph_odpthreads_create(odph_odpthread_t *thread_tbl, > > > + const odp_cpumask_t *mask, > > > + const odph_odpthread_params_t *thr_params) > > > +{ > > > + int i; > > > + int num; > > > + int cpu_count; > > > + int cpu; > > > + > > > + num = odp_cpumask_count(mask); > > > + > > > + memset(thread_tbl, 0, num * sizeof(odph_odpthread_t)); > > > + > > > + cpu_count = odp_cpu_count(); > > > + > > > + if (num < 1 || num > cpu_count) { > > > + ODPH_ERR("Invalid number of odpthreads:%d" > > > + " (%d cores available)\n", > > > + num, cpu_count); > > > + return -1; > > > + } > > > + > > > + cpu = odp_cpumask_first(mask); > > > + for (i = 0; i < num; i++) { > > > + /* > > > + * Thread mode by default, or if both thread and proc mode > > > + * are required each second odpthread is a linux thread. > > > + */ > > > > This is a weird logic. All threads should be one type (when you have > > boolean cmd line params for thread type). Pthread should be the default, so > > all threads are pthreads if user don't give any param. It's an error if > > user gives both. > > > > > > > > This follows a requirement from you that the linux ODP implementation > > should support a mix of both ODP threads implemented as pthread and process > > at the same time. I felt that a "--odph_mixed" option is not more clear > > than specifying both the things we want, i.e. processes and thread. > > > > Any better suggestion? > > > > > > > > You’d need more detailed information (than Boolean) for that advanced use > > case. At this point, all pthreads or all processes is sufficient for > > odp-linux. I have not required implementation of thread/process mix, but > > required that ODP API does not prevent different threading models. > > > > > > > > Create all threads by default, all processes if helper_options.proc is > > true and report error is both are true. > > > > > > > > if (helper_options.proc && helper_options.thrd) > > > > return -1; > > > > > > > > if (helper_options.proc) { > > > > odph_linux_process_create(); > > > > } else { > > > > odph_linux_thread_create(); > > > > } > > > > Now, I am getting very confused... I thought we agreed that support for > > both process mode and thread mode at the same time was needed. I actually > > remember asking that specific question at some arch call and the answer > > being positive: And it made sense!: If ODP threads are OS objects created > > by the application directly towards the OS, how would you expect ODP to > > prevent an app to mix pthreads and fork?. If we cannot prevent it, don't we > > have to support it? > > > > > > > > Shall I really remove support for the mixed mode from the patch series? > > > > > > > > > > > > There is no need for _*helper*_ to setup every second thread as process > > and every second as pthread. If an application needs mixed mode, it can set > > it up without a helper. We just need basic stuff working first (all > > processes). More flexible setup would need more params: “create 1 process > > with 3 additional pthread, and a another 2 processes with 2 additional > > pthreads each, and …” > > > > But wasn't the point of this patch series to be able to run > test/examples/perf test ... in all suported modes? >
I admit I found it bizarre at first too, but it shows how tricky the multi- process use case can be for such a library. This patch series simply enables existing binaries, which make use of the ODP APIs, to run in either multi-threaded, multi-process, or a mixture of both environments. Blasphemy! But, the original multi-threaded behavior is preserved *and* it has proven to be a good catalyst for making further progress. It seems like we're identifying behavior which would become a follow-up to this patch series. I don't think we will be able to nail down all of the requirements, bang out all of the code changes, and get it perfect in this thread. But, we can take note of what needs to change and work towards it. Petri, it's hard to read your mail in this thread because it appears the email client you're using does not use the '> ' prefix. Can you double check this setting? It will help others follow you. _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
