[dpdk-dev] [PATCH v2 1/2] timer: allow timer management in shared memory
Stephen Hemminger
stephen at networkplumber.org
Fri Dec 7 19:10:11 CET 2018
On Fri, 7 Dec 2018 11:52:59 -0600
Erik Gabriel Carrillo <erik.g.carrillo at intel.com> wrote:
> Currently, the timer library uses a per-process table of structures to
> manage skiplists of timers presumably because timers contain arbitrary
> function pointers whose value may not resolve properly in other
> processes.
>
> However, if the same callback is used handle all timers, and that
> callback is only invoked in one process, then it woud be safe to allow
> the data structures to be allocated in shared memory, and to allow
> secondary processes to modify the timer lists. This would let timers be
> used in more multi-process scenarios.
>
> The library's global variables are wrapped with a struct, and an array
> of these structures is created in shared memory. The original APIs
> are updated to reference the zeroth entry in the array. This maintains
> the original behavior for both primary and secondary processes since
> the set intersection of their coremasks should be empty [1]. New APIs
> are introduced to enable the allocation/deallocation of other entries
> in the array.
>
> New variants of the APIs used to start and stop timers are introduced;
> they allow a caller to specify which array entry should be used to
> locate the timer list to insert into or delete from.
>
> Finally, a new variant of rte_timer_manage() is introduced, which
> allows a caller to specify which array entry should be used to locate
> the timer lists to process; it can also process multiple timer lists per
> invocation.
>
> [1] https://doc.dpdk.org/guides/prog_guide/multi_proc_support.html#multi-process-limitations
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo at intel.com>
Makes sense but it looks to me like an ABI breakage. Experimental isn't going to
work for this.
> +static uint32_t default_data_id; // id set to zero automatically
C++ style comments are not allowed per DPDK coding style.
Best to just drop the comment, it is stating the obvious.
> -/* Init the timer library. */
> +static inline int
> +timer_data_valid(uint32_t id)
> +{
> + return !!(rte_timer_data_arr[id].internal_flags & FL_ALLOCATED);
> +}
Don't need inline on static functions.
...
> +MAP_STATIC_SYMBOL(int rte_timer_manage(void), rte_timer_manage_v1902);
> +BIND_DEFAULT_SYMBOL(rte_timer_manage, _v1902, 19.02);
> +
> +int __rte_experimental
> +rte_timer_alt_manage(uint32_t timer_data_id,
> + unsigned int *poll_lcores,
> + int nb_poll_lcores,
> + rte_timer_alt_manage_cb_t f)
> +{
> + union rte_timer_status status;
> + struct rte_timer *tim, *next_tim, **pprev;
> + struct rte_timer *run_first_tims[RTE_MAX_LCORE];
> + unsigned int runlist_lcore_ids[RTE_MAX_LCORE];
> + unsigned int this_lcore = rte_lcore_id();
> + struct rte_timer *prev[MAX_SKIPLIST_DEPTH + 1];
> + uint64_t cur_time;
> + int i, j, ret;
> + int nb_runlists = 0;
> + struct rte_timer_data *data;
> + struct priv_timer *privp;
> + uint32_t poll_lcore;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, data, -EINVAL);
> +
> + /* timer manager only runs on EAL thread with valid lcore_id */
> + assert(this_lcore < RTE_MAX_LCORE);
> +
> + __TIMER_STAT_ADD(data->priv_timer, manage, 1);
> +
> + if (poll_lcores == NULL) {
> + poll_lcores = (unsigned int []){rte_lcore_id()};
This isn't going to be safe. It assigns poll_lcores to an array
allocated on the stack.
> +
> + for (i = 0, poll_lcore = poll_lcores[i]; i < nb_poll_lcores;
> + poll_lcore = poll_lcores[++i]) {
> + privp = &data->priv_timer[poll_lcore];
> +
> + /* optimize for the case where per-cpu list is empty */
> + if (privp->pending_head.sl_next[0] == NULL)
> + continue;
> + cur_time = rte_get_timer_cycles();
> +
> +#ifdef RTE_ARCH_64
> + /* on 64-bit the value cached in the pending_head.expired will
> + * be updated atomically, so we can consult that for a quick
> + * check here outside the lock
> + */
> + if (likely(privp->pending_head.expire > cur_time))
> + continue;
> +#endif
This code needs to be optimized so that application can call this at a very
high rate without performance impact.
More information about the dev
mailing list