On 19 April 2016 at 09:46, Maxim Uvarov <maxim.uva...@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.savolai...@nokia.com <mailto:petri.savolai...@nokia.com>> wrote:
>>
>> Hi,
>>
>> Looks reasonable. There are some style / coding guideline faults
>> (like should introduce all variables in the beginning of a code
>> block). Also pure process and pthread helpers should remain
>> (suggesting to leave out "helper: removing dead code"), since not
>> all apps want to obscure whether process or pthread is used.
>> Opaque thread type helps our purposes to test everything on both
>> pthread and process models, but other apps are likely to care only
>> one model.
>>
>> -Petri
>>
>>
>>
>> > -----Original Message-----
>> > From: EXT Christophe Milard [mailto:christophe.mil...@linaro.org
>> <mailto:christophe.mil...@linaro.org>]
>> > Sent: Monday, April 18, 2016 5:00 PM
>> > To: lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org>;
>> Savolainen, Petri (Nokia - FI/Espoo)
>> > <petri.savolai...@nokia.com <mailto:petri.savolai...@nokia.com>>
>> > Cc: mike.hol...@linaro.org <mailto:mike.hol...@linaro.org>
>> > Subject: [GIT PULL ODP] running things in process mode
>> >
>> > Hi,
>> >
>> > This patch series adds the ability to run tests/ exemples /
>> perf-test
>> > in "process mode" (i.e letting OPD threads being linux processes)
>> > It it hence tackling ODP-171.
>> >
>> > This is achieved in 2 main steps:
>> >
>> > A]
>> > The 2 pairs of helper functions:
>> > odph_linux_pthread_create(), odph_linux_pthread_join()
>> > and
>> > odph_linux_process_fork_n(), odph_linux_process_wait_n()
>> > are replaced by:
>> > odph_linux_odpthreads_create() and odph_linux_odpthreads_join()
>> > The latter's callers are unaware of the actual implementation of
>> the ODP
>> > thread (making test truly platform agnostic).
>> > The helper functions decide at run time whether an odp thread is
>> to be
>> > a linux process or pthread based on the command line argument.
>> >
>> > B] each test/example now calls a new helper function,
>> > odph_linux_parse_options(), so that the helper can get its own
>> arguments
>> > out of the command line.
>> > Currentely supported args are: --odph_proc, --odph_thread.
>> > Defaults assumes thread. specifying both options runs in mixed mode.
>> >
>> > The changed are first done on the shmem tests, and thereafter
>> propagated
>> > to other helper users.
>> > Note that this patch series enable the option but does not affect
>> > make check at this time: make check still calls the tests with
>> no options
>> > which default to thread mode.
>> >
>> > This patch series nicely splits it two groups (you can split
>> your review
>> > there):
>> > 1) up to "validation: pktio: adding command line argument
>> parsing", the
>> > new
>> > helper functions are introduced, and used in the validation tests.
>> > 2) from "helper: adding a function to merge getopt parameters"
>> the ability
>> > to parse command line arguments in subset in added and applied to
>> > the example and performance tests.
>> >
>> > Hope this makes sence for you too!
>> >
>> > ----
>> >
>> > The following changes since commit
>> > 1f58a8fd51ab0304c0ae767e225abfec8448ac0b:
>> >
>> > validation: scheduler: correct pause/resume sequence (2016-04-18
>> > 15:03:22 +0300)
>> >
>> > are available in the git repository at:
>> >
>> > https://git.linaro.org/people/christophe.milard/odp.git
>> > test_in_process_mode_v1
>> >
>> > for you to fetch changes up to
>> af9f80c6e9faaaed0c26b0825c25b5d5bb8ef028:
>> >
>> > helper: removing dead code (2016-04-18 15:25:48 +0200)
>> >
>> > ----------------------------------------------------------------
>> > Christophe Milard (56):
>> > helpers: adding command line argument parsing
>> > validation: common: adding command line argument parsing
>> > validation: shmem: adding command line argument parsing
>> > helpers: linux: creating common entry for process and thread
>> > helpers: linux: creating functions to handle odpthreads
>> > helper: test: adding odpthread functions tests
>> > validation: using implementation agnostic function for ODP
>> threads
>> > validation: traffic_mngr: adding command line argument parsing
>> > validation: timer: adding command line argument parsing
>> > validation: time: adding command line argument parsing
>> > validation: thread: adding command line argument parsing
>> > validation: system: adding command line argument parsing
>> > validation: std_clib: adding command line argument parsing
>> > validation: scheduler: adding command line argument parsing
>> > validation: random: adding command line argument parsing
>> > validation: queue: adding command line argument parsing
>> > validation: pool: adding command line argument parsing
>> > validation: packet: adding command line argument parsing
>> > validation: lock: adding command line argument parsing
>> > validation: init: adding command line argument parsing
>> > validation: hash: adding command line argument parsing
>> > validation: errno: adding command line argument parsing
>> > validation: crypto: adding command line argument parsing
>> > validation: cpumask: adding command line argument parsing
>> > validation: config: adding command line argument parsing
>> > validation: classification: adding command line argument
>> parsing
>> > validation: buffer: adding command line argument parsing
>> > validation: barrier: adding command line argument parsing
>> > validation: atomic: adding command line argument parsing
>> > validation: pktio: adding command line argument parsing
>> > helper: adding a function to merge getopt parameters
>> > helper: parsing the complete set of options
>> > performance: odp_scheduling: proc mode done by helper
>> > performance: odp_pktio_perf: using agnostic function for
>> ODP threads
>> > performance: odp_pktio_perf: adding helper cmd line parsing
>> > performance: odp_l2fwd: using agnostic function for ODP
>> threads
>> > performance: odp_l2fwd: adding helper cmd line parsing
>> > performance: crypto: using agnostic function for ODP threads
>> > performance: crypto: adding helper cmd line parsing
>> > example: classifier: using agnostic function for ODP threads
>> > example: classifier: adding helper cmd line parsing
>> > example: generator: using agnostic function for ODP threads
>> > example: generator: adding helper cmd line parsing
>> > example: ipsec: using agnostic function for ODP threads
>> > example: ipsec: adding helper cmd line parsing
>> > example: l2fwd_simple: using agnostic function for ODP threads
>> > example: l2fwd_simple: adding helper cmd line parsing
>> > example: pktio: using agnostic function for ODP threads
>> > example: pktio: adding helper cmd line parsing
>> > example: time: using agnostic function for ODP threads
>> > example: time: adding helper cmd line parsing
>> > example: timer: using agnostic function for ODP threads
>> > example: timer: adding helper cmd line parsing
>> > example: switch: using agnostic function for ODP threads
>> > example: switch: adding helper cmd line parsing
>> > helper: removing dead code
>> >
>> > example/classifier/odp_classifier.c | 39 +-
>> > example/generator/odp_generator.c | 55 +--
>> > example/ipsec/odp_ipsec.c | 26 +-
>> > example/l2fwd_simple/odp_l2fwd_simple.c | 47 ++-
>> > example/packet/odp_pktio.c | 47 ++-
>> > example/switch/odp_switch.c | 29 +-
>> > example/time/time_global_test.c | 17 +-
>> > example/timer/odp_timer_test.c | 26 +-
>> > helper/include/odp/helper/linux.h | 170 +++++----
>> > helper/linux.c | 423 ++++++++++++++--
>> > -----
>> > helper/test/.gitignore | 3 +-
>> > helper/test/Makefile.am | 14 +-
>> > helper/test/{thread.c => odpthreads.c} | 30 +-
>> > helper/test/odpthreads_as_mixed | 15 +
>> > helper/test/odpthreads_as_processes | 15 +
>> > helper/test/odpthreads_as_pthreads | 15 +
>> > helper/test/process.c | 86 -----
>> > platform/linux-generic/test/pktio/pktio_run | 21 +-
>> > platform/linux-generic/test/pktio/pktio_run_dpdk | 17 +-
>> > platform/linux-generic/test/pktio/pktio_run_netmap | 5 +-
>> > platform/linux-generic/test/pktio/pktio_run_pcap | 5 +-
>> > platform/linux-generic/test/pktio/pktio_run_tap | 6 +-
>> > test/performance/odp_crypto.c | 28 +-
>> > test/performance/odp_l2fwd.c | 39 +-
>> > test/performance/odp_pktio_perf.c | 36 +-
>> > test/performance/odp_scheduling.c | 85 ++---
>> > test/validation/atomic/atomic.c | 40 +-
>> > test/validation/atomic/atomic.h | 2 +-
>> > test/validation/atomic/atomic_main.c | 4 +-
>> > test/validation/barrier/barrier.c | 14 +-
>> > test/validation/barrier/barrier.h | 2 +-
>> > test/validation/barrier/barrier_main.c | 4 +-
>> > test/validation/buffer/buffer.c | 10 +-
>> > test/validation/buffer/buffer.h | 2 +-
>> > test/validation/buffer/buffer_main.c | 4 +-
>> > test/validation/classification/classification.c | 10 +-
>> > test/validation/classification/classification.h | 2 +-
>> > .../classification/classification_main.c | 4 +-
>> > test/validation/common/odp_cunit_common.c | 24 +-
>> > test/validation/common/odp_cunit_common.h | 6 +-
>> > test/validation/config/config.c | 10 +-
>> > test/validation/config/config.h | 2 +-
>> > test/validation/config/config_main.c | 4 +-
>> > test/validation/cpumask/cpumask.c | 10 +-
>> > test/validation/cpumask/cpumask.h | 2 +-
>> > test/validation/cpumask/cpumask_main.c | 4 +-
>> > test/validation/crypto/crypto.c | 6 +-
>> > test/validation/crypto/crypto.h | 2 +-
>> > test/validation/crypto/crypto_main.c | 4 +-
>> > test/validation/errno/errno.c | 10 +-
>> > test/validation/errno/errno.h | 2 +-
>> > test/validation/errno/errno_main.c | 4 +-
>> > test/validation/hash/hash.c | 10 +-
>> > test/validation/hash/hash.h | 2 +-
>> > test/validation/hash/hash_main.c | 4 +-
>> > test/validation/init/init.c | 18 +-
>> > test/validation/init/init.h | 6 +-
>> > test/validation/init/init_main_abort.c | 4 +-
>> > test/validation/init/init_main_log.c | 4 +-
>> > test/validation/init/init_main_ok.c | 4 +-
>> > test/validation/lock/lock.c | 50 +--
>> > test/validation/lock/lock.h | 2 +-
>> > test/validation/lock/lock_main.c | 4 +-
>> > test/validation/packet/packet.c | 6 +-
>> > test/validation/packet/packet.h | 2 +-
>> > test/validation/packet/packet_main.c | 4 +-
>> > test/validation/pktio/pktio.c | 10 +-
>> > test/validation/pktio/pktio.h | 2 +-
>> > test/validation/pktio/pktio_main.c | 4 +-
>> > test/validation/pool/pool.c | 6 +-
>> > test/validation/pool/pool.h | 2 +-
>> > test/validation/pool/pool_main.c | 4 +-
>> > test/validation/queue/queue.c | 6 +-
>> > test/validation/queue/queue.h | 2 +-
>> > test/validation/queue/queue_main.c | 4 +-
>> > test/validation/random/random.c | 6 +-
>> > test/validation/random/random.h | 2 +-
>> > test/validation/random/random_main.c | 4 +-
>> > test/validation/scheduler/scheduler.c | 14 +-
>> > test/validation/scheduler/scheduler.h | 2 +-
>> > test/validation/scheduler/scheduler_main.c | 4 +-
>> > test/validation/shmem/shmem.c | 12 +-
>> > test/validation/shmem/shmem.h | 2 +-
>> > test/validation/shmem/shmem_main.c | 4 +-
>> > test/validation/std_clib/std_clib.c | 6 +-
>> > test/validation/std_clib/std_clib.h | 2 +-
>> > test/validation/std_clib/std_clib_main.c | 4 +-
>> > test/validation/system/system.c | 6 +-
>> > test/validation/system/system.h | 2 +-
>> > test/validation/system/system_main.c | 4 +-
>> > test/validation/thread/thread.c | 10 +-
>> > test/validation/thread/thread.h | 2 +-
>> > test/validation/thread/thread_main.c | 4 +-
>> > test/validation/time/time.c | 6 +-
>> > test/validation/time/time.h | 2 +-
>> > test/validation/time/time_main.c | 4 +-
>> > test/validation/timer/timer.c | 10 +-
>> > test/validation/timer/timer.h | 2 +-
>> > test/validation/timer/timer_main.c | 4 +-
>> > test/validation/traffic_mngr/traffic_mngr.c | 6 +-
>> > test/validation/traffic_mngr/traffic_mngr.h | 2 +-
>> > test/validation/traffic_mngr/traffic_mngr_main.c | 4 +-
>> > 102 files changed, 1080 insertions(+), 697 deletions(-)
>> > rename helper/test/{thread.c => odpthreads.c} (68%)
>> > create mode 100755 helper/test/odpthreads_as_mixed
>> > create mode 100755 helper/test/odpthreads_as_processes
>> > create mode 100755 helper/test/odpthreads_as_pthreads
>> > delete mode 100644 helper/test/process.c
>>
>>
>>
>>
>> _______________________________________________
>> lng-odp mailing list
>> lng-odp@lists.linaro.org
>> https://lists.linaro.org/mailman/listinfo/lng-odp
>>
>
> _______________________________________________
> 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
