On 19 April 2016 at 14:30, Mike Holmes <mike.hol...@linaro.org> wrote:
> I have a thought about odph_linux_odpthreads_create() and odph_linux_ > odpthreads_join() > > Should these be > > odph_odpthreads_create() and odph_odpthreads_join() > > > That way we create the notion of an odpthread that is OS independent and > then the tests possibly extend to another OS without modification. > > > +1 I'd be very happy to take these names!: my feeling is that we eventually want the helpers to hide these things. The linux helpers create ODP threads their way. the <whatever other OS> creates them their way. This "linux" should eventually be removed so that helpers becomes an absrtation of any OS. I am happy to do it now, if we can agree on that! Christophe > > On 19 April 2016 at 05:02, Savolainen, Petri (Nokia - FI/Espoo) < > petri.savolai...@nokia.com> wrote: > >> As I said, our test need to be thread agnostic (because we must test both >> model) but those helpers would be for other apps (that are not agnostic). >> Only pthread/process validation test cases would be needed (as any other >> helper code). Thread creation is so common task for any app that those are >> surely needed. Implementation of the agnostic and not agnostic functions >> may share the core parts of the code. >> >> >> >> -Petri >> >> >> >> >> >> *From:* EXT Christophe Milard [mailto:christophe.mil...@linaro.org] >> *Sent:* Tuesday, April 19, 2016 11:10 AM >> >> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com> >> *Cc:* lng-odp@lists.linaro.org; mike.hol...@linaro.org >> *Subject:* Re: [GIT PULL ODP] running things in process mode >> >> >> >> hmmm. >> >> OK. so you want to leave the old functions because they give the >> possibility to pass pointers to threads (and get pointer back), whereas >> processes would just return an int? >> >> (and by doing the change I did, I had to lower all these to the lowest >> common denominator, returning an int in both cases). >> >> Possibly makes sense. The question is really if we want to see that in >> the helper code, or if this is so rare so that the tests willing to do so >> can use fork and pthread directely... >> >> No platform agnostic test should be allowed to do so, actually. And the >> single platform dependent test we have today does nor create odpthreads. >> >> Not sure I really like the idea to leave unused code "just in case", but >> If I get your blessings for the lot except the last patch, I am an happy >> man. :-) >> >> We can delay and see for the "removing dead code", if you want >> >> >> >> If which case my proposal for a fifth parameter should be delayed as well. >> >> >> >> Christophe >> >> >> >> On 19 April 2016 at 09:58, Savolainen, Petri (Nokia - FI/Espoo) < >> petri.savolai...@nokia.com> wrote: >> >> Actually, it’s not the control of thread/process creation but the >> representation of those. With pthread create, user expects to give void >> *(*start)(void *) function pointer for thread starting point (with void* >> return type) and with processes there’s no entry point, but current thread >> of execution is forked. Also parameters are different (pid_t, status, >> pthread_attr_t, …) >> >> >> >> -Petri >> >> >> >> >> >> *From:* EXT Christophe Milard [mailto:christophe.mil...@linaro.org] >> *Sent:* Tuesday, April 19, 2016 10:40 AM >> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com> >> *Cc:* lng-odp@lists.linaro.org; mike.hol...@linaro.org >> *Subject:* Re: [GIT PULL ODP] running things in process mode >> >> >> >> Thanks for your comment, Petri. You are welcome to point out the coding >> guideline faults you have seen, so I don't miss them. I will review for the >> variable declaration position. >> >> Regarding your other comments, wouldn't it be better to add a fifth >> parameter to odph_linux_odpthreads_create() like this: >> >> >> >> +int odph_linux_odpthreads_create(odph_linux_odpthread_t *thread_tbl, >> >> + const odp_cpumask_t *mask, >> >> + int (*start_routine)(void *), void *arg, >> >> + odp_thread_type_t thr_type, >> >> + odph_thread_implementation_e imp); >> >> >> >> where odph_thread_implementation_e would be an enum: { >> >> ANY (i.e. helpers decide according to command line) >> >> FORCE_PTHREAD >> >> FORCE_PROCESS >> >> } >> >> >> >> platform agnostic tests would then only be allowed to use ANY. tests on >> the platform side could pick what they want. >> >> And we can remove the dead code. >> >> >> >> Make sense? >> >> >> >> >> >> On 19 April 2016 at 09:19, Savolainen, Petri (Nokia - FI/Espoo) < >> petri.savolai...@nokia.com> wrote: >> >> Hi, >> >> Looks reasonable. There are some style / coding guideline faults (like >> should introduce all variables in the beginning of a code block). Also pure >> process and pthread helpers should remain (suggesting to leave out "helper: >> removing dead code"), since not all apps want to obscure whether process or >> pthread is used. Opaque thread type helps our purposes to test everything >> on both pthread and process models, but other apps are likely to care only >> one model. >> >> -Petri >> >> >> >> >> > -----Original Message----- >> > From: EXT Christophe Milard [mailto:christophe.mil...@linaro.org] >> > Sent: Monday, April 18, 2016 5:00 PM >> > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) >> > <petri.savolai...@nokia.com> >> > Cc: mike.hol...@linaro.org >> > Subject: [GIT PULL ODP] running things in process mode >> > >> > Hi, >> > >> > This patch series adds the ability to run tests/ exemples / perf-test >> > in "process mode" (i.e letting OPD threads being linux processes) >> > It it hence tackling ODP-171. >> > >> > This is achieved in 2 main steps: >> > >> > A] >> > The 2 pairs of helper functions: >> > odph_linux_pthread_create(), odph_linux_pthread_join() >> > and >> > odph_linux_process_fork_n(), odph_linux_process_wait_n() >> > are replaced by: >> > odph_linux_odpthreads_create() and odph_linux_odpthreads_join() >> > The latter's callers are unaware of the actual implementation of the ODP >> > thread (making test truly platform agnostic). >> > The helper functions decide at run time whether an odp thread is to be >> > a linux process or pthread based on the command line argument. >> > >> > B] each test/example now calls a new helper function, >> > odph_linux_parse_options(), so that the helper can get its own arguments >> > out of the command line. >> > Currentely supported args are: --odph_proc, --odph_thread. >> > Defaults assumes thread. specifying both options runs in mixed mode. >> > >> > The changed are first done on the shmem tests, and thereafter propagated >> > to other helper users. >> > Note that this patch series enable the option but does not affect >> > make check at this time: make check still calls the tests with no >> options >> > which default to thread mode. >> > >> > This patch series nicely splits it two groups (you can split your review >> > there): >> > 1) up to "validation: pktio: adding command line argument parsing", the >> > new >> > helper functions are introduced, and used in the validation tests. >> > 2) from "helper: adding a function to merge getopt parameters" the >> ability >> > to parse command line arguments in subset in added and applied to >> > the example and performance tests. >> > >> > Hope this makes sence for you too! >> > >> > ---- >> > >> > The following changes since commit >> > 1f58a8fd51ab0304c0ae767e225abfec8448ac0b: >> > >> > validation: scheduler: correct pause/resume sequence (2016-04-18 >> > 15:03:22 +0300) >> > >> > are available in the git repository at: >> > >> > https://git.linaro.org/people/christophe.milard/odp.git >> > test_in_process_mode_v1 >> > >> > for you to fetch changes up to af9f80c6e9faaaed0c26b0825c25b5d5bb8ef028: >> > >> > helper: removing dead code (2016-04-18 15:25:48 +0200) >> > >> > ---------------------------------------------------------------- >> > Christophe Milard (56): >> > helpers: adding command line argument parsing >> > validation: common: adding command line argument parsing >> > validation: shmem: adding command line argument parsing >> > helpers: linux: creating common entry for process and thread >> > helpers: linux: creating functions to handle odpthreads >> > helper: test: adding odpthread functions tests >> > validation: using implementation agnostic function for ODP threads >> > validation: traffic_mngr: adding command line argument parsing >> > validation: timer: adding command line argument parsing >> > validation: time: adding command line argument parsing >> > validation: thread: adding command line argument parsing >> > validation: system: adding command line argument parsing >> > validation: std_clib: adding command line argument parsing >> > validation: scheduler: adding command line argument parsing >> > validation: random: adding command line argument parsing >> > validation: queue: adding command line argument parsing >> > validation: pool: adding command line argument parsing >> > validation: packet: adding command line argument parsing >> > validation: lock: adding command line argument parsing >> > validation: init: adding command line argument parsing >> > validation: hash: adding command line argument parsing >> > validation: errno: adding command line argument parsing >> > validation: crypto: adding command line argument parsing >> > validation: cpumask: adding command line argument parsing >> > validation: config: adding command line argument parsing >> > validation: classification: adding command line argument parsing >> > validation: buffer: adding command line argument parsing >> > validation: barrier: adding command line argument parsing >> > validation: atomic: adding command line argument parsing >> > validation: pktio: adding command line argument parsing >> > helper: adding a function to merge getopt parameters >> > helper: parsing the complete set of options >> > performance: odp_scheduling: proc mode done by helper >> > performance: odp_pktio_perf: using agnostic function for ODP >> threads >> > performance: odp_pktio_perf: adding helper cmd line parsing >> > performance: odp_l2fwd: using agnostic function for ODP threads >> > performance: odp_l2fwd: adding helper cmd line parsing >> > performance: crypto: using agnostic function for ODP threads >> > performance: crypto: adding helper cmd line parsing >> > example: classifier: using agnostic function for ODP threads >> > example: classifier: adding helper cmd line parsing >> > example: generator: using agnostic function for ODP threads >> > example: generator: adding helper cmd line parsing >> > example: ipsec: using agnostic function for ODP threads >> > example: ipsec: adding helper cmd line parsing >> > example: l2fwd_simple: using agnostic function for ODP threads >> > example: l2fwd_simple: adding helper cmd line parsing >> > example: pktio: using agnostic function for ODP threads >> > example: pktio: adding helper cmd line parsing >> > example: time: using agnostic function for ODP threads >> > example: time: adding helper cmd line parsing >> > example: timer: using agnostic function for ODP threads >> > example: timer: adding helper cmd line parsing >> > example: switch: using agnostic function for ODP threads >> > example: switch: adding helper cmd line parsing >> > helper: removing dead code >> > >> > example/classifier/odp_classifier.c | 39 +- >> > example/generator/odp_generator.c | 55 +-- >> > example/ipsec/odp_ipsec.c | 26 +- >> > example/l2fwd_simple/odp_l2fwd_simple.c | 47 ++- >> > example/packet/odp_pktio.c | 47 ++- >> > example/switch/odp_switch.c | 29 +- >> > example/time/time_global_test.c | 17 +- >> > example/timer/odp_timer_test.c | 26 +- >> > helper/include/odp/helper/linux.h | 170 +++++---- >> > helper/linux.c | 423 >> ++++++++++++++-- >> > ----- >> > helper/test/.gitignore | 3 +- >> > helper/test/Makefile.am | 14 +- >> > helper/test/{thread.c => odpthreads.c} | 30 +- >> > helper/test/odpthreads_as_mixed | 15 + >> > helper/test/odpthreads_as_processes | 15 + >> > helper/test/odpthreads_as_pthreads | 15 + >> > helper/test/process.c | 86 ----- >> > platform/linux-generic/test/pktio/pktio_run | 21 +- >> > platform/linux-generic/test/pktio/pktio_run_dpdk | 17 +- >> > platform/linux-generic/test/pktio/pktio_run_netmap | 5 +- >> > platform/linux-generic/test/pktio/pktio_run_pcap | 5 +- >> > platform/linux-generic/test/pktio/pktio_run_tap | 6 +- >> > test/performance/odp_crypto.c | 28 +- >> > test/performance/odp_l2fwd.c | 39 +- >> > test/performance/odp_pktio_perf.c | 36 +- >> > test/performance/odp_scheduling.c | 85 ++--- >> > test/validation/atomic/atomic.c | 40 +- >> > test/validation/atomic/atomic.h | 2 +- >> > test/validation/atomic/atomic_main.c | 4 +- >> > test/validation/barrier/barrier.c | 14 +- >> > test/validation/barrier/barrier.h | 2 +- >> > test/validation/barrier/barrier_main.c | 4 +- >> > test/validation/buffer/buffer.c | 10 +- >> > test/validation/buffer/buffer.h | 2 +- >> > test/validation/buffer/buffer_main.c | 4 +- >> > test/validation/classification/classification.c | 10 +- >> > test/validation/classification/classification.h | 2 +- >> > .../classification/classification_main.c | 4 +- >> > test/validation/common/odp_cunit_common.c | 24 +- >> > test/validation/common/odp_cunit_common.h | 6 +- >> > test/validation/config/config.c | 10 +- >> > test/validation/config/config.h | 2 +- >> > test/validation/config/config_main.c | 4 +- >> > test/validation/cpumask/cpumask.c | 10 +- >> > test/validation/cpumask/cpumask.h | 2 +- >> > test/validation/cpumask/cpumask_main.c | 4 +- >> > test/validation/crypto/crypto.c | 6 +- >> > test/validation/crypto/crypto.h | 2 +- >> > test/validation/crypto/crypto_main.c | 4 +- >> > test/validation/errno/errno.c | 10 +- >> > test/validation/errno/errno.h | 2 +- >> > test/validation/errno/errno_main.c | 4 +- >> > test/validation/hash/hash.c | 10 +- >> > test/validation/hash/hash.h | 2 +- >> > test/validation/hash/hash_main.c | 4 +- >> > test/validation/init/init.c | 18 +- >> > test/validation/init/init.h | 6 +- >> > test/validation/init/init_main_abort.c | 4 +- >> > test/validation/init/init_main_log.c | 4 +- >> > test/validation/init/init_main_ok.c | 4 +- >> > test/validation/lock/lock.c | 50 +-- >> > test/validation/lock/lock.h | 2 +- >> > test/validation/lock/lock_main.c | 4 +- >> > test/validation/packet/packet.c | 6 +- >> > test/validation/packet/packet.h | 2 +- >> > test/validation/packet/packet_main.c | 4 +- >> > test/validation/pktio/pktio.c | 10 +- >> > test/validation/pktio/pktio.h | 2 +- >> > test/validation/pktio/pktio_main.c | 4 +- >> > test/validation/pool/pool.c | 6 +- >> > test/validation/pool/pool.h | 2 +- >> > test/validation/pool/pool_main.c | 4 +- >> > test/validation/queue/queue.c | 6 +- >> > test/validation/queue/queue.h | 2 +- >> > test/validation/queue/queue_main.c | 4 +- >> > test/validation/random/random.c | 6 +- >> > test/validation/random/random.h | 2 +- >> > test/validation/random/random_main.c | 4 +- >> > test/validation/scheduler/scheduler.c | 14 +- >> > test/validation/scheduler/scheduler.h | 2 +- >> > test/validation/scheduler/scheduler_main.c | 4 +- >> > test/validation/shmem/shmem.c | 12 +- >> > test/validation/shmem/shmem.h | 2 +- >> > test/validation/shmem/shmem_main.c | 4 +- >> > test/validation/std_clib/std_clib.c | 6 +- >> > test/validation/std_clib/std_clib.h | 2 +- >> > test/validation/std_clib/std_clib_main.c | 4 +- >> > test/validation/system/system.c | 6 +- >> > test/validation/system/system.h | 2 +- >> > test/validation/system/system_main.c | 4 +- >> > test/validation/thread/thread.c | 10 +- >> > test/validation/thread/thread.h | 2 +- >> > test/validation/thread/thread_main.c | 4 +- >> > test/validation/time/time.c | 6 +- >> > test/validation/time/time.h | 2 +- >> > test/validation/time/time_main.c | 4 +- >> > test/validation/timer/timer.c | 10 +- >> > test/validation/timer/timer.h | 2 +- >> > test/validation/timer/timer_main.c | 4 +- >> > test/validation/traffic_mngr/traffic_mngr.c | 6 +- >> > test/validation/traffic_mngr/traffic_mngr.h | 2 +- >> > test/validation/traffic_mngr/traffic_mngr_main.c | 4 +- >> > 102 files changed, 1080 insertions(+), 697 deletions(-) >> > rename helper/test/{thread.c => odpthreads.c} (68%) >> > create mode 100755 helper/test/odpthreads_as_mixed >> > create mode 100755 helper/test/odpthreads_as_processes >> > create mode 100755 helper/test/odpthreads_as_pthreads >> > delete mode 100644 helper/test/process.c >> >> >> >> >> > > > > -- > Mike Holmes > Technical Manager - Linaro Networking Group > Linaro.org <http://www.linaro.org/> *│ *Open source software for ARM SoCs > "Work should be fun and collaborative, the rest follows" > > >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
