Re: [dpdk-dev] [PATCH dpdk-dev 1/2] eal: introduce last-init queue for libraries initialization

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Apr 9, 2020 at 4:24 PM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch introduces last-init queue, user can register a
> callback for theirs initialization. Running rte_last_init_run(),
> the almost resource of DPDK are available, such as memzone, ring.
> With this way, user don't introduce additional codes in eal layer.
>
> [This patch will be used for next patch.]
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
>  lib/librte_eal/common/eal_common_last_init.c | 43 ++++++++++++++++++++++++++++
>  lib/librte_eal/common/meson.build            |  1 +
>  lib/librte_eal/freebsd/Makefile              |  1 +
>  lib/librte_eal/freebsd/eal.c                 |  7 +++++
>  lib/librte_eal/include/rte_last_init.h       | 36 +++++++++++++++++++++++

Update doc/api/doxy-api-index.md and hook to documenion.

> +void
> +rte_last_init_register(rte_last_init_cb cb, const void *arg)
> +{
> +       struct rte_last_init *last;
> +
> +       RTE_VERIFY(cb);
> +
> +       last = malloc(sizeof(*last));

Does not look like this memory is NOT freed anywhere.

> +       if (last == NULL)
> +               rte_panic("Alloc memory for rte_last_init node failed\n");
> +
> +       last->cb = cb;
> +       last->arg = arg;
> +
> +       TAILQ_INSERT_TAIL(&rte_last_init_list, last, next);
> +}
> +
> +int
> +rte_last_init_run(void)
> +{
> +       struct rte_last_init *init;
> +       int ret;
> +
> +       TAILQ_FOREACH(init, &rte_last_init_list, next) {
> +               ret = init->cb(init->arg);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       return 0;
> +}
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build

> +typedef int (*rte_last_init_cb)(const void *arg);
> +
> +/**
> + * A structure describing a generic initialization.
> + */
> +struct rte_last_init {
> +       TAILQ_ENTRY(rte_last_init) next;   /**< Next bus object in linked list */
> +       const void *arg;
> +       rte_last_init_cb cb;
> +};

No need to expose this structure. Move to eal_private.h


> +
> +/** Double linked list of buses */
> +TAILQ_HEAD(rte_last_init_list, rte_last_init);

No need to expose this structure. Move to eal_private.h

> +
> +void rte_last_init_register(rte_last_init_cb cb, const void *arg);

Add Doxygen comment.

Should we change to rte_init_register()? add an enum to specify where to
call this RTE_INIT_PRE, RTE_INIT_POST to take care of the future needs.
Just thought of bringing this point to make sure we can use this
scheme for another use case IF ANY.


> +int rte_last_init_run(void);

No need to expose this function in public API. Move to eal_private.h.
Please start the internal functions with eal_(dont use rte_ for
internal functions)