From: "Mattias Rönnblom" <mattias.ronnblom@ericsson.com>
To: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v3 1/1] eventdev: add new software event timer adapter
Date: Fri, 14 Dec 2018 22:15:05 +0100 [thread overview]
Message-ID: <e4112ea0-bcac-64fe-4bc2-37de7dac0e85@ericsson.com> (raw)
In-Reply-To: <1544802346-1249-2-git-send-email-erik.g.carrillo@intel.com>
On 2018-12-14 16:45, Erik Gabriel Carrillo wrote:
> This patch introduces a new version of the event timer adapter software
> PMD. In the original design, timer event producer lcores in the primary
> and secondary processes enqueued event timers into a ring, and a
> service core in the primary process dequeued them and processed them
> further. To improve performance, this version does away with the ring
> and lets lcores in both primary and secondary processes insert timers
> directly into timer skiplist data structures; the service core directly
> accesses the lists as well, when looking for timers that have expired.
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> ---
> lib/librte_eventdev/rte_event_timer_adapter.c | 688 +++++++++++---------------
> 1 file changed, 276 insertions(+), 412 deletions(-)
>
> diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c b/lib/librte_eventdev/rte_event_timer_adapter.c
> index 79070d4..029a45a 100644
> --- a/lib/librte_eventdev/rte_event_timer_adapter.c
> +++ b/lib/librte_eventdev/rte_event_timer_adapter.c
> @@ -19,6 +19,7 @@
> #include <rte_timer.h>
> #include <rte_service_component.h>
> #include <rte_cycles.h>
> +#include <rte_random.h>
You aren't using anything from rte_random.h.
/../
> -static __rte_always_inline uint16_t
> -__sw_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
> - struct rte_event_timer **evtims,
> - uint16_t nb_evtims)
> +static uint16_t
> +__swtim_arm_burst(const struct rte_event_timer_adapter *adapter,
> + struct rte_event_timer **evtims,
> + uint16_t nb_evtims)
> {
> - uint16_t i;
> - int ret;
> - struct rte_event_timer_adapter_sw_data *sw_data;
> - struct msg *msgs[nb_evtims];
> + int i, ret;
> + struct swtim *sw = swtim_pmd_priv(adapter);
> + uint32_t lcore_id = rte_lcore_id();
> + struct rte_timer *tim, *tims[nb_evtims];
> + uint64_t cycles;
>
> #ifdef RTE_LIBRTE_EVENTDEV_DEBUG
> /* Check that the service is running. */
> @@ -1101,101 +979,104 @@ __sw_event_timer_arm_burst(const struct rte_event_timer_adapter *adapter,
> }
> #endif
>
> - sw_data = adapter->data->adapter_priv;
> + /* Adjust lcore_id if non-EAL thread. Arbitrarily pick the timer list of
> + * the highest lcore to insert such timers into
> + */
> + if (lcore_id == LCORE_ID_ANY)
> + lcore_id = RTE_MAX_LCORE - 1;
> +
> + /* If this is the first time we're arming an event timer on this lcore,
> + * mark this lcore as "in use"; this will cause the service
> + * function to process the timer list that corresponds to this lcore.
> + */
> + if (unlikely(rte_atomic16_test_and_set(&sw->in_use[lcore_id].v))) {
> + rte_spinlock_lock(&sw->poll_lcores_sl);
> + EVTIM_LOG_DBG("Adding lcore id = %u to list of lcores to poll",
> + lcore_id);
> + sw->poll_lcores[sw->n_poll_lcores++] = lcore_id;
> + rte_spinlock_unlock(&sw->poll_lcores_sl);
> + }
>
> - ret = rte_mempool_get_bulk(sw_data->msg_pool, (void **)msgs, nb_evtims);
> + ret = rte_mempool_get_bulk(sw->tim_pool, (void **)tims,
> + nb_evtims);
> if (ret < 0) {
> rte_errno = ENOSPC;
> return 0;
> }
>
> - /* Let the service know we're producing messages for it to process */
> - rte_atomic16_inc(&sw_data->message_producer_count);
> -
> - /* If the service is managing timers, wait for it to finish */
> - while (sw_data->service_phase == 2)
> - rte_pause();
> -
> - rte_smp_rmb();
> -
> for (i = 0; i < nb_evtims; i++) {
> /* Don't modify the event timer state in these cases */
> if (evtims[i]->state == RTE_EVENT_TIMER_ARMED) {
> rte_errno = EALREADY;
> break;
> } else if (!(evtims[i]->state == RTE_EVENT_TIMER_NOT_ARMED ||
> - evtims[i]->state == RTE_EVENT_TIMER_CANCELED)) {
> + evtims[i]->state == RTE_EVENT_TIMER_CANCELED)) {
> rte_errno = EINVAL;
> break;
> }
>
> ret = check_timeout(evtims[i], adapter);
> - if (ret == -1) {
> + if (unlikely(ret == -1)) {
> evtims[i]->state = RTE_EVENT_TIMER_ERROR_TOOLATE;
> rte_errno = EINVAL;
> break;
> - }
> - if (ret == -2) {
> + } else if (unlikely(ret == -2)) {
> evtims[i]->state = RTE_EVENT_TIMER_ERROR_TOOEARLY;
> rte_errno = EINVAL;
> break;
> }
>
> - if (check_destination_event_queue(evtims[i], adapter) < 0) {
> + if (unlikely(check_destination_event_queue(evtims[i],
> + adapter) < 0)) {
> evtims[i]->state = RTE_EVENT_TIMER_ERROR;
> rte_errno = EINVAL;
> break;
> }
>
> - /* Checks passed, set up a message to enqueue */
> - msgs[i]->type = MSG_TYPE_ARM;
> - msgs[i]->evtim = evtims[i];
> + tim = tims[i];
> + rte_timer_init(tim);
>
> - /* Set the payload pointer if not set. */
> - if (evtims[i]->ev.event_ptr == NULL)
> - evtims[i]->ev.event_ptr = evtims[i];
> + evtims[i]->impl_opaque[0] = (uintptr_t)tim;
> + evtims[i]->impl_opaque[1] = (uintptr_t)adapter;
>
> - /* msg objects that get enqueued successfully will be freed
> - * either by a future cancel operation or by the timer
> - * expiration callback.
> - */
> - if (rte_ring_enqueue(sw_data->msg_ring, msgs[i]) < 0) {
> - rte_errno = ENOSPC;
> + cycles = get_timeout_cycles(evtims[i], adapter);
> + ret = rte_timer_alt_reset(sw->timer_data_id, tim, cycles,
> + SINGLE, lcore_id, NULL, evtims[i]);
> + if (ret < 0) {
> + /* tim was in RUNNING or CONFIG state */
> + evtims[i]->state = RTE_EVENT_TIMER_ERROR;
> break;
> }
>
> - EVTIM_LOG_DBG("enqueued ARM message to ring");
> -
> + rte_smp_wmb();
> + EVTIM_LOG_DBG("armed an event timer");
> evtims[i]->state = RTE_EVENT_TIMER_ARMED;
This looks like you want a reader to see the impl_opaque[] stores,
before the state store, which sounds like a good idea.
However, I fail to find the corresponding read barriers on the reader
side. Shouldn't swtim_cancel_burst() have such? It's loading state, and
loading impl_opaque[], but there's no guarantee those memory loads
happens in program order.
next prev parent reply other threads:[~2018-12-14 21:15 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-29 23:35 [dpdk-dev] [PATCH 0/3] " Erik Gabriel Carrillo
2018-11-29 23:35 ` [dpdk-dev] [PATCH 1/3] timer: allow timer management in shared memory Erik Gabriel Carrillo
2018-11-29 23:35 ` [dpdk-dev] [PATCH 2/3] timer: add function to stop all timers in a list Erik Gabriel Carrillo
2018-11-29 23:35 ` [dpdk-dev] [PATCH 3/3] eventdev: add new software event timer adapter Erik Gabriel Carrillo
2018-11-30 7:26 ` [dpdk-dev] [PATCH 0/3] " Pavan Nikhilesh
2018-11-30 19:07 ` Carrillo, Erik G
2018-12-07 17:52 ` [dpdk-dev] [PATCH v2 0/2] Timer library changes Erik Gabriel Carrillo
2018-12-07 17:52 ` [dpdk-dev] [PATCH v2 1/2] timer: allow timer management in shared memory Erik Gabriel Carrillo
2018-12-07 18:10 ` Stephen Hemminger
2018-12-07 19:21 ` Carrillo, Erik G
2018-12-07 17:53 ` [dpdk-dev] [PATCH v2 2/2] timer: add function to stop all timers in a list Erik Gabriel Carrillo
2018-12-13 22:26 ` [dpdk-dev] [PATCH v3 0/2] Timer library changes Erik Gabriel Carrillo
2018-12-13 22:26 ` [dpdk-dev] [PATCH v3 1/2] timer: allow timer management in shared memory Erik Gabriel Carrillo
2018-12-13 22:26 ` [dpdk-dev] [PATCH v3 2/2] timer: add function to stop all timers in a list Erik Gabriel Carrillo
2018-12-19 3:35 ` [dpdk-dev] [PATCH v3 0/2] Timer library changes Thomas Monjalon
2018-12-19 7:33 ` Mattias Rönnblom
2019-03-05 22:41 ` Carrillo, Erik G
2019-03-05 22:58 ` [dpdk-dev] [dpdk-techboard] " Thomas Monjalon
2019-03-06 18:54 ` Carrillo, Erik G
2019-03-06 20:17 ` Thomas Monjalon
2019-03-06 2:39 ` [dpdk-dev] " Varghese, Vipin
2019-03-06 15:15 ` Carrillo, Erik G
2019-03-07 2:33 ` Varghese, Vipin
2019-03-06 17:20 ` [dpdk-dev] [PATCH v4 " Erik Gabriel Carrillo
2019-03-06 17:20 ` [dpdk-dev] [PATCH v4 1/2] timer: allow timer management in shared memory Erik Gabriel Carrillo
2019-03-20 13:52 ` Sanford, Robert
2019-03-20 13:52 ` Sanford, Robert
2019-03-21 1:01 ` Carrillo, Erik G
2019-03-21 1:01 ` Carrillo, Erik G
2019-03-27 14:03 ` Thomas Monjalon
2019-03-27 14:03 ` Thomas Monjalon
2019-03-28 12:42 ` Carrillo, Erik G
2019-03-28 12:42 ` Carrillo, Erik G
2019-04-15 21:49 ` Carrillo, Erik G
2019-04-15 21:49 ` Carrillo, Erik G
2019-03-06 17:20 ` [dpdk-dev] [PATCH v4 2/2] timer: add function to stop all timers in a list Erik Gabriel Carrillo
2019-04-15 21:41 ` [dpdk-dev] [PATCH v5 0/2] Timer library changes Erik Gabriel Carrillo
2019-04-15 21:41 ` Erik Gabriel Carrillo
2019-04-15 21:41 ` [dpdk-dev] [PATCH v5 1/2] timer: allow timer management in shared memory Erik Gabriel Carrillo
2019-04-15 21:41 ` Erik Gabriel Carrillo
2019-04-17 17:09 ` Thomas Monjalon
2019-04-17 17:09 ` Thomas Monjalon
2019-04-15 21:41 ` [dpdk-dev] [PATCH v5 2/2] timer: add function to stop all timers in a list Erik Gabriel Carrillo
2019-04-15 21:41 ` Erik Gabriel Carrillo
2019-04-17 19:54 ` [dpdk-dev] [PATCH v5 0/2] Timer library changes Thomas Monjalon
2019-04-17 19:54 ` Thomas Monjalon
2018-12-07 20:34 ` [dpdk-dev] [PATCH v2 0/1] New software event timer adapter Erik Gabriel Carrillo
2018-12-07 20:34 ` [dpdk-dev] [PATCH v2 1/1] eventdev: add new " Erik Gabriel Carrillo
2018-12-09 19:17 ` Mattias Rönnblom
2018-12-10 17:17 ` Carrillo, Erik G
2018-12-14 15:45 ` [dpdk-dev] [PATCH v3 0/1] New " Erik Gabriel Carrillo
2018-12-14 15:45 ` [dpdk-dev] [PATCH v3 1/1] eventdev: add new " Erik Gabriel Carrillo
2018-12-14 21:15 ` Mattias Rönnblom [this message]
2018-12-14 23:04 ` Carrillo, Erik G
2018-12-14 23:15 ` [dpdk-dev] [PATCH v4 0/1] New " Erik Gabriel Carrillo
2018-12-14 23:15 ` [dpdk-dev] [PATCH v4 1/1] eventdev: add new " Erik Gabriel Carrillo
2018-12-18 20:11 ` [dpdk-dev] [EXT] [PATCH v4 0/1] New " Jerin Jacob Kollanukkaran
2018-12-18 20:14 ` Carrillo, Erik G
2019-04-22 14:57 ` [dpdk-dev] [PATCH v5 " Erik Gabriel Carrillo
2019-04-22 14:57 ` Erik Gabriel Carrillo
2019-04-22 14:57 ` [dpdk-dev] [PATCH v5 1/1] eventdev: add new " Erik Gabriel Carrillo
2019-04-22 14:57 ` Erik Gabriel Carrillo
2019-04-26 15:14 ` [dpdk-dev] [PATCH v6 0/1] New " Erik Gabriel Carrillo
2019-04-26 15:14 ` Erik Gabriel Carrillo
2019-04-26 15:14 ` [dpdk-dev] [PATCH v6 1/1] eventdev: add new " Erik Gabriel Carrillo
2019-04-26 15:14 ` Erik Gabriel Carrillo
2019-04-26 18:51 ` Honnappa Nagarahalli
2019-04-26 18:51 ` Honnappa Nagarahalli
2019-04-26 18:58 ` Carrillo, Erik G
2019-04-26 18:58 ` Carrillo, Erik G
2019-06-05 13:34 ` Jerin Jacob Kollanukkaran
2019-06-19 15:14 ` [dpdk-dev] [PATCH v7 0/1] New " Erik Gabriel Carrillo
2019-06-19 15:14 ` [dpdk-dev] [PATCH v7 1/1] eventdev: add new " Erik Gabriel Carrillo
2019-06-19 16:25 ` [dpdk-dev] [PATCH v8 0/1] New " Erik Gabriel Carrillo
2019-06-19 16:25 ` [dpdk-dev] [PATCH v8 1/1] eventdev: add new " Erik Gabriel Carrillo
2019-06-24 6:12 ` Jerin Jacob Kollanukkaran
2019-06-25 6:06 ` Jerin Jacob Kollanukkaran
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=e4112ea0-bcac-64fe-4bc2-37de7dac0e85@ericsson.com \
--to=mattias.ronnblom@ericsson.com \
--cc=dev@dpdk.org \
--cc=erik.g.carrillo@intel.com \
/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).