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<mailto: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<mailto:christophe.mil...@linaro.org>] Sent: Tuesday, April 19, 2016 10:40 AM To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> Cc: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>; mike.hol...@linaro.org<mailto: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<mailto: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<mailto:christophe.mil...@linaro.org>] > Sent: Monday, April 18, 2016 5:00 PM > To: lng-odp@lists.linaro.org<mailto:lng-odp@lists.linaro.org>; Savolainen, > Petri (Nokia - FI/Espoo) > <petri.savolai...@nokia.com<mailto:petri.savolai...@nokia.com>> > Cc: mike.hol...@linaro.org<mailto: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
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
