From: Tyler Retzlaff <roretzla@linux.microsoft.com>
To: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
Cc: dev@dpdk.org, "Erik Gabriel Carrillo" <erik.g.carrillo@intel.com>,
"David Marchand" <david.marchand@redhat.com>,
maria.lingemark@ericsson.com,
"Stefan Sundkvist" <stefan.sundkvist@ericsson.com>,
"Stephen Hemminger" <stephen@networkplumber.org>,
"Morten Brørup" <mb@smartsharesystems.com>
Subject: Re: [RFC v2 2/2] eal: add high-performance timer facility
Date: Wed, 15 Mar 2023 20:55:56 -0700 [thread overview]
Message-ID: <20230316035556.GB5703@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> (raw)
In-Reply-To: <20230315170342.214127-3-mattias.ronnblom@ericsson.com>
On Wed, Mar 15, 2023 at 06:03:42PM +0100, Mattias Rönnblom wrote:
> The htimer library attempts at providing a timer facility with roughly
> the same functionality, but less overhead and better scalability than
> DPDK timer library.
>
> The htimer library employs per-lcore hierarchical timer wheels and a
> message-based synchronization/MT-safety scheme.
>
> RFC v2:
> * Fix spelling.
> * Fix signed/unsigned comparisons and discontinue the use of name-less
> function parameters, both of which may result in compiler warnings.
> * Undo the accidental removal of the bitset tests from the 'fast_tests'.
> * Add a number of missing include files, causing build failures
> (e.g., on AArch64 builds).
> * Add perf test attempting to compare rte_timer, rte_htimer and rte_htw.
> * Use nanoseconds (instead of TSC) as the default time unit.
> * add() and manage() has flags which allows the caller to specify the
> time unit (nanoseconds, TSC, or ticks) for the times provided.
>
> Signed-off-by: Mattias Rönnblom <mattias.ronnblom@ericsson.com>
> ---
> app/test/meson.build | 8 +
> app/test/test_htimer_mgr.c | 674 +++++++++++++++++++++++++
> app/test/test_htimer_mgr_perf.c | 322 ++++++++++++
> app/test/test_htw.c | 478 ++++++++++++++++++
> app/test/test_htw_perf.c | 181 +++++++
> app/test/test_timer_htimer_htw_perf.c | 693 ++++++++++++++++++++++++++
> doc/api/doxy-api-index.md | 5 +-
> doc/api/doxy-api.conf.in | 1 +
> lib/htimer/meson.build | 7 +
> lib/htimer/rte_htimer.h | 68 +++
> lib/htimer/rte_htimer_mgr.c | 547 ++++++++++++++++++++
> lib/htimer/rte_htimer_mgr.h | 516 +++++++++++++++++++
> lib/htimer/rte_htimer_msg.h | 44 ++
> lib/htimer/rte_htimer_msg_ring.c | 18 +
> lib/htimer/rte_htimer_msg_ring.h | 55 ++
> lib/htimer/rte_htw.c | 445 +++++++++++++++++
> lib/htimer/rte_htw.h | 49 ++
> lib/htimer/version.map | 17 +
> lib/meson.build | 1 +
> 19 files changed, 4128 insertions(+), 1 deletion(-)
> create mode 100644 app/test/test_htimer_mgr.c
> create mode 100644 app/test/test_htimer_mgr_perf.c
> create mode 100644 app/test/test_htw.c
> create mode 100644 app/test/test_htw_perf.c
> create mode 100644 app/test/test_timer_htimer_htw_perf.c
> create mode 100644 lib/htimer/meson.build
> create mode 100644 lib/htimer/rte_htimer.h
> create mode 100644 lib/htimer/rte_htimer_mgr.c
> create mode 100644 lib/htimer/rte_htimer_mgr.h
> create mode 100644 lib/htimer/rte_htimer_msg.h
> create mode 100644 lib/htimer/rte_htimer_msg_ring.c
> create mode 100644 lib/htimer/rte_htimer_msg_ring.h
> create mode 100644 lib/htimer/rte_htw.c
> create mode 100644 lib/htimer/rte_htw.h
> create mode 100644 lib/htimer/version.map
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index 03811ff692..d0308ac09d 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -140,9 +140,14 @@ test_sources = files(
> 'test_thash_perf.c',
> 'test_threads.c',
> 'test_timer.c',
> + 'test_timer_htimer_htw_perf.c',
> 'test_timer_perf.c',
> 'test_timer_racecond.c',
> 'test_timer_secondary.c',
> + 'test_htimer_mgr.c',
> + 'test_htimer_mgr_perf.c',
> + 'test_htw.c',
> + 'test_htw_perf.c',
> 'test_ticketlock.c',
> 'test_trace.c',
> 'test_trace_register.c',
> @@ -193,6 +198,7 @@ fast_tests = [
> ['fib6_autotest', true, true],
> ['func_reentrancy_autotest', false, true],
> ['hash_autotest', true, true],
> + ['htimer_mgr_autotest', true, true],
> ['interrupt_autotest', true, true],
> ['ipfrag_autotest', false, true],
> ['lcores_autotest', true, true],
> @@ -265,6 +271,8 @@ perf_test_names = [
> 'memcpy_perf_autotest',
> 'hash_perf_autotest',
> 'timer_perf_autotest',
> + 'htimer_mgr_perf_autotest',
> + 'htw_perf_autotest',
> 'reciprocal_division',
> 'reciprocal_division_perf',
> 'lpm_perf_autotest',
> diff --git a/app/test/test_htimer_mgr.c b/app/test/test_htimer_mgr.c
> new file mode 100644
> index 0000000000..9e46dec53e
> --- /dev/null
> +++ b/app/test/test_htimer_mgr.c
> @@ -0,0 +1,674 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 Ericsson AB
> + */
> +
> +#include "test.h"
> +
> +#include <sys/queue.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +
> +#include <rte_common.h>
> +#include <rte_cycles.h>
> +#include <rte_htimer_mgr.h>
> +#include <rte_launch.h>
> +#include <rte_lcore.h>
> +#include <rte_random.h>
> +
> +static int
> +timer_lcore(void *arg)
> +{
> + bool *stop = arg;
> +
> + while (!__atomic_load_n(stop, __ATOMIC_RELAXED))
> + rte_htimer_mgr_manage();
> +
> + return 0;
> +}
> +
> +static void
> +count_timer_cb(struct rte_htimer *timer __rte_unused, void *arg)
> +{
> + unsigned int *count = arg;
> +
> + __atomic_fetch_add(count, 1, __ATOMIC_RELAXED);
> +}
> +
> +static void
> +count_async_cb(struct rte_htimer *timer __rte_unused, int result,
> + void *cb_arg)
> +{
> + unsigned int *count = cb_arg;
> +
> + if (result == RTE_HTIMER_MGR_ASYNC_RESULT_ADDED)
> + __atomic_fetch_add(count, 1, __ATOMIC_RELAXED);
> +}
> +
> +static uint64_t
> +s_to_tsc(double s)
> +{
> + return s * rte_get_tsc_hz();
> +}
> +
> +#define ASYNC_ADD_TEST_EXPIRATION_TIME (250*1000) /* ns */
> +#define ASYNC_TEST_TICK (10*1000) /* ns */
> +
> +static int
> +test_htimer_mgr_async_add(unsigned int num_timers_per_lcore)
> +{
> + struct rte_htimer *timers;
> + unsigned int timer_idx;
> + unsigned int lcore_id;
> + bool stop = false;
> + unsigned int timeout_count = 0;
> + unsigned int async_count = 0;
> + unsigned int num_workers = 0;
> + uint64_t expiration_time;
> + unsigned int num_total_timers;
> +
> + rte_htimer_mgr_init(ASYNC_TEST_TICK);
> +
> + RTE_LCORE_FOREACH_WORKER(lcore_id) {
> + if (rte_eal_remote_launch(timer_lcore, &stop, lcore_id) != 0)
> + rte_panic("Unable to launch timer lcore\n");
> + num_workers++;
> + }
> +
> + num_total_timers = num_workers * num_timers_per_lcore;
> +
> + timers = malloc(num_total_timers * sizeof(struct rte_htimer));
> + timer_idx = 0;
> +
> + if (timers == NULL)
> + rte_panic("Unable to allocate heap memory\n");
> +
> + expiration_time = ASYNC_ADD_TEST_EXPIRATION_TIME;
> +
> + RTE_LCORE_FOREACH_WORKER(lcore_id) {
> + unsigned int i;
> +
> + for (i = 0; i < num_timers_per_lcore; i++) {
> + struct rte_htimer *timer = &timers[timer_idx++];
> +
> + for (;;) {
> + int rc;
> +
> + rc = rte_htimer_mgr_async_add(timer, lcore_id,
> + expiration_time,
> + RTE_HTIMER_FLAG_TIME_TSC,
> + count_timer_cb,
> + &timeout_count, 0,
> + count_async_cb,
> + &async_count);
> + if (unlikely(rc == -EBUSY))
> + rte_htimer_mgr_process();
> + else
> + break;
> + }
> + }
> + }
> +
> + while (__atomic_load_n(&async_count, __ATOMIC_RELAXED) !=
> + num_total_timers ||
> + __atomic_load_n(&timeout_count, __ATOMIC_RELAXED) !=
> + num_total_timers)
> + rte_htimer_mgr_manage();
> +
> + __atomic_store_n(&stop, true, __ATOMIC_RELAXED);
> +
> + rte_eal_mp_wait_lcore();
> +
> + rte_htimer_mgr_deinit();
> +
> + free(timers);
> +
> + return TEST_SUCCESS;
> +}
> +
> +struct async_recorder_state {
> + bool timer_cb_run;
> + bool async_add_cb_run;
> + bool async_cancel_cb_run;
> + bool failed;
> +};
> +
> +static void
> +record_async_add_cb(struct rte_htimer *timer __rte_unused,
> + int result, void *cb_arg)
> +{
> + struct async_recorder_state *state = cb_arg;
> +
> + if (state->failed)
> + return;
> +
> + if (state->async_add_cb_run ||
> + result != RTE_HTIMER_MGR_ASYNC_RESULT_ADDED) {
> + puts("async add run already");
> + state->failed = true;
> + }
> +
> + state->async_add_cb_run = true;
> +}
> +
> +static void
> +record_async_cancel_cb(struct rte_htimer *timer __rte_unused,
> + int result, void *cb_arg)
> +{
> + struct async_recorder_state *state = cb_arg;
> +
> + if (state->failed)
> + return;
> +
> + if (state->async_cancel_cb_run) {
> + state->failed = true;
> + return;
> + }
> +
> + switch (result) {
> + case RTE_HTIMER_MGR_ASYNC_RESULT_EXPIRED:
> + if (!state->timer_cb_run)
> + state->failed = true;
> + break;
> + case RTE_HTIMER_MGR_ASYNC_RESULT_CANCELED:
> + if (state->timer_cb_run)
> + state->failed = true;
> + break;
> + case RTE_HTIMER_MGR_ASYNC_RESULT_ALREADY_CANCELED:
> + state->failed = true;
> + }
> +
> + state->async_cancel_cb_run = true;
> +}
> +
> +static int
> +record_check_consistency(struct async_recorder_state *state)
> +{
> + if (state->failed)
> + return -1;
> +
> + return state->async_cancel_cb_run ? 1 : 0;
> +}
> +
> +static int
> +records_check_consistency(struct async_recorder_state *states,
> + unsigned int num_states)
> +{
> + unsigned int i;
> + int canceled = 0;
> +
> + for (i = 0; i < num_states; i++) {
> + int rc;
> +
> + rc = record_check_consistency(&states[i]);
> +
> + if (rc < 0)
> + return -1;
> + canceled += rc;
> + }
> +
> + return canceled;
> +}
> +
> +static void
> +log_timer_expiry_cb(struct rte_htimer *timer __rte_unused,
> + void *arg)
> +{
> + bool *timer_run = arg;
> +
> + *timer_run = true;
> +}
> +
> +
> +#define ASYNC_ADD_CANCEL_TEST_EXPIRATION_TIME_MAX 10e-3 /* s */
> +
> +static int
> +test_htimer_mgr_async_add_cancel(unsigned int num_timers_per_lcore)
> +{
> + struct rte_htimer *timers;
> + struct async_recorder_state *recorder_states;
> + unsigned int timer_idx = 0;
> + unsigned int lcore_id;
> + uint64_t now;
> + unsigned int num_workers = 0;
> + bool stop = false;
> + uint64_t max_expiration_time =
> + s_to_tsc(ASYNC_ADD_CANCEL_TEST_EXPIRATION_TIME_MAX);
> + unsigned int num_total_timers;
> + int canceled = 0;
> +
> + rte_htimer_mgr_init(ASYNC_TEST_TICK);
> +
> + RTE_LCORE_FOREACH_WORKER(lcore_id) {
> + if (rte_eal_remote_launch(timer_lcore, &stop, lcore_id) != 0)
> + rte_panic("Unable to launch timer lcore\n");
> + num_workers++;
> + }
> +
> + num_total_timers = num_workers * num_timers_per_lcore;
> +
> + timers = malloc(num_total_timers * sizeof(struct rte_htimer));
> + recorder_states =
> + malloc(num_total_timers * sizeof(struct async_recorder_state));
> +
> + if (timers == NULL || recorder_states == NULL)
> + rte_panic("Unable to allocate heap memory\n");
> +
> + now = rte_get_tsc_cycles();
> +
> + RTE_LCORE_FOREACH_WORKER(lcore_id) {
> + unsigned int i;
> +
> + for (i = 0; i < num_timers_per_lcore; i++) {
> + struct rte_htimer *timer = &timers[timer_idx];
> + struct async_recorder_state *state =
> + &recorder_states[timer_idx];
> +
> + timer_idx++;
> +
> + *state = (struct async_recorder_state) {};
> +
> + uint64_t expiration_time =
> + now + rte_rand_max(max_expiration_time);
> +
> + for (;;) {
> + int rc;
> +
> + rc = rte_htimer_mgr_async_add(timer, lcore_id,
> + expiration_time,
> + 0,
> + log_timer_expiry_cb,
> + &state->timer_cb_run,
> + 0,
> + record_async_add_cb,
> + state);
> +
> + if (unlikely(rc == -EBUSY))
> + rte_htimer_mgr_process();
> + else
> + break;
> + }
> + }
> + }
> +
> + timer_idx = 0;
> +
> + RTE_LCORE_FOREACH_WORKER(lcore_id) {
> + unsigned int i;
> +
> + for (i = 0; i < num_timers_per_lcore; i++) {
> + struct rte_htimer *timer = &timers[timer_idx];
> + struct async_recorder_state *state =
> + &recorder_states[timer_idx];
> +
> + timer_idx++;
> +
> + /* cancel roughly half of the timers */
> + if (rte_rand_max(2) == 0)
> + continue;
> +
> + for (;;) {
> + int rc;
> +
> + rc = rte_htimer_mgr_async_cancel(timer,
> + record_async_cancel_cb,
> + state);
> +
> + if (unlikely(rc == -EBUSY)) {
> + puts("busy");
> + rte_htimer_mgr_process();
> + } else
> + break;
> + }
> +
> + canceled++;
> + }
> + }
> +
> + for (;;) {
> + int cancel_completed;
> +
> + cancel_completed = records_check_consistency(recorder_states,
> + num_total_timers);
> +
> + if (cancel_completed < 0) {
> + puts("Inconstinency found");
> + return TEST_FAILED;
> + }
> +
> + if (cancel_completed == canceled)
> + break;
> +
> + rte_htimer_mgr_process();
> + }
> +
> + __atomic_store_n(&stop, true, __ATOMIC_RELAXED);
> +
> + rte_eal_mp_wait_lcore();
> +
> + rte_htimer_mgr_deinit();
> +
> + free(timers);
> + free(recorder_states);
> +
> + return TEST_SUCCESS;
> +}
> +
> +/*
> + * This is a test case where one thread asynchronously adds two timers,
> + * with the same expiration time; one on the local lcore and one on a
> + * remote lcore. This creates a tricky situation for the timer
> + * manager, and for the application as well, if the htimer struct is
> + * dynamically allocated.
> + */
> +
> +struct test_timer {
> + uint32_t ref_cnt;
> + uint64_t expiration_time; /* in TSC, not tick */
> + uint32_t *timeout_count;
> + bool *failure_occurred;
> + struct rte_htimer htimer;
> +};
> +
> +
> +static struct test_timer *
> +test_timer_create(uint64_t expiration_time, uint32_t *timeout_count,
> + bool *failure_occurred)
> +{
> + struct test_timer *timer;
> +
> + timer = malloc(sizeof(struct test_timer));
> +
> + if (timer == NULL)
> + rte_panic("Unable to allocate timer memory\n");
> +
> + timer->ref_cnt = 1;
> + timer->expiration_time = expiration_time;
> + timer->timeout_count = timeout_count;
> + timer->failure_occurred = failure_occurred;
> +
> + return timer;
> +}
> +
> +static void
> +test_timer_inc_ref_cnt(struct test_timer *timer)
> +{
> + __atomic_add_fetch(&timer->ref_cnt, 1, __ATOMIC_RELEASE);
__atomic_fetch_add instead please
there's future work to align with C11 atomics using the previous
__atomic_fetch_<op> is preferred because it just becomes
s/__atomic/atomic/ (well mostly...)
> +}
> +
> +static void
> +test_timer_dec_ref_cnt(struct test_timer *timer)
> +{
> + if (timer != NULL) {
> + uint32_t cnt = __atomic_sub_fetch(&timer->ref_cnt, 1,
> + __ATOMIC_RELEASE);
same here
i'll try to get a patch up for checkpatches warning soon.
thanks!
next prev parent reply other threads:[~2023-03-16 3:55 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-28 9:39 [RFC 0/2] Add " Mattias Rönnblom
2023-02-28 9:39 ` [RFC 1/2] eal: add bitset type Mattias Rönnblom
2023-02-28 18:46 ` Tyler Retzlaff
2023-03-02 6:31 ` Mattias Rönnblom
2023-03-02 20:39 ` Tyler Retzlaff
2023-02-28 9:39 ` [RFC 2/2] eal: add high-performance timer facility Mattias Rönnblom
2023-03-05 17:25 ` Stephen Hemminger
2023-03-09 15:20 ` Mattias Rönnblom
2023-02-28 16:01 ` [RFC 0/2] Add " Morten Brørup
2023-03-01 11:18 ` Mattias Rönnblom
2023-03-01 13:31 ` Morten Brørup
2023-03-01 15:50 ` Mattias Rönnblom
2023-03-01 17:06 ` Morten Brørup
2023-03-15 17:03 ` [RFC v2 " Mattias Rönnblom
2023-03-15 17:03 ` [RFC v2 1/2] eal: add bitset type Mattias Rönnblom
2023-03-15 17:20 ` Stephen Hemminger
2023-03-15 18:27 ` Mattias Rönnblom
2023-03-15 17:03 ` [RFC v2 2/2] eal: add high-performance timer facility Mattias Rönnblom
2023-03-16 3:55 ` Tyler Retzlaff [this message]
2023-03-17 1:58 ` Stephen Hemminger
2023-03-22 12:18 ` Morten Brørup
2023-04-03 12:04 ` Mattias Rönnblom
2023-04-04 7:32 ` Morten Brørup
2023-03-24 16:00 ` Morten Brørup
2023-07-06 22:41 ` Stephen Hemminger
2023-07-12 8:58 ` Mattias Rönnblom
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230316035556.GB5703@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net \
--to=roretzla@linux.microsoft.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
--cc=maria.lingemark@ericsson.com \
--cc=mattias.ronnblom@ericsson.com \
--cc=mb@smartsharesystems.com \
--cc=stefan.sundkvist@ericsson.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).