| Message ID | 1418078986-24980-4-git-send-email-ola.liljedahl@linaro.org |
|---|---|
| State | New |
| Headers | show |
Has checkpatch issues
Using patch:
/home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox
git am
/home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox
Patch applied, building...
WARNING: line over 80 characters
#238: FILE: test/validation/odp_timer.c:185:
+ CU_FAIL("Failed to set timer (tooearly/toolate)");
total: 0 errors, 1 warnings, 0 checks, 359 lines checked
NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS
0001-test-odp_timer.h-cunit-test.patch has style problems, please review.
On 8 December 2014 at 17:49, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:
> Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org>
> ---
> (This document/code contribution attached is provided under the terms of
> agreement LES-LTM-21309)
> A new cunit test program test/validation/odp_timer.c for the updated timer
> API.
>
> test/validation/.gitignore | 1 +
> test/validation/Makefile.am | 4 +-
> test/validation/odp_timer.c | 336
> ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 340 insertions(+), 1 deletion(-)
> create mode 100644 test/validation/odp_timer.c
>
> diff --git a/test/validation/.gitignore b/test/validation/.gitignore
> index 37e2594..586def0 100644
> --- a/test/validation/.gitignore
> +++ b/test/validation/.gitignore
> @@ -4,3 +4,4 @@ odp_init
> odp_queue
> odp_crypto
> odp_shm
> +odp_timer
> diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am
> index 8547085..043bf4c 100644
> --- a/test/validation/Makefile.am
> +++ b/test/validation/Makefile.am
> @@ -6,13 +6,14 @@ AM_LDFLAGS += -static
> if ODP_CUNIT_ENABLED
> TESTS = ${bin_PROGRAMS}
> check_PROGRAMS = ${bin_PROGRAMS}
> -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm
> +bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_timer
> odp_init_LDFLAGS = $(AM_LDFLAGS)
> odp_queue_LDFLAGS = $(AM_LDFLAGS)
> odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto
> odp_crypto_LDFLAGS = $(AM_LDFLAGS)
> odp_shm_CFLAGS = $(AM_CFLAGS)
> odp_shm_LDFLAGS = $(AM_LDFLAGS)
> +odp_timer_LDFLAGS = $(AM_LDFLAGS)
> endif
>
> dist_odp_init_SOURCES = odp_init.c
> @@ -22,3 +23,4 @@ dist_odp_crypto_SOURCES =
> crypto/odp_crypto_test_async_inp.c \
> crypto/odp_crypto_test_rng.c \
> odp_crypto.c common/odp_cunit_common.c
> dist_odp_shm_SOURCES = odp_shm.c common/odp_cunit_common.c
> +dist_odp_timer_SOURCES = odp_timer.c common/odp_cunit_common.c
> diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c
> new file mode 100644
> index 0000000..4b6b872
> --- /dev/null
> +++ b/test/validation/odp_timer.c
> @@ -0,0 +1,336 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +/**
> + * @file
> + */
> +
> +#include <assert.h>
> +#include <unistd.h>
> +#include <odp.h>
> +#include <odp_timer.h>
> +#include <odph_linux.h>
> +#include <odph_chksum.h>
> +#include "odp_cunit_common.h"
> +
> +/** @private Timeout range in milliseconds (ms) */
> +#define RANGE_MS 2000
> +
> +/** @private Number of timers per thread */
> +#define NTIMERS 2000
> +
> +/** @private Timeout pool size per thread */
> +#define TMO_POOL_SIZE (512 * NTIMERS)
> +
> +/** @private Barrier for thread synchronisation */
> +static odp_barrier_t test_barrier;
> +
> +/** @private Timeout buffer pool handle used by all threads */
> +static odp_buffer_pool_t tbp;
> +
> +/** @private Timer pool handle used by all threads */
> +static odp_timer_pool_t tp;
> +
> +/** @private min() function */
> +static int min(int a, int b)
> +{
> + return a < b ? a : b;
> +}
> +
> +/* @private Timer helper structure */
> +struct test_timer {
> + odp_timer_t tim; /* Timer handle */
> + odp_buffer_t buf; /* Timeout buffer */
> + odp_buffer_t buf2; /* Copy of buffer handle */
> + uint64_t tick; /* Expiration tick or ODP_TICK_INVALID */
> +};
> +
> +/* @private Handle a received (timeout) buffer */
> +static void handle_tmo(odp_buffer_t buf, bool stale, uint64_t prev_tick)
> +{
> + odp_timer_t tim = ODP_TIMER_INVALID;
> + uint64_t tick = ODP_TICK_INVALID;
> + struct test_timer *ttp = NULL;
> +
> + /* Use assert() for correctness check of test program itself */
> + assert(buf != ODP_BUFFER_INVALID);
> + if (!odp_timer_tmo_metadata(buf, &tim, &tick, (void **)&ttp)) {
> + /* Not a default timeout buffer */
> + CU_FAIL("Unexpected buffer type received");
> + return;
> + }
> +
> + if (tim == ODP_TIMER_INVALID)
> + CU_FAIL("odp_timer_tmo_metadata() invalid timer");
> + if (tick == ODP_TICK_INVALID)
> + CU_FAIL("odp_timer_tmo_metadata() invalid tick");
> + if (ttp == NULL)
> + CU_FAIL("odp_timer_tmo_metadata() null user ptr");
> +
> + if (ttp->buf2 != buf)
> + CU_FAIL("odp_timer_tmo_metadata() wrong user ptr");
> + if (ttp->tim != tim)
> + CU_FAIL("odp_timer_tmo_metadata() wrong timer");
> + if (stale) {
> + /* Stale timeout => timer must have invalid tick */
> + if (ttp->tick != ODP_TICK_INVALID)
> + CU_FAIL("Stale timeout for active timer");
> + } else {
> + /* Fresh timeout => timer must have matching tick */
> + if (ttp->tick != tick)
> + CU_FAIL("odp_timer_tmo_metadata() wrong tick");
> + /* Check that timeout was delivered 'timely' */
> + if (tick > odp_timer_current_tick(tp))
> + CU_FAIL("Timeout delivered too early");
> + if (tick < prev_tick)
> + CU_FAIL("Timeout delivered too late");
> + }
> +
> + /* Use assert() for correctness check of test program itself */
> + assert(ttp->buf == ODP_BUFFER_INVALID);
> + ttp->buf = buf;
> +}
> +
> +/* @private Worker thread entrypoint which performs timer
> alloc/set/cancel/free
> + * tests */
> +static void *worker_entrypoint(void *arg)
> +{
> + int thr = odp_thread_id();
> + uint32_t i;
> + unsigned seed = thr;
> + (void)arg;
> +
> + odp_queue_t queue = odp_queue_create("timer_queue",
> + ODP_QUEUE_TYPE_POLL,
> + NULL);
> + if (queue == ODP_QUEUE_INVALID)
> + CU_FAIL_FATAL("Queue create failed");
> +
> + struct test_timer *tt = malloc(sizeof(struct test_timer) *
> NTIMERS);
> + if (tt == NULL)
> + perror("malloc"), abort();
> +
> + /* Prepare all timers */
> + for (i = 0; i < NTIMERS; i++) {
> + tt[i].tim = odp_timer_alloc(tp, queue, &tt[i]);
> + if (tt[i].tim == ODP_TIMER_INVALID)
> + CU_FAIL_FATAL("Failed to allocate timer");
> + tt[i].buf = odp_buffer_alloc(tbp);
> + if (tt[i].buf == ODP_BUFFER_INVALID)
> + CU_FAIL_FATAL("Failed to allocate timeout buffer");
> + tt[i].buf2 = tt[i].buf;
> + tt[i].tick = ODP_TICK_INVALID;
> + }
> +
> + odp_barrier_wait(&test_barrier);
> +
> + /* Initial set all timers with a random expiration time */
> + uint32_t nset = 0;
> + for (i = 0; i < NTIMERS; i++) {
> + uint64_t tck = odp_timer_current_tick(tp) + 1 +
> + odp_timer_ns_to_tick(tp,
> + (rand_r(&seed) %
> RANGE_MS)
> + * 1000000ULL);
> + tt[i].tick = odp_timer_set_abs(tt[i].tim, tck, &tt[i].buf);
> + uint64_t rc = tt[i].tick;
> + if (rc == ODP_TICK_TOOEARLY ||
> + rc == ODP_TICK_TOOLATE ||
> + rc == ODP_TICK_INVALID) {
> + CU_FAIL("Failed to set timer");
> + }
> + nset++;
> + }
> +
> + /* Step through wall time, 1ms at a time and check for expired
> timers */
> + uint32_t nrcv = 0;
> + uint32_t nreset = 0;
> + uint32_t ncancel = 0;
> + uint32_t ntoolate = 0;
> + uint32_t ms;
> + uint64_t prev_tick = odp_timer_current_tick(tp);
> + for (ms = 0; ms < 7 * RANGE_MS / 10; ms++) {
> + odp_buffer_t buf;
> + while ((buf = odp_queue_deq(queue)) != ODP_BUFFER_INVALID)
> {
> + handle_tmo(buf, false, prev_tick - 1);
> + nrcv++;
> + }
> + prev_tick = odp_timer_current_tick(tp);
> + i = rand_r(&seed) % NTIMERS;
> + if (tt[i].buf == ODP_BUFFER_INVALID &&
> + (rand_r(&seed) % 2 == 0)) {
> + /* Timer active, cancel it */
> + tt[i].tick = odp_timer_cancel(tt[i].tim,
> &tt[i].buf);
> + if (tt[i].buf == ODP_BUFFER_INVALID) {
> + /* Cancel failed, timer already expired */
> + ntoolate++;
> + }
> + ncancel++;
> + } else {
> + if (tt[i].buf != ODP_BUFFER_INVALID)
> + /* Timer inactive => set */
> + nset++;
> + else
> + /* Timer active => reset */
> + nreset++;
> + uint64_t tck = 1 + odp_timer_ns_to_tick(tp,
> + (rand_r(&seed) % RANGE_MS) *
> 1000000ULL);
> + tt[i].tick = odp_timer_set_rel(tt[i].tim, tck,
> + &tt[i].buf);
> + uint64_t rc = tt[i].tick;
> + if (rc == ODP_TICK_TOOEARLY ||
> + rc == ODP_TICK_TOOLATE) {
> + CU_FAIL("Failed to set timer
> (tooearly/toolate)");
> + } else if (rc == ODP_TICK_INVALID) {
> + /* Reset failed, timer already expired */
> + ntoolate++;
> + }
> + }
> + if (usleep(1000/*1ms*/) < 0)
> + perror("usleep"), abort();
> + }
> +
> + /* Free (including cancel) all timers */
> + uint32_t nstale = 0;
> + for (i = 0; i < NTIMERS; i++) {
> + tt[i].tick = odp_timer_free(tt[i].tim, &tt[i].buf);
> + if (tt[i].buf == ODP_BUFFER_INVALID)
> + /* Cancel/free too late, timer already expired and
> + * timoeut buffer enqueued */
> + nstale++;
> + }
> +
> + printf("Thread %u: %u timers set\n", thr, nset);
> + printf("Thread %u: %u timers reset\n", thr, nreset);
> + printf("Thread %u: %u timers cancelled\n", thr, ncancel);
> + printf("Thread %u: %u timers reset/cancelled too late\n",
> + thr, ntoolate);
> + printf("Thread %u: %u timeouts received\n", thr, nrcv);
> + printf("Thread %u: %u stale timeout(s) after odp_timer_free()\n",
> + thr, nstale);
> +
> + /* Delay some more to ensure timeouts for expired timers can be
> + * received */
> + usleep(1000/*1ms*/);
> + while (nstale != 0) {
> + odp_buffer_t buf = odp_queue_deq(queue);
> + if (buf != ODP_BUFFER_INVALID) {
> + handle_tmo(buf, true, 0/*Dont' care for stale
> tmo's*/);
> + nstale--;
> + } else {
> + CU_FAIL("Failed to receive stale timeout");
> + break;
> + }
> + }
> + /* Check if there any more (unexpected) buffers */
> + odp_buffer_t buf = odp_queue_deq(queue);
> + if (buf != ODP_BUFFER_INVALID)
> + CU_FAIL("Unexpected buffer received");
> +
> + printf("Thread %u: exiting\n", thr);
> + return NULL;
> +}
> +
> +/* @private Timer test case entrypoint */
> +static void test_odp_timer_all(void)
> +{
> + odp_shm_t shm;
> + int num_workers = min(odp_sys_core_count(), MAX_WORKERS);
> +
> + /* Create timeout buffer pools */
> + shm = odp_shm_reserve("tmo_bufs",
> + TMO_POOL_SIZE * num_workers,
> + ODP_CACHE_LINE_SIZE, 0);
> + if (shm == ODP_SHM_INVALID)
> + CU_FAIL_FATAL("Timeout shared memory alloc failed");
> +
> + tbp = odp_buffer_pool_create("tmo_pool", odp_shm_addr(shm),
> + TMO_POOL_SIZE * num_workers,
> + 0,
> + ODP_CACHE_LINE_SIZE,
> + ODP_BUFFER_TYPE_TIMEOUT);
> + if (tbp == ODP_BUFFER_POOL_INVALID)
> + CU_FAIL_FATAL("Timeout buffer pool create failed");
> +
> +#define NAME "timer_pool"
> +#define RES (10 * ODP_TIME_MSEC / 3)
> +#define MIN (10 * ODP_TIME_MSEC / 3)
> +#define MAX (1000000 * ODP_TIME_MSEC)
> + /* Create a timer pool */
> + tp = odp_timer_pool_create(NAME, tbp,
> + RES, MIN, MAX,
> + num_workers * NTIMERS,
> + true, ODP_CLOCK_CPU);
> + if (tp == ODP_TIMER_POOL_INVALID)
> + CU_FAIL_FATAL("Timer pool create failed");
> +
> + /* Start all created timer pools */
> + odp_timer_pool_start();
> +
> + odp_timer_pool_info_t tpinfo;
> + size_t sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo));
> + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1)
> + CU_FAIL("odp_timer_pool_info");
> + CU_ASSERT(strcmp(tpinfo.name, NAME) == 0);
> + CU_ASSERT(tpinfo.resolution == RES);
> + CU_ASSERT(tpinfo.min_tmo == odp_timer_ns_to_tick(tp, MIN));
> + CU_ASSERT(tpinfo.max_tmo == odp_timer_ns_to_tick(tp, MAX));
> + printf("Timer pool\n");
> + printf("----------\n");
> + printf(" name: %s\n", tpinfo.name);
> + printf(" resolution: %"PRIu64" ns (%"PRIu64" us)\n",
> + tpinfo.resolution, tpinfo.resolution / 1000);
> + printf(" min tmo: %"PRIu64" tick(s)\n", tpinfo.min_tmo);
> + printf(" max tmo: %"PRIu64" ticks\n", tpinfo.max_tmo);
> + printf("\n");
> +
> + printf("#timers..: %u\n", NTIMERS);
> + printf("Tmo range: %u ms (%"PRIu64" ticks)\n", RANGE_MS,
> + odp_timer_ns_to_tick(tp, 1000000ULL * RANGE_MS));
> + printf("\n");
> +
> + uint64_t tick;
> + for (tick = 0; tick < 1000000000000ULL; tick += 1000000ULL) {
> + uint64_t ns = odp_timer_tick_to_ns(tp, tick);
> + uint64_t t2 = odp_timer_ns_to_tick(tp, ns);
> + if (tick != t2)
> + CU_FAIL("Invalid conversion tick->ns->tick");
> + }
> +
> + /* Initialize barrier used by worker threads for synchronization */
> + odp_barrier_init(&test_barrier, num_workers);
> +
> + /* Create and start worker threads */
> + pthrd_arg thrdarg;
> + thrdarg.testcase = 0;
> + thrdarg.numthrds = num_workers;
> + odp_cunit_thread_create(worker_entrypoint, &thrdarg);
> +
> + /* Wait for worker threads to exit */
> + odp_cunit_thread_exit(&thrdarg);
> +
> + /* Check some statistics after the test */
> + sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo));
> + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1)
> + CU_FAIL("odp_timer_pool_info");
> + CU_ASSERT(tpinfo.num_timers == (unsigned)num_workers * NTIMERS);
> + CU_ASSERT(tpinfo.cur_timers == 0);
> + CU_ASSERT(tpinfo.hwm_timers == (unsigned)num_workers * NTIMERS);
> +
> + /* Destroy timer pool, all timers must have been freed */
> + odp_timer_pool_destroy(tp);
> +
> + CU_PASS("ODP timer test");
> +}
> +
> +CU_TestInfo test_odp_timer[] = {
> + {"test_odp_timer_all", test_odp_timer_all},
> + CU_TEST_INFO_NULL,
> +};
> +
> +CU_SuiteInfo odp_testsuites[] = {
> + {"Timer", NULL, NULL, NULL, NULL, test_odp_timer},
> + CU_SUITE_INFO_NULL,
> +};
> --
> 1.9.1
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
On 2014-12-09 17:56, Mike Holmes wrote: > Has checkpatch issues > > Using patch: > /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > git am > /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > Patch applied, building... > WARNING: line over 80 characters > #238: FILE: test/validation/odp_timer.c:185: > + CU_FAIL("Failed to set timer (tooearly/toolate)"); > > total: 0 errors, 1 warnings, 0 checks, 359 lines checked > > NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > > 0001-test-odp_timer.h-cunit-test.patch has style problems, please review. Do we want to split up this printout? If you're not familiar with the code and you run the validation tests and got this failure printed out. I would grep for that failure message (the string) in the code too see how I got there. And if we split this string up into multiple rows just to make check-patch happy it will make it harder to search after the failure message then right? Cheers, Anders
On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> wrote: > On 2014-12-09 17:56, Mike Holmes wrote: >> Has checkpatch issues >> >> Using patch: >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> git am >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> Patch applied, building... >> WARNING: line over 80 characters >> #238: FILE: test/validation/odp_timer.c:185: >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); >> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked >> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS >> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please review. > > Do we want to split up this printout? No. > If you're not familiar with the code and you run the validation tests > and got this failure printed out. I would grep for that failure message > (the string) in the code too see how I got there. Exactly. > And if we split this string up into multiple rows just to make > check-patch happy it will make it harder to search after the failure > message then right? Correct. And I even think checkpatch for this reason will complain if you split the string. -- Ola > > Cheers, > Anders
Agreed, this is one of the exceptions. On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> > wrote: > > On 2014-12-09 17:56, Mike Holmes wrote: > >> Has checkpatch issues > >> > >> Using patch: > >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > >> git am > >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > >> Patch applied, building... > >> WARNING: line over 80 characters > >> #238: FILE: test/validation/odp_timer.c:185: > >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); > >> > >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked > >> > >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > >> > >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please > review. > > > > Do we want to split up this printout? > No. > > > If you're not familiar with the code and you run the validation tests > > and got this failure printed out. I would grep for that failure message > > (the string) in the code too see how I got there. > Exactly. > > > And if we split this string up into multiple rows just to make > > check-patch happy it will make it harder to search after the failure > > message then right? > Correct. > And I even think checkpatch for this reason will complain if you split > the string. > > -- Ola > > > > > Cheers, > > Anders >
Rather than having a list of "privileged" code that gets special exceptions, why not just increase the checkpatch line length limit beyond the arbitrary 80-char limit? We're not coding on punch cards any more. :) Raising the limit to say 100 chars would eliminate the vast majority of these spurious issues. I know I've had to torture the buffer code in several places to comply with this procrustean "standard". Bill On Wed, Dec 10, 2014 at 10:33 AM, Mike Holmes <mike.holmes@linaro.org> wrote: > Agreed, this is one of the exceptions. > > On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: > >> On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> >> wrote: >> > On 2014-12-09 17:56, Mike Holmes wrote: >> >> Has checkpatch issues >> >> >> >> Using patch: >> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> >> git am >> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> >> Patch applied, building... >> >> WARNING: line over 80 characters >> >> #238: FILE: test/validation/odp_timer.c:185: >> >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); >> >> >> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked >> >> >> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS >> >> >> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please >> review. >> > >> > Do we want to split up this printout? >> No. >> >> > If you're not familiar with the code and you run the validation tests >> > and got this failure printed out. I would grep for that failure message >> > (the string) in the code too see how I got there. >> Exactly. >> >> > And if we split this string up into multiple rows just to make >> > check-patch happy it will make it harder to search after the failure >> > message then right? >> Correct. >> And I even think checkpatch for this reason will complain if you split >> the string. >> >> -- Ola >> >> > >> > Cheers, >> > Anders >> > > > > -- > *Mike Holmes* > Linaro Sr Technical Manager > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
On 10 December 2014 at 17:33, Mike Holmes <mike.holmes@linaro.org> wrote: > Agreed, this is one of the exceptions. It should be possible to fix checkpatch so that it does not warn for too long lines when that line is a string constant. I could remove some of the indentation but I assume checkpatch would complain on that intstead. > > On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> > wrote: >> >> On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> >> wrote: >> > On 2014-12-09 17:56, Mike Holmes wrote: >> >> Has checkpatch issues >> >> >> >> Using patch: >> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> >> git am >> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >> >> Patch applied, building... >> >> WARNING: line over 80 characters >> >> #238: FILE: test/validation/odp_timer.c:185: >> >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); >> >> >> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked >> >> >> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS >> >> >> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please >> >> review. >> > >> > Do we want to split up this printout? >> No. >> >> > If you're not familiar with the code and you run the validation tests >> > and got this failure printed out. I would grep for that failure message >> > (the string) in the code too see how I got there. >> Exactly. >> >> > And if we split this string up into multiple rows just to make >> > check-patch happy it will make it harder to search after the failure >> > message then right? >> Correct. >> And I even think checkpatch for this reason will complain if you split >> the string. >> >> -- Ola >> >> > >> > Cheers, >> > Anders > > > > > -- > Mike Holmes > Linaro Sr Technical Manager > LNG - ODP
On 10 December 2014 at 11:58, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Rather than having a list of "privileged" code that gets special > exceptions, > I don't think it is privileged, but as checkpatch docs say, if you have a good case for ignoring a guideline in a given case - ok > why not just increase the checkpatch line length limit beyond the > arbitrary 80-char limit? > We can start that thread again but it went in huge loops last time with no agreement between large camps on suggestions between 120 or 80 chars > We're not coding on punch cards any more. :) > That was the argument form the 120 crowd :) > > Raising the limit to say 100 chars would eliminate the vast majority of > these spurious issues. I know I've had to torture the buffer code in > several places to comply with this procrustean "standard". > If we get a quorum - why not change it, I have no issue. > > Bill > > On Wed, Dec 10, 2014 at 10:33 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> Agreed, this is one of the exceptions. >> >> On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >> >>> On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> >>> wrote: >>> > On 2014-12-09 17:56, Mike Holmes wrote: >>> >> Has checkpatch issues >>> >> >>> >> Using patch: >>> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >>> >> git am >>> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >>> >> Patch applied, building... >>> >> WARNING: line over 80 characters >>> >> #238: FILE: test/validation/odp_timer.c:185: >>> >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); >>> >> >>> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked >>> >> >>> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS >>> >> >>> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please >>> review. >>> > >>> > Do we want to split up this printout? >>> No. >>> >>> > If you're not familiar with the code and you run the validation tests >>> > and got this failure printed out. I would grep for that failure message >>> > (the string) in the code too see how I got there. >>> Exactly. >>> >>> > And if we split this string up into multiple rows just to make >>> > check-patch happy it will make it harder to search after the failure >>> > message then right? >>> Correct. >>> And I even think checkpatch for this reason will complain if you split >>> the string. >>> >>> -- Ola >>> >>> > >>> > Cheers, >>> > Anders >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Sr Technical Manager >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
VT100 forever! However 132 chars per line was supported even back then. I guess only PC text consoles are limited to 80 chars. On 10 December 2014 at 17:58, Bill Fischofer <bill.fischofer@linaro.org> wrote: > Rather than having a list of "privileged" code that gets special exceptions, > why not just increase the checkpatch line length limit beyond the arbitrary > 80-char limit? We're not coding on punch cards any more. :) > > Raising the limit to say 100 chars would eliminate the vast majority of > these spurious issues. I know I've had to torture the buffer code in > several places to comply with this procrustean "standard". > > Bill > > On Wed, Dec 10, 2014 at 10:33 AM, Mike Holmes <mike.holmes@linaro.org> > wrote: >> >> Agreed, this is one of the exceptions. >> >> On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> >> wrote: >>> >>> On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> >>> wrote: >>> > On 2014-12-09 17:56, Mike Holmes wrote: >>> >> Has checkpatch issues >>> >> >>> >> Using patch: >>> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >>> >> git am >>> >> /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox >>> >> Patch applied, building... >>> >> WARNING: line over 80 characters >>> >> #238: FILE: test/validation/odp_timer.c:185: >>> >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); >>> >> >>> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked >>> >> >>> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS >>> >> >>> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please >>> >> review. >>> > >>> > Do we want to split up this printout? >>> No. >>> >>> > If you're not familiar with the code and you run the validation tests >>> > and got this failure printed out. I would grep for that failure message >>> > (the string) in the code too see how I got there. >>> Exactly. >>> >>> > And if we split this string up into multiple rows just to make >>> > check-patch happy it will make it harder to search after the failure >>> > message then right? >>> Correct. >>> And I even think checkpatch for this reason will complain if you split >>> the string. >>> >>> -- Ola >>> >>> > >>> > Cheers, >>> > Anders >> >> >> >> >> -- >> Mike Holmes >> Linaro Sr Technical Manager >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >
Earlier the debates were pure philosophy. We now have a substantial body of code and that "sleek and stylish" 80-char straightjacket is a lot less comfortable for daily wear than it may have looked "on the rack" before we bought it. Even a bump to 85 would be a welcome relief. 100 or 132 would be equally arbitrary and I'd have no problem with removing the limit altogether and simply relying on the consensus of reviewers as to when lines are "too long". The point is that there's no real justification for keeping 80 other than "well, Linux does it that way". But ODP is not Linux and we've already abandoned Linux''s ban on typedefs so obviously we're not constrained by what Linux does. Bill On Wed, Dec 10, 2014 at 11:19 AM, Ola Liljedahl <ola.liljedahl@linaro.org> wrote: > VT100 forever! > However 132 chars per line was supported even back then. I guess only > PC text consoles are limited to 80 chars. > > > On 10 December 2014 at 17:58, Bill Fischofer <bill.fischofer@linaro.org> > wrote: > > Rather than having a list of "privileged" code that gets special > exceptions, > > why not just increase the checkpatch line length limit beyond the > arbitrary > > 80-char limit? We're not coding on punch cards any more. :) > > > > Raising the limit to say 100 chars would eliminate the vast majority of > > these spurious issues. I know I've had to torture the buffer code in > > several places to comply with this procrustean "standard". > > > > Bill > > > > On Wed, Dec 10, 2014 at 10:33 AM, Mike Holmes <mike.holmes@linaro.org> > > wrote: > >> > >> Agreed, this is one of the exceptions. > >> > >> On 10 December 2014 at 04:22, Ola Liljedahl <ola.liljedahl@linaro.org> > >> wrote: > >>> > >>> On 10 December 2014 at 09:13, Anders Roxell <anders.roxell@linaro.org> > >>> wrote: > >>> > On 2014-12-09 17:56, Mike Holmes wrote: > >>> >> Has checkpatch issues > >>> >> > >>> >> Using patch: > >>> >> > /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > >>> >> git am > >>> >> > /home/mike/incoming/lng-odp_PATCH_3-3_test_odp_timer.h_cunit_test.mbox > >>> >> Patch applied, building... > >>> >> WARNING: line over 80 characters > >>> >> #238: FILE: test/validation/odp_timer.c:185: > >>> >> + CU_FAIL("Failed to set timer (tooearly/toolate)"); > >>> >> > >>> >> total: 0 errors, 1 warnings, 0 checks, 359 lines checked > >>> >> > >>> >> NOTE: Ignored message types: DEPRECATED_VARIABLE NEW_TYPEDEFS > >>> >> > >>> >> 0001-test-odp_timer.h-cunit-test.patch has style problems, please > >>> >> review. > >>> > > >>> > Do we want to split up this printout? > >>> No. > >>> > >>> > If you're not familiar with the code and you run the validation tests > >>> > and got this failure printed out. I would grep for that failure > message > >>> > (the string) in the code too see how I got there. > >>> Exactly. > >>> > >>> > And if we split this string up into multiple rows just to make > >>> > check-patch happy it will make it harder to search after the failure > >>> > message then right? > >>> Correct. > >>> And I even think checkpatch for this reason will complain if you split > >>> the string. > >>> > >>> -- Ola > >>> > >>> > > >>> > Cheers, > >>> > Anders > >> > >> > >> > >> > >> -- > >> Mike Holmes > >> Linaro Sr Technical Manager > >> LNG - ODP > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org > >> http://lists.linaro.org/mailman/listinfo/lng-odp > >> > > >
On 17 December 2014 at 10:19, Jerin Jacob <jerin.jacob@caviumnetworks.com> wrote: > On Wed, Dec 17, 2014 at 09:28:48AM +0530, Jerin Jacob wrote: >> On Mon, Dec 08, 2014 at 11:49:46PM +0100, Ola Liljedahl wrote: >> > Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> >> > --- >> > (This document/code contribution attached is provided under the terms of >> > agreement LES-LTM-21309) >> > A new cunit test program test/validation/odp_timer.c for the updated timer API. >> > >> > test/validation/.gitignore | 1 + >> > test/validation/Makefile.am | 4 +- >> > test/validation/odp_timer.c | 336 ++++++++++++++++++++++++++++++++++++++++++++ >> > 3 files changed, 340 insertions(+), 1 deletion(-) >> > create mode 100644 test/validation/odp_timer.c >> > >> > diff --git a/test/validation/.gitignore b/test/validation/.gitignore >> > index 37e2594..586def0 100644 >> > --- a/test/validation/.gitignore >> > +++ b/test/validation/.gitignore >> > @@ -4,3 +4,4 @@ odp_init >> > odp_queue >> > odp_crypto >> > odp_shm >> > +odp_timer >> > diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am >> > index 8547085..043bf4c 100644 >> > --- a/test/validation/Makefile.am >> > +++ b/test/validation/Makefile.am >> > @@ -6,13 +6,14 @@ AM_LDFLAGS += -static >> > if ODP_CUNIT_ENABLED >> > TESTS = ${bin_PROGRAMS} >> > check_PROGRAMS = ${bin_PROGRAMS} >> > -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm >> > +bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_timer >> > odp_init_LDFLAGS = $(AM_LDFLAGS) >> > odp_queue_LDFLAGS = $(AM_LDFLAGS) >> > odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto >> > odp_crypto_LDFLAGS = $(AM_LDFLAGS) >> > odp_shm_CFLAGS = $(AM_CFLAGS) >> > odp_shm_LDFLAGS = $(AM_LDFLAGS) >> > +odp_timer_LDFLAGS = $(AM_LDFLAGS) >> > endif >> > >> > dist_odp_init_SOURCES = odp_init.c >> > @@ -22,3 +23,4 @@ dist_odp_crypto_SOURCES = crypto/odp_crypto_test_async_inp.c \ >> > crypto/odp_crypto_test_rng.c \ >> > odp_crypto.c common/odp_cunit_common.c >> > dist_odp_shm_SOURCES = odp_shm.c common/odp_cunit_common.c >> > +dist_odp_timer_SOURCES = odp_timer.c common/odp_cunit_common.c >> > diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c >> > new file mode 100644 >> > index 0000000..4b6b872 >> > --- /dev/null >> > +++ b/test/validation/odp_timer.c >> > @@ -0,0 +1,336 @@ >> > +/* Copyright (c) 2014, Linaro Limited >> > + * All rights reserved. >> > + * >> > + * SPDX-License-Identifier: BSD-3-Clause >> > + */ >> > + >> > +/** >> > + * @file >> > + */ >> > + >> > +#include <assert.h> >> > +#include <unistd.h> >> > +#include <odp.h> >> > +#include <odp_timer.h> >> > +#include <odph_linux.h> >> > +#include <odph_chksum.h> >> > +#include "odp_cunit_common.h" >> > + >> > +/** @private Timeout range in milliseconds (ms) */ >> > +#define RANGE_MS 2000 >> > + >> > +/** @private Number of timers per thread */ >> > +#define NTIMERS 2000 >> > + >> > +/** @private Timeout pool size per thread */ >> > +#define TMO_POOL_SIZE (512 * NTIMERS) >> > + >> > +/** @private Barrier for thread synchronisation */ >> > +static odp_barrier_t test_barrier; >> > + >> > +/** @private Timeout buffer pool handle used by all threads */ >> > +static odp_buffer_pool_t tbp; >> > + >> > +/** @private Timer pool handle used by all threads */ >> > +static odp_timer_pool_t tp; >> > + >> > +/** @private min() function */ >> > +static int min(int a, int b) >> > +{ >> > + return a < b ? a : b; >> > +} >> > + >> > +/* @private Timer helper structure */ >> > +struct test_timer { >> > + odp_timer_t tim; /* Timer handle */ >> > + odp_buffer_t buf; /* Timeout buffer */ >> > + odp_buffer_t buf2; /* Copy of buffer handle */ >> > + uint64_t tick; /* Expiration tick or ODP_TICK_INVALID */ >> > +}; >> > + >> > +/* @private Handle a received (timeout) buffer */ >> > +static void handle_tmo(odp_buffer_t buf, bool stale, uint64_t prev_tick) >> > +{ >> > + odp_timer_t tim = ODP_TIMER_INVALID; >> > + uint64_t tick = ODP_TICK_INVALID; >> > + struct test_timer *ttp = NULL; >> > + >> > + /* Use assert() for correctness check of test program itself */ >> > + assert(buf != ODP_BUFFER_INVALID); >> > + if (!odp_timer_tmo_metadata(buf, &tim, &tick, (void **)&ttp)) { >> > + /* Not a default timeout buffer */ >> > + CU_FAIL("Unexpected buffer type received"); >> > + return; >> > + } >> > + >> > + if (tim == ODP_TIMER_INVALID) >> > + CU_FAIL("odp_timer_tmo_metadata() invalid timer"); >> > + if (tick == ODP_TICK_INVALID) >> > + CU_FAIL("odp_timer_tmo_metadata() invalid tick"); >> > + if (ttp == NULL) >> > + CU_FAIL("odp_timer_tmo_metadata() null user ptr"); >> > + >> > + if (ttp->buf2 != buf) >> > + CU_FAIL("odp_timer_tmo_metadata() wrong user ptr"); >> > + if (ttp->tim != tim) >> > + CU_FAIL("odp_timer_tmo_metadata() wrong timer"); >> > + if (stale) { >> > + /* Stale timeout => timer must have invalid tick */ >> > + if (ttp->tick != ODP_TICK_INVALID) >> > + CU_FAIL("Stale timeout for active timer"); >> > + } else { >> > + /* Fresh timeout => timer must have matching tick */ >> > + if (ttp->tick != tick) >> > + CU_FAIL("odp_timer_tmo_metadata() wrong tick"); >> > + /* Check that timeout was delivered 'timely' */ >> > + if (tick > odp_timer_current_tick(tp)) >> > + CU_FAIL("Timeout delivered too early"); >> > + if (tick < prev_tick) >> > + CU_FAIL("Timeout delivered too late"); >> > + } >> > + >> > + /* Use assert() for correctness check of test program itself */ >> > + assert(ttp->buf == ODP_BUFFER_INVALID); >> > + ttp->buf = buf; >> > +} >> > + >> > +/* @private Worker thread entrypoint which performs timer alloc/set/cancel/free >> > + * tests */ >> > +static void *worker_entrypoint(void *arg) >> > +{ >> > + int thr = odp_thread_id(); >> > + uint32_t i; >> > + unsigned seed = thr; >> > + (void)arg; >> > + >> > + odp_queue_t queue = odp_queue_create("timer_queue", >> > + ODP_QUEUE_TYPE_POLL, >> > + NULL); >> > + if (queue == ODP_QUEUE_INVALID) >> > + CU_FAIL_FATAL("Queue create failed"); >> > + >> > + struct test_timer *tt = malloc(sizeof(struct test_timer) * NTIMERS); >> >> >> allocate the memory from odp shm mem to share between different worker threads > > Found tt used only for local storage. malloc is fine then. Good. Are unit tests supposed to work with the multiprocess model as well and thus cannot use malloc() for shared data? > >> >> >> > + if (tt == NULL) >> > + perror("malloc"), abort(); >> > + >> > + /* Prepare all timers */ >> > + for (i = 0; i < NTIMERS; i++) { >> > + tt[i].tim = odp_timer_alloc(tp, queue, &tt[i]); >> > + if (tt[i].tim == ODP_TIMER_INVALID) >> > + CU_FAIL_FATAL("Failed to allocate timer"); >> > + tt[i].buf = odp_buffer_alloc(tbp); >> > + if (tt[i].buf == ODP_BUFFER_INVALID) >> > + CU_FAIL_FATAL("Failed to allocate timeout buffer"); >> > + tt[i].buf2 = tt[i].buf; >> > + tt[i].tick = ODP_TICK_INVALID; >> > + } >> > + >> > + odp_barrier_wait(&test_barrier); >> > + >> > + /* Initial set all timers with a random expiration time */ >> > + uint32_t nset = 0; >> > + for (i = 0; i < NTIMERS; i++) { >> > + uint64_t tck = odp_timer_current_tick(tp) + 1 + >> > + odp_timer_ns_to_tick(tp, >> > + (rand_r(&seed) % RANGE_MS) >> > + * 1000000ULL); >> > + tt[i].tick = odp_timer_set_abs(tt[i].tim, tck, &tt[i].buf); >> > + uint64_t rc = tt[i].tick; >> > + if (rc == ODP_TICK_TOOEARLY || >> > + rc == ODP_TICK_TOOLATE || >> > + rc == ODP_TICK_INVALID) { >> > + CU_FAIL("Failed to set timer"); >> > + } >> > + nset++; >> > + } >> > + >> > + /* Step through wall time, 1ms at a time and check for expired timers */ >> > + uint32_t nrcv = 0; >> > + uint32_t nreset = 0; >> > + uint32_t ncancel = 0; >> > + uint32_t ntoolate = 0; >> > + uint32_t ms; >> > + uint64_t prev_tick = odp_timer_current_tick(tp); >> > + for (ms = 0; ms < 7 * RANGE_MS / 10; ms++) { >> > + odp_buffer_t buf; >> > + while ((buf = odp_queue_deq(queue)) != ODP_BUFFER_INVALID) { >> > + handle_tmo(buf, false, prev_tick - 1); >> > + nrcv++; >> > + } >> > + prev_tick = odp_timer_current_tick(tp); >> > + i = rand_r(&seed) % NTIMERS; >> > + if (tt[i].buf == ODP_BUFFER_INVALID && >> > + (rand_r(&seed) % 2 == 0)) { >> > + /* Timer active, cancel it */ >> > + tt[i].tick = odp_timer_cancel(tt[i].tim, &tt[i].buf); >> > + if (tt[i].buf == ODP_BUFFER_INVALID) { >> > + /* Cancel failed, timer already expired */ >> > + ntoolate++; >> > + } >> > + ncancel++; >> > + } else { >> > + if (tt[i].buf != ODP_BUFFER_INVALID) >> > + /* Timer inactive => set */ >> > + nset++; >> > + else >> > + /* Timer active => reset */ >> > + nreset++; >> > + uint64_t tck = 1 + odp_timer_ns_to_tick(tp, >> > + (rand_r(&seed) % RANGE_MS) * 1000000ULL); >> > + tt[i].tick = odp_timer_set_rel(tt[i].tim, tck, >> > + &tt[i].buf); >> > + uint64_t rc = tt[i].tick; >> > + if (rc == ODP_TICK_TOOEARLY || >> > + rc == ODP_TICK_TOOLATE) { >> > + CU_FAIL("Failed to set timer (tooearly/toolate)"); >> > + } else if (rc == ODP_TICK_INVALID) { >> > + /* Reset failed, timer already expired */ >> > + ntoolate++; >> > + } >> > + } >> > + if (usleep(1000/*1ms*/) < 0) >> > + perror("usleep"), abort(); >> > + } >> > + >> > + /* Free (including cancel) all timers */ >> > + uint32_t nstale = 0; >> > + for (i = 0; i < NTIMERS; i++) { >> > + tt[i].tick = odp_timer_free(tt[i].tim, &tt[i].buf); >> > + if (tt[i].buf == ODP_BUFFER_INVALID) >> > + /* Cancel/free too late, timer already expired and >> > + * timoeut buffer enqueued */ >> > + nstale++; >> > + } >> > + >> > + printf("Thread %u: %u timers set\n", thr, nset); >> > + printf("Thread %u: %u timers reset\n", thr, nreset); >> > + printf("Thread %u: %u timers cancelled\n", thr, ncancel); >> > + printf("Thread %u: %u timers reset/cancelled too late\n", >> > + thr, ntoolate); >> > + printf("Thread %u: %u timeouts received\n", thr, nrcv); >> > + printf("Thread %u: %u stale timeout(s) after odp_timer_free()\n", >> > + thr, nstale); >> > + >> > + /* Delay some more to ensure timeouts for expired timers can be >> > + * received */ >> > + usleep(1000/*1ms*/); >> > + while (nstale != 0) { >> > + odp_buffer_t buf = odp_queue_deq(queue); >> > + if (buf != ODP_BUFFER_INVALID) { >> > + handle_tmo(buf, true, 0/*Dont' care for stale tmo's*/); >> > + nstale--; >> > + } else { >> > + CU_FAIL("Failed to receive stale timeout"); >> > + break; >> > + } >> > + } >> > + /* Check if there any more (unexpected) buffers */ >> > + odp_buffer_t buf = odp_queue_deq(queue); >> > + if (buf != ODP_BUFFER_INVALID) >> > + CU_FAIL("Unexpected buffer received"); >> > + >> > + printf("Thread %u: exiting\n", thr); >> > + return NULL; >> > +} >> > + >> > +/* @private Timer test case entrypoint */ >> > +static void test_odp_timer_all(void) >> > +{ >> > + odp_shm_t shm; >> > + int num_workers = min(odp_sys_core_count(), MAX_WORKERS); >> > + >> > + /* Create timeout buffer pools */ >> > + shm = odp_shm_reserve("tmo_bufs", >> > + TMO_POOL_SIZE * num_workers, >> > + ODP_CACHE_LINE_SIZE, 0); >> > + if (shm == ODP_SHM_INVALID) >> > + CU_FAIL_FATAL("Timeout shared memory alloc failed"); >> > + >> > + tbp = odp_buffer_pool_create("tmo_pool", odp_shm_addr(shm), >> > + TMO_POOL_SIZE * num_workers, >> > + 0, >> > + ODP_CACHE_LINE_SIZE, >> > + ODP_BUFFER_TYPE_TIMEOUT); >> > + if (tbp == ODP_BUFFER_POOL_INVALID) >> > + CU_FAIL_FATAL("Timeout buffer pool create failed"); >> > + >> > +#define NAME "timer_pool" >> > +#define RES (10 * ODP_TIME_MSEC / 3) >> > +#define MIN (10 * ODP_TIME_MSEC / 3) >> > +#define MAX (1000000 * ODP_TIME_MSEC) >> > + /* Create a timer pool */ >> > + tp = odp_timer_pool_create(NAME, tbp, >> > + RES, MIN, MAX, >> > + num_workers * NTIMERS, >> > + true, ODP_CLOCK_CPU); >> > + if (tp == ODP_TIMER_POOL_INVALID) >> > + CU_FAIL_FATAL("Timer pool create failed"); >> > + >> > + /* Start all created timer pools */ >> > + odp_timer_pool_start(); >> > + >> > + odp_timer_pool_info_t tpinfo; >> > + size_t sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo)); >> > + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1) >> > + CU_FAIL("odp_timer_pool_info"); >> > + CU_ASSERT(strcmp(tpinfo.name, NAME) == 0); >> > + CU_ASSERT(tpinfo.resolution == RES); >> > + CU_ASSERT(tpinfo.min_tmo == odp_timer_ns_to_tick(tp, MIN)); >> > + CU_ASSERT(tpinfo.max_tmo == odp_timer_ns_to_tick(tp, MAX)); >> > + printf("Timer pool\n"); >> > + printf("----------\n"); >> > + printf(" name: %s\n", tpinfo.name); >> > + printf(" resolution: %"PRIu64" ns (%"PRIu64" us)\n", >> > + tpinfo.resolution, tpinfo.resolution / 1000); >> > + printf(" min tmo: %"PRIu64" tick(s)\n", tpinfo.min_tmo); >> > + printf(" max tmo: %"PRIu64" ticks\n", tpinfo.max_tmo); >> > + printf("\n"); >> > + >> > + printf("#timers..: %u\n", NTIMERS); >> > + printf("Tmo range: %u ms (%"PRIu64" ticks)\n", RANGE_MS, >> > + odp_timer_ns_to_tick(tp, 1000000ULL * RANGE_MS)); >> > + printf("\n"); >> > + >> > + uint64_t tick; >> > + for (tick = 0; tick < 1000000000000ULL; tick += 1000000ULL) { >> > + uint64_t ns = odp_timer_tick_to_ns(tp, tick); >> > + uint64_t t2 = odp_timer_ns_to_tick(tp, ns); >> > + if (tick != t2) >> > + CU_FAIL("Invalid conversion tick->ns->tick"); >> > + } >> > + >> > + /* Initialize barrier used by worker threads for synchronization */ >> > + odp_barrier_init(&test_barrier, num_workers); >> > + >> > + /* Create and start worker threads */ >> > + pthrd_arg thrdarg; >> > + thrdarg.testcase = 0; >> > + thrdarg.numthrds = num_workers; >> > + odp_cunit_thread_create(worker_entrypoint, &thrdarg); >> > + >> > + /* Wait for worker threads to exit */ >> > + odp_cunit_thread_exit(&thrdarg); >> > + >> > + /* Check some statistics after the test */ >> > + sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo)); >> > + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1) >> > + CU_FAIL("odp_timer_pool_info"); >> > + CU_ASSERT(tpinfo.num_timers == (unsigned)num_workers * NTIMERS); >> > + CU_ASSERT(tpinfo.cur_timers == 0); >> > + CU_ASSERT(tpinfo.hwm_timers == (unsigned)num_workers * NTIMERS); >> > + >> > + /* Destroy timer pool, all timers must have been freed */ >> > + odp_timer_pool_destroy(tp); >> > + >> > + CU_PASS("ODP timer test"); >> > +} >> > + >> > +CU_TestInfo test_odp_timer[] = { >> > + {"test_odp_timer_all", test_odp_timer_all}, >> > + CU_TEST_INFO_NULL, >> > +}; >> > + >> > +CU_SuiteInfo odp_testsuites[] = { >> > + {"Timer", NULL, NULL, NULL, NULL, test_odp_timer}, >> > + CU_SUITE_INFO_NULL, >> > +}; >> > -- >> > 1.9.1 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp
diff --git a/test/validation/.gitignore b/test/validation/.gitignore index 37e2594..586def0 100644 --- a/test/validation/.gitignore +++ b/test/validation/.gitignore @@ -4,3 +4,4 @@ odp_init odp_queue odp_crypto odp_shm +odp_timer diff --git a/test/validation/Makefile.am b/test/validation/Makefile.am index 8547085..043bf4c 100644 --- a/test/validation/Makefile.am +++ b/test/validation/Makefile.am @@ -6,13 +6,14 @@ AM_LDFLAGS += -static if ODP_CUNIT_ENABLED TESTS = ${bin_PROGRAMS} check_PROGRAMS = ${bin_PROGRAMS} -bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm +bin_PROGRAMS = odp_init odp_queue odp_crypto odp_shm odp_timer odp_init_LDFLAGS = $(AM_LDFLAGS) odp_queue_LDFLAGS = $(AM_LDFLAGS) odp_crypto_CFLAGS = $(AM_CFLAGS) -I$(srcdir)/crypto odp_crypto_LDFLAGS = $(AM_LDFLAGS) odp_shm_CFLAGS = $(AM_CFLAGS) odp_shm_LDFLAGS = $(AM_LDFLAGS) +odp_timer_LDFLAGS = $(AM_LDFLAGS) endif dist_odp_init_SOURCES = odp_init.c @@ -22,3 +23,4 @@ dist_odp_crypto_SOURCES = crypto/odp_crypto_test_async_inp.c \ crypto/odp_crypto_test_rng.c \ odp_crypto.c common/odp_cunit_common.c dist_odp_shm_SOURCES = odp_shm.c common/odp_cunit_common.c +dist_odp_timer_SOURCES = odp_timer.c common/odp_cunit_common.c diff --git a/test/validation/odp_timer.c b/test/validation/odp_timer.c new file mode 100644 index 0000000..4b6b872 --- /dev/null +++ b/test/validation/odp_timer.c @@ -0,0 +1,336 @@ +/* Copyright (c) 2014, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +/** + * @file + */ + +#include <assert.h> +#include <unistd.h> +#include <odp.h> +#include <odp_timer.h> +#include <odph_linux.h> +#include <odph_chksum.h> +#include "odp_cunit_common.h" + +/** @private Timeout range in milliseconds (ms) */ +#define RANGE_MS 2000 + +/** @private Number of timers per thread */ +#define NTIMERS 2000 + +/** @private Timeout pool size per thread */ +#define TMO_POOL_SIZE (512 * NTIMERS) + +/** @private Barrier for thread synchronisation */ +static odp_barrier_t test_barrier; + +/** @private Timeout buffer pool handle used by all threads */ +static odp_buffer_pool_t tbp; + +/** @private Timer pool handle used by all threads */ +static odp_timer_pool_t tp; + +/** @private min() function */ +static int min(int a, int b) +{ + return a < b ? a : b; +} + +/* @private Timer helper structure */ +struct test_timer { + odp_timer_t tim; /* Timer handle */ + odp_buffer_t buf; /* Timeout buffer */ + odp_buffer_t buf2; /* Copy of buffer handle */ + uint64_t tick; /* Expiration tick or ODP_TICK_INVALID */ +}; + +/* @private Handle a received (timeout) buffer */ +static void handle_tmo(odp_buffer_t buf, bool stale, uint64_t prev_tick) +{ + odp_timer_t tim = ODP_TIMER_INVALID; + uint64_t tick = ODP_TICK_INVALID; + struct test_timer *ttp = NULL; + + /* Use assert() for correctness check of test program itself */ + assert(buf != ODP_BUFFER_INVALID); + if (!odp_timer_tmo_metadata(buf, &tim, &tick, (void **)&ttp)) { + /* Not a default timeout buffer */ + CU_FAIL("Unexpected buffer type received"); + return; + } + + if (tim == ODP_TIMER_INVALID) + CU_FAIL("odp_timer_tmo_metadata() invalid timer"); + if (tick == ODP_TICK_INVALID) + CU_FAIL("odp_timer_tmo_metadata() invalid tick"); + if (ttp == NULL) + CU_FAIL("odp_timer_tmo_metadata() null user ptr"); + + if (ttp->buf2 != buf) + CU_FAIL("odp_timer_tmo_metadata() wrong user ptr"); + if (ttp->tim != tim) + CU_FAIL("odp_timer_tmo_metadata() wrong timer"); + if (stale) { + /* Stale timeout => timer must have invalid tick */ + if (ttp->tick != ODP_TICK_INVALID) + CU_FAIL("Stale timeout for active timer"); + } else { + /* Fresh timeout => timer must have matching tick */ + if (ttp->tick != tick) + CU_FAIL("odp_timer_tmo_metadata() wrong tick"); + /* Check that timeout was delivered 'timely' */ + if (tick > odp_timer_current_tick(tp)) + CU_FAIL("Timeout delivered too early"); + if (tick < prev_tick) + CU_FAIL("Timeout delivered too late"); + } + + /* Use assert() for correctness check of test program itself */ + assert(ttp->buf == ODP_BUFFER_INVALID); + ttp->buf = buf; +} + +/* @private Worker thread entrypoint which performs timer alloc/set/cancel/free + * tests */ +static void *worker_entrypoint(void *arg) +{ + int thr = odp_thread_id(); + uint32_t i; + unsigned seed = thr; + (void)arg; + + odp_queue_t queue = odp_queue_create("timer_queue", + ODP_QUEUE_TYPE_POLL, + NULL); + if (queue == ODP_QUEUE_INVALID) + CU_FAIL_FATAL("Queue create failed"); + + struct test_timer *tt = malloc(sizeof(struct test_timer) * NTIMERS); + if (tt == NULL) + perror("malloc"), abort(); + + /* Prepare all timers */ + for (i = 0; i < NTIMERS; i++) { + tt[i].tim = odp_timer_alloc(tp, queue, &tt[i]); + if (tt[i].tim == ODP_TIMER_INVALID) + CU_FAIL_FATAL("Failed to allocate timer"); + tt[i].buf = odp_buffer_alloc(tbp); + if (tt[i].buf == ODP_BUFFER_INVALID) + CU_FAIL_FATAL("Failed to allocate timeout buffer"); + tt[i].buf2 = tt[i].buf; + tt[i].tick = ODP_TICK_INVALID; + } + + odp_barrier_wait(&test_barrier); + + /* Initial set all timers with a random expiration time */ + uint32_t nset = 0; + for (i = 0; i < NTIMERS; i++) { + uint64_t tck = odp_timer_current_tick(tp) + 1 + + odp_timer_ns_to_tick(tp, + (rand_r(&seed) % RANGE_MS) + * 1000000ULL); + tt[i].tick = odp_timer_set_abs(tt[i].tim, tck, &tt[i].buf); + uint64_t rc = tt[i].tick; + if (rc == ODP_TICK_TOOEARLY || + rc == ODP_TICK_TOOLATE || + rc == ODP_TICK_INVALID) { + CU_FAIL("Failed to set timer"); + } + nset++; + } + + /* Step through wall time, 1ms at a time and check for expired timers */ + uint32_t nrcv = 0; + uint32_t nreset = 0; + uint32_t ncancel = 0; + uint32_t ntoolate = 0; + uint32_t ms; + uint64_t prev_tick = odp_timer_current_tick(tp); + for (ms = 0; ms < 7 * RANGE_MS / 10; ms++) { + odp_buffer_t buf; + while ((buf = odp_queue_deq(queue)) != ODP_BUFFER_INVALID) { + handle_tmo(buf, false, prev_tick - 1); + nrcv++; + } + prev_tick = odp_timer_current_tick(tp); + i = rand_r(&seed) % NTIMERS; + if (tt[i].buf == ODP_BUFFER_INVALID && + (rand_r(&seed) % 2 == 0)) { + /* Timer active, cancel it */ + tt[i].tick = odp_timer_cancel(tt[i].tim, &tt[i].buf); + if (tt[i].buf == ODP_BUFFER_INVALID) { + /* Cancel failed, timer already expired */ + ntoolate++; + } + ncancel++; + } else { + if (tt[i].buf != ODP_BUFFER_INVALID) + /* Timer inactive => set */ + nset++; + else + /* Timer active => reset */ + nreset++; + uint64_t tck = 1 + odp_timer_ns_to_tick(tp, + (rand_r(&seed) % RANGE_MS) * 1000000ULL); + tt[i].tick = odp_timer_set_rel(tt[i].tim, tck, + &tt[i].buf); + uint64_t rc = tt[i].tick; + if (rc == ODP_TICK_TOOEARLY || + rc == ODP_TICK_TOOLATE) { + CU_FAIL("Failed to set timer (tooearly/toolate)"); + } else if (rc == ODP_TICK_INVALID) { + /* Reset failed, timer already expired */ + ntoolate++; + } + } + if (usleep(1000/*1ms*/) < 0) + perror("usleep"), abort(); + } + + /* Free (including cancel) all timers */ + uint32_t nstale = 0; + for (i = 0; i < NTIMERS; i++) { + tt[i].tick = odp_timer_free(tt[i].tim, &tt[i].buf); + if (tt[i].buf == ODP_BUFFER_INVALID) + /* Cancel/free too late, timer already expired and + * timoeut buffer enqueued */ + nstale++; + } + + printf("Thread %u: %u timers set\n", thr, nset); + printf("Thread %u: %u timers reset\n", thr, nreset); + printf("Thread %u: %u timers cancelled\n", thr, ncancel); + printf("Thread %u: %u timers reset/cancelled too late\n", + thr, ntoolate); + printf("Thread %u: %u timeouts received\n", thr, nrcv); + printf("Thread %u: %u stale timeout(s) after odp_timer_free()\n", + thr, nstale); + + /* Delay some more to ensure timeouts for expired timers can be + * received */ + usleep(1000/*1ms*/); + while (nstale != 0) { + odp_buffer_t buf = odp_queue_deq(queue); + if (buf != ODP_BUFFER_INVALID) { + handle_tmo(buf, true, 0/*Dont' care for stale tmo's*/); + nstale--; + } else { + CU_FAIL("Failed to receive stale timeout"); + break; + } + } + /* Check if there any more (unexpected) buffers */ + odp_buffer_t buf = odp_queue_deq(queue); + if (buf != ODP_BUFFER_INVALID) + CU_FAIL("Unexpected buffer received"); + + printf("Thread %u: exiting\n", thr); + return NULL; +} + +/* @private Timer test case entrypoint */ +static void test_odp_timer_all(void) +{ + odp_shm_t shm; + int num_workers = min(odp_sys_core_count(), MAX_WORKERS); + + /* Create timeout buffer pools */ + shm = odp_shm_reserve("tmo_bufs", + TMO_POOL_SIZE * num_workers, + ODP_CACHE_LINE_SIZE, 0); + if (shm == ODP_SHM_INVALID) + CU_FAIL_FATAL("Timeout shared memory alloc failed"); + + tbp = odp_buffer_pool_create("tmo_pool", odp_shm_addr(shm), + TMO_POOL_SIZE * num_workers, + 0, + ODP_CACHE_LINE_SIZE, + ODP_BUFFER_TYPE_TIMEOUT); + if (tbp == ODP_BUFFER_POOL_INVALID) + CU_FAIL_FATAL("Timeout buffer pool create failed"); + +#define NAME "timer_pool" +#define RES (10 * ODP_TIME_MSEC / 3) +#define MIN (10 * ODP_TIME_MSEC / 3) +#define MAX (1000000 * ODP_TIME_MSEC) + /* Create a timer pool */ + tp = odp_timer_pool_create(NAME, tbp, + RES, MIN, MAX, + num_workers * NTIMERS, + true, ODP_CLOCK_CPU); + if (tp == ODP_TIMER_POOL_INVALID) + CU_FAIL_FATAL("Timer pool create failed"); + + /* Start all created timer pools */ + odp_timer_pool_start(); + + odp_timer_pool_info_t tpinfo; + size_t sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo)); + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1) + CU_FAIL("odp_timer_pool_info"); + CU_ASSERT(strcmp(tpinfo.name, NAME) == 0); + CU_ASSERT(tpinfo.resolution == RES); + CU_ASSERT(tpinfo.min_tmo == odp_timer_ns_to_tick(tp, MIN)); + CU_ASSERT(tpinfo.max_tmo == odp_timer_ns_to_tick(tp, MAX)); + printf("Timer pool\n"); + printf("----------\n"); + printf(" name: %s\n", tpinfo.name); + printf(" resolution: %"PRIu64" ns (%"PRIu64" us)\n", + tpinfo.resolution, tpinfo.resolution / 1000); + printf(" min tmo: %"PRIu64" tick(s)\n", tpinfo.min_tmo); + printf(" max tmo: %"PRIu64" ticks\n", tpinfo.max_tmo); + printf("\n"); + + printf("#timers..: %u\n", NTIMERS); + printf("Tmo range: %u ms (%"PRIu64" ticks)\n", RANGE_MS, + odp_timer_ns_to_tick(tp, 1000000ULL * RANGE_MS)); + printf("\n"); + + uint64_t tick; + for (tick = 0; tick < 1000000000000ULL; tick += 1000000ULL) { + uint64_t ns = odp_timer_tick_to_ns(tp, tick); + uint64_t t2 = odp_timer_ns_to_tick(tp, ns); + if (tick != t2) + CU_FAIL("Invalid conversion tick->ns->tick"); + } + + /* Initialize barrier used by worker threads for synchronization */ + odp_barrier_init(&test_barrier, num_workers); + + /* Create and start worker threads */ + pthrd_arg thrdarg; + thrdarg.testcase = 0; + thrdarg.numthrds = num_workers; + odp_cunit_thread_create(worker_entrypoint, &thrdarg); + + /* Wait for worker threads to exit */ + odp_cunit_thread_exit(&thrdarg); + + /* Check some statistics after the test */ + sz = odp_timer_pool_info(tp, &tpinfo, sizeof(tpinfo)); + if (sz < offsetof(odp_timer_pool_info_t, name) + strlen(NAME) + 1) + CU_FAIL("odp_timer_pool_info"); + CU_ASSERT(tpinfo.num_timers == (unsigned)num_workers * NTIMERS); + CU_ASSERT(tpinfo.cur_timers == 0); + CU_ASSERT(tpinfo.hwm_timers == (unsigned)num_workers * NTIMERS); + + /* Destroy timer pool, all timers must have been freed */ + odp_timer_pool_destroy(tp); + + CU_PASS("ODP timer test"); +} + +CU_TestInfo test_odp_timer[] = { + {"test_odp_timer_all", test_odp_timer_all}, + CU_TEST_INFO_NULL, +}; + +CU_SuiteInfo odp_testsuites[] = { + {"Timer", NULL, NULL, NULL, NULL, test_odp_timer}, + CU_SUITE_INFO_NULL, +};
Signed-off-by: Ola Liljedahl <ola.liljedahl@linaro.org> --- (This document/code contribution attached is provided under the terms of agreement LES-LTM-21309) A new cunit test program test/validation/odp_timer.c for the updated timer API. test/validation/.gitignore | 1 + test/validation/Makefile.am | 4 +- test/validation/odp_timer.c | 336 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 test/validation/odp_timer.c