On 4/24/2023 2:02 PM, Chengwen Feng wrote:
> This patch adds coroutine library. The main elements are:
> 1. scheduler: container of coroutines, which is responsible for
> scheduling coroutine.
> 2. coroutine: Minimum scheduling unit, it should associated to one
> scheduler.
>
> In the coroutine callback, application could invoke rte_co_yield() to
> give up the CPU, and invoke rte_co_delay() to delay the specified
> microsecond.
>
> Signed-off-by: Chengwen Feng <fengcheng...@huawei.com>
<...>
> +++ b/lib/coroutine/rte_coroutine.c
> @@ -0,0 +1,190 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2023 HiSilicon Limited
> + */
> +
> +#include <pthread.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <ucontext.h>
> +
> +#include <rte_alarm.h>
> +#include <rte_ring.h>
> +
> +#include "rte_coroutine.h"
> +#include "rte_coroutine_imp.h"
> +
> +#define FATAL(fmt, args...) printf("[FATAL] %s() %d: " fmt "\n", __func__,
> __LINE__, ##args)
> +
No 'printf' for logging please.
<...>
> +int
> +rte_schedule_run(struct rte_schedule *s)
> +{
> + struct rte_cocontext *co = NULL;
> + uintptr_t ptr;
> +
> + /* Set local thread variable as input argument. */
> + co_schedule = s;
> +
> + while (!rte_ring_empty(s->ring)) {
> + rte_ring_dequeue(s->ring, (void **)&co);
> + if (co->state == COROUTINE_READY) {
> + getcontext(&co->ctx);
> + co->ctx.uc_stack.ss_sp = co->stack;
> + co->ctx.uc_stack.ss_size = co->stack_sz;
> + co->ctx.uc_link = &s->main;
> + co->state = COROUTINE_RUNNING;
> + s->running = co;
> + ptr = (uintptr_t)co;
> + makecontext(&co->ctx, (void (*)(void))co_run_func, 2,
> + (uint32_t)ptr, (uint32_t)(ptr >> 32));
Why passing 'co' address as two 32bit values? Why not just pass address
of it?
Also can it be possible to set context to 'co->cb()' here and do the
cleanup after context returned to 's->main' (below)?
> + swapcontext(&s->main, &co->ctx);
> + } else if (co->state == COROUTINE_SUSPEND) {
> + co->state = COROUTINE_RUNNING;
> + s->running = co;
> + swapcontext(&s->main, &co->ctx);
> + } else {
> + FATAL("invalid state!");
> + }
> + }
> +
> + while (s->yield_head != NULL) {
> + co = s->yield_head;
> + s->yield_head = co->yield_next;
> + if (co->state == COROUTINE_YIELD) {
> + co->state = COROUTINE_RUNNING;
> + s->running = co;
> + swapcontext(&s->main, &co->ctx);
> + } else {
> + FATAL("invalid yield state!");
> + }
> + }
> +
As coroutines in 'ready' state stored in a ring, same can be done for
the coroutines in 'yield' state, instead of using a linked list, to
simplify the flow. Just a question, both works fine.
> + return 0;
> +}
> +
> +int
> +rte_co_create(struct rte_schedule *s, coroutine_callback_t cb, void *arg,
> uint32_t stack_sz)
> +{
> + struct rte_cocontext *co = calloc(1, sizeof(struct rte_cocontext));
> + int ret;
> + if (co == NULL)
> + return -ENOMEM;
> +
I guess somewhere here should check the 's->max_coroutines';
