> -----Original Message----- > From: lng-odp [mailto:[email protected]] On Behalf Of Brian > Brooks > Sent: Thursday, May 12, 2016 6:02 PM > To: Christophe Milard <[email protected]> > Cc: [email protected]; Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> > Subject: Re: [lng-odp] [PATCHv6 05/38] helpers: linux: creating functions > to handle odpthreads > > On 05/12 13:57:52, Christophe Milard wrote: > > On 12 May 2016 at 13:33, Savolainen, Petri (Nokia - FI/Espoo) < > > [email protected]> wrote: > > > > > > > > *From:* lng-odp [mailto:[email protected]] *On Behalf > Of *Christophe > > > Milard > > > *Sent:* Thursday, May 12, 2016 2:14 PM > > > *To:* Savolainen, Petri (Nokia - FI/Espoo) > <[email protected]> > > > *Cc:* [email protected] > > > *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) < > > > [email protected]> 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.
My mail client (Outlook) actually adds '>', when the mail I'm replying is in plain text (as it should always be on this list). When people reply in HTML, my client does not cope with it. If I convert HTML mail back to plain text, it loses all alignments (defined in HTML of course). I have proposed this already couple of time before and I'll do it once again: Let's set lng-odp list to be PLAIN TEXT only. I think it's the only way to ensure readability of the list. Back to the patch series. Yes, this series should provide support for testing process mode. Thread mode should be still the default, and proc mode can be activated with the new option. Any proc/thread mixture setups, should be left for future as it needs more options anyway. The proposed hidden mixture mode, would surely bite someone someday. Helpers are used not only in validation tests, but in examples - which show example how application developers should setup things and use the APIs. From examples the same patterns (and helpers) will spread to external applications. So, helpers should be also well documented, intuitive and easy to use. -Petri _______________________________________________ lng-odp mailing list [email protected] https://lists.linaro.org/mailman/listinfo/lng-odp
