| Message ID | 20160418140003.GA23567@erachmi-ericsson.ki.sw.ericsson.se |
|---|---|
| State | New |
| Headers | show |
Similar changes can go to one patch. Like the same patches that change args for unit tests. Is that patches sequence buildable? I think we also need to send this to mailing list. But it's huge number of small patches which probably might be reasonable to split on 2 or 3 logical patch sets if that possible. Maxim. On 19.04.2016 10:19, Savolainen, Petri (Nokia - FI/Espoo) 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.milard@linaro.org] >> Sent: Monday, April 18, 2016 5:00 PM >> To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) >> <petri.savolainen@nokia.com> >> Cc: mike.holmes@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
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.savolainen@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.milard@linaro.org]
> > Sent: Monday, April 18, 2016 5:00 PM
> > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo)
> > <petri.savolainen@nokia.com>
> > Cc: mike.holmes@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
>
On 19.04.2016 10:40, Christophe Milard wrote: > 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? > odph_instance_type_t { THREAD_PTHREAD, THREAD_FORK } Maxim. > > On 19 April 2016 at 09:19, Savolainen, Petri (Nokia - FI/Espoo) > <petri.savolainen@nokia.com <mailto:petri.savolainen@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.milard@linaro.org > <mailto:christophe.milard@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.savolainen@nokia.com <mailto:petri.savolainen@nokia.com>> > > Cc: mike.holmes@linaro.org <mailto:mike.holmes@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
On 19 April 2016 at 09:39, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > Similar changes can go to one patch. Like the same patches that change > args for unit tests. > Not sure what you mean here. If you mean that the test changes could be gathered in one single patch I could possibly agree if these changes were the same. but they differ, sometime. (e.g. init has 3 main functions, pktio io has wrappers etc...). So ot make sense to have small separated patches to me. If you meant something else, please explain. > > Is that patches sequence buildable? > yes. > > I think we also need to send this to mailing list. But it's huge number of > small patches which probably might be reasonable > to split on 2 or 3 logical patch sets if that possible. > Well, I have an issue here: when I send an incomplete series, people complain they want to see the complete picture. If I send the complete picture people complain it is too many patches. If I send few large patches, some other complain they should be split ... :-) it is getting hard :-) > > Maxim. > > > On 19.04.2016 10:19, Savolainen, Petri (Nokia - FI/Espoo) 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.milard@linaro.org] >>> Sent: Monday, April 18, 2016 5:00 PM >>> To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) >>> <petri.savolainen@nokia.com> >>> Cc: mike.holmes@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 >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
On 19 April 2016 at 09:46, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 19.04.2016 10:40, Christophe Milard wrote: > >> 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? >> >> odph_instance_type_t > yes, "odph_instance_type_t" is shorter than "odph_thread_implementation_e" but less clear. As, in practical, only one enum value will be used as parameter to the function, I think "odph_thread_implementation_e" is better { > THREAD_PTHREAD, > THREAD_FORK > THREAD_PROCESS I guess, right? Christophe. > } > > Maxim. > >> >> On 19 April 2016 at 09:19, Savolainen, Petri (Nokia - FI/Espoo) < >> petri.savolainen@nokia.com <mailto:petri.savolainen@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.milard@linaro.org >> <mailto:christophe.milard@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.savolainen@nokia.com <mailto:petri.savolainen@nokia.com>> >> > Cc: mike.holmes@linaro.org <mailto:mike.holmes@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 >> > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/lng-odp >
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.savolainen@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.milard@linaro.org] > *Sent:* Tuesday, April 19, 2016 10:40 AM > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] > > Sent: Monday, April 18, 2016 5:00 PM > > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) > > <petri.savolainen@nokia.com> > > Cc: mike.holmes@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 > > >
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. On 19 April 2016 at 05:02, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@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.milard@linaro.org] > *Sent:* Tuesday, April 19, 2016 11:10 AM > > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] > *Sent:* Tuesday, April 19, 2016 10:40 AM > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] > > Sent: Monday, April 18, 2016 5:00 PM > > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) > > <petri.savolainen@nokia.com> > > Cc: mike.holmes@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"
On 19 April 2016 at 14:30, Mike Holmes <mike.holmes@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.savolainen@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.milard@linaro.org] >> *Sent:* Tuesday, April 19, 2016 11:10 AM >> >> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >> *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] >> *Sent:* Tuesday, April 19, 2016 10:40 AM >> *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> >> *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] >> > Sent: Monday, April 18, 2016 5:00 PM >> > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) >> > <petri.savolainen@nokia.com> >> > Cc: mike.holmes@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" > > >
Not sure I like the idea to write code that is not used. Everything that could be written in a platform agnostic way should be done this way. The presence of these process/threads specific functions may just suggest to use them. Nothing of what we have been doing so far needed this distinction. This can be looked at if this problem araise. Christophe. On 19 April 2016 at 11:02, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolainen@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.milard@linaro.org] > *Sent:* Tuesday, April 19, 2016 11:10 AM > > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] > *Sent:* Tuesday, April 19, 2016 10:40 AM > *To:* Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com> > *Cc:* lng-odp@lists.linaro.org; mike.holmes@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.savolainen@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.milard@linaro.org] > > Sent: Monday, April 18, 2016 5:00 PM > > To: lng-odp@lists.linaro.org; Savolainen, Petri (Nokia - FI/Espoo) > > <petri.savolainen@nokia.com> > > Cc: mike.holmes@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 > > > > >
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