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

[Jerin Jacob <jerinjacobk@gmail.com>]

On Thu, Apr 9, 2020 at 8:33 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 above section needs to be rewritten wrt v2 changes.

> 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.]

See below


>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
See above

Move [This patch will be used for next patch.] here to avoid
unnecessary information in the git commit.

> v2:
> * rename rte_last_init_register ->rte_init_register
> * rename rte_last_init struct ->rte_init
> * rename rte_init_cb ->rte_init_cb_t
> * free the rte_init node when not used.
> * remove rte_init and others to eal_private.h
> * add comments
> * fix checkpatch warning
> ---
> diff --git a/lib/librte_eal/include/rte_init.h b/lib/librte_eal/include/rte_init.h
> new file mode 100644
> index 0000000..636efff
> --- /dev/null
> +++ b/lib/librte_eal/include/rte_init.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright 2020 DPDK Community
> + */
> +
> +#ifndef _RTE_INIT_H_
> +#define _RTE_INIT_H_

@file section is missing. See
lib/librte_eal/common/include/rte_errno.h as example.


> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdio.h>
> +#include <sys/queue.h>

<sys/queue.h> is not required in public API header file.

> +
> +/**
> + * Implementation specific callback function which is
> + * responsible for specificed initialization.
> + *
> + * This is called when almost resources are available.
> + *
> + * @return
> + *     0 for successful callback
> + *     Negative for unsuccessful callback with error value
> + */
> +typedef int (*rte_init_cb_t)(const void *arg);
> +
> +/**
> + * rte_init type.
> + *
> + * The rte_init of RTE_INIT_PRE are called firstly,
> + * and then RTE_INIT_POST.
> + */
> +enum rte_init_type {
> +       RTE_INIT_PRE,

Type specific comment is missing.

Example as reference for formatting.

/**
 * Enumerate trace mode operation.
 */
enum rte_trace_mode_e {
        /**
         * In this mode, When no space left in trace buffer, the subsequent
         * events overwrite the old events in the trace buffer.
         */
        RTE_TRACE_MODE_OVERWRITE,
        /**
         * In this mode, When no space left on trace buffer, the subsequent
         * events shall not be recorded in the trace buffer.
         */
        RTE_TRACE_MODE_DISCARD,
};

> +       RTE_INIT_POST
> +};


> +
> +/**
> + * Register a rte_init callback.
> + *
> + * @param cb
> + *   A pointer to a rte_init_cb structure, which will be used

s/used/invoked?

> + *   in rte_eal_init().
> + *
> + * @param arg
> + *   The cb will use that as param.
> + *
> + * @param type
> + *   The type of rte_init registered.
> + */
> +
> +void rte_init_register(rte_init_cb_t cb, const void *arg,
> +                      enum rte_init_type type);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_INIT_H_ */