From: "Mattias Rönnblom" <hofors@lysator.liu.se>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>, dev@dpdk.org
Cc: thomas@monjalon.net, david.marchand@redhat.com,
stephen@networkplumber.org, olivier.matz@6wind.com,
mb@smartsharesystems.com
Subject: Re: [PATCH v2 1/3] eal: add rte control thread create API
Date: Wed, 7 Dec 2022 10:13:39 +0100 [thread overview]
Message-ID: <5b080d72-f7a8-232d-86a1-9164a005fcfb@lysator.liu.se> (raw)
In-Reply-To: <1670347706-23802-2-git-send-email-roretzla@linux.microsoft.com>
On 2022-12-06 18:28, Tyler Retzlaff wrote:
> Add rte_control_thread_create API as a replacement for
> rte_ctrl_thread_create to allow deprecation of the use of platform
> specific types in DPDK public API.
>
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
> lib/eal/common/eal_common_thread.c | 93 ++++++++++++++++++++++++++++++++++----
> lib/eal/include/rte_thread.h | 29 ++++++++++++
> lib/eal/version.map | 3 ++
> 3 files changed, 117 insertions(+), 8 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index c5d8b43..7950b50 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -234,7 +234,10 @@ enum __rte_ctrl_thread_status {
> };
>
> struct rte_thread_ctrl_params {
> - void *(*start_routine)(void *);
> + union {
> + void *(*start_routine)(void *);
> + rte_thread_func thread_func;
To be consistent with function naming scheme, where "ctrl" is the old
API, and "control" the new, you could call the start functions something
with "ctrl" and "control" as well.
Maybe it's worth mentioning that fact somewhere in the beginning of the
file, as a comment (i.e., that "ctrl" denotes the old API).
> + } u;
> void *arg;
> int ret;
Why is 'ret' needed? (This question is unrelated to your patch.)
> /* Control thread status.
> @@ -243,14 +246,12 @@ struct rte_thread_ctrl_params {
> enum __rte_ctrl_thread_status ctrl_thread_status;
> };
>
> -static void *ctrl_thread_init(void *arg)
> +static int ctrl_thread_init(void *arg)
> {
> struct internal_config *internal_conf =
> eal_get_internal_configuration();
> rte_cpuset_t *cpuset = &internal_conf->ctrl_cpuset;
> struct rte_thread_ctrl_params *params = arg;
> - void *(*start_routine)(void *) = params->start_routine;
> - void *routine_arg = params->arg;
>
> __rte_thread_init(rte_lcore_id(), cpuset);
> params->ret = pthread_setaffinity_np(pthread_self(), sizeof(*cpuset),
> @@ -258,13 +259,35 @@ static void *ctrl_thread_init(void *arg)
> if (params->ret != 0) {
> __atomic_store_n(¶ms->ctrl_thread_status,
> CTRL_THREAD_ERROR, __ATOMIC_RELEASE);
> - return NULL;
> + return params->ret;
> }
>
> __atomic_store_n(¶ms->ctrl_thread_status,
> CTRL_THREAD_RUNNING, __ATOMIC_RELEASE);
>
> - return start_routine(routine_arg);
> + return 0;
> +}
> +
> +static void *ctrl_thread_start(void *arg)
> +{
> + struct rte_thread_ctrl_params *params = arg;
> + void *(*start_routine)(void *) = params->u.start_routine;
> +
> + if (ctrl_thread_init(arg) != 0)
> + return NULL;
> +
> + return start_routine(params->arg);
> +}
> +
> +static uint32_t control_thread_start(void *arg)
> +{
> + struct rte_thread_ctrl_params *params = arg;
> + rte_thread_func start_routine = params->u.thread_func;
> +
> + if (ctrl_thread_init(arg) != 0)
> + return params->ret;
> +
> + return start_routine(params->arg);
> }
>
> int
> @@ -280,12 +303,12 @@ static void *ctrl_thread_init(void *arg)
> if (!params)
> return -ENOMEM;
>
> - params->start_routine = start_routine;
> + params->u.start_routine = start_routine;
> params->arg = arg;
> params->ret = 0;
> params->ctrl_thread_status = CTRL_THREAD_LAUNCHING;
>
> - ret = pthread_create(thread, attr, ctrl_thread_init, (void *)params);
> + ret = pthread_create(thread, attr, ctrl_thread_start, (void *)params);
> if (ret != 0) {
> free(params);
> return -ret;
> @@ -322,6 +345,60 @@ static void *ctrl_thread_init(void *arg)
> }
>
> int
> +rte_control_thread_create(rte_thread_t *thread, const char *name,
> + const rte_thread_attr_t *attr,
> + rte_thread_func start_routine, void *arg)
> +{
> + struct rte_thread_ctrl_params *params;
> + enum __rte_ctrl_thread_status ctrl_thread_status;
> + int ret;
> +
> + params = malloc(sizeof(*params));
> + if (!params)
params == NULL
> + return -ENOMEM;
> +
> + params->u.thread_func = start_routine;
> + params->arg = arg;
> + params->ret = 0;
> + params->ctrl_thread_status = CTRL_THREAD_LAUNCHING;
> +
> + ret = rte_thread_create(thread, attr, control_thread_start, params);
> + if (ret != 0) {
> + free(params);
> + return -ret;
> + }
> +
> + if (name != NULL) {
> + ret = rte_thread_setname((pthread_t)thread->opaque_id, name);
> + if (ret < 0)
> + RTE_LOG(DEBUG, EAL,
> + "Cannot set name for ctrl thread\n");
> + }
> +
> + /* Wait for the control thread to initialize successfully */
> + while ((ctrl_thread_status =
> + __atomic_load_n(¶ms->ctrl_thread_status,
> + __ATOMIC_ACQUIRE)) == CTRL_THREAD_LAUNCHING) {
> + /* Yield the CPU. Using sched_yield call requires maintaining
> + * another implementation for Windows as sched_yield is not
> + * supported on Windows.
> + */
sched_yield() also doesn't necessarily yield the CPU.
> + rte_delay_us_sleep(1);
> + }
> +
> + /* Check if the control thread encountered an error */
> + if (ctrl_thread_status == CTRL_THREAD_ERROR) {
> + /* ctrl thread is exiting */
> + rte_thread_join(*thread, NULL);
> + }
> +
> + ret = params->ret;
> + free(params);
> +
> + return ret;
> +}
> +
> +int
> rte_thread_register(void)
> {
> unsigned int lcore_id;
> diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h
> index b9edf70..ae7afbe 100644
> --- a/lib/eal/include/rte_thread.h
> +++ b/lib/eal/include/rte_thread.h
> @@ -95,6 +95,35 @@ int rte_thread_create(rte_thread_t *thread_id,
> rte_thread_func thread_func, void *arg);
>
> /**
> + * Create a control thread.
> + *
> + * Creates a control thread with the given name and attributes. The
> + * affinity of the new thread is based on the CPU affinity retrieved
> + * at the time rte_eal_init() was called, the dataplane and service
> + * lcores are then excluded. If setting the name of the thread fails,
"the EAL threads are then excluded"
> + * the error is ignored and a debug message is logged.
> + *
> + * @param thread
> + * Filled with the thread id of the new created thread.
> + * @param name
> + * The name of the control thread (max 16 characters including '\0').
Is there a constant for this limit?
> + * @param thread_attr
> + * Attributes for the new thread.
> + * @param thread_func
> + * Function to be executed by the new thread.
> + * @param arg
> + * Argument passed to start_routine.
> + * @return
> + * On success, returns 0; on error, it returns a negative value
> + * corresponding to the error number.
List the possible error codes.
> + */
> +__rte_experimental
> +int
> +rte_control_thread_create(rte_thread_t *thread, const char *name,
> + const rte_thread_attr_t *thread_attr,
> + rte_thread_func thread_func, void *arg);
> +
> +/**
> * @warning
> * @b EXPERIMENTAL: this API may change without prior notice.
> *
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index 7ad12a7..8f9eeb9 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -440,6 +440,9 @@ EXPERIMENTAL {
> rte_thread_detach;
> rte_thread_equal;
> rte_thread_join;
> +
> + # added in 23.03
> + rte_control_thread_create;
> };
>
> INTERNAL {
next prev parent reply other threads:[~2022-12-07 9:13 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-05 20:24 [PATCH 0/3] eal: rte_ctrl_thread_create API replacement Tyler Retzlaff
2022-12-05 20:24 ` [PATCH 1/3] eal: add rte control thread create API Tyler Retzlaff
2022-12-05 21:11 ` Stephen Hemminger
2022-12-06 0:21 ` Tyler Retzlaff
2022-12-06 17:35 ` Tyler Retzlaff
2022-12-07 0:49 ` Stephen Hemminger
2022-12-05 20:24 ` [PATCH 2/3] test: add rte control thread create API test Tyler Retzlaff
2022-12-05 20:24 ` [PATCH 3/3] eal: deprecate pthread control thread create API Tyler Retzlaff
2022-12-05 21:18 ` Stephen Hemminger
2022-12-06 0:24 ` Tyler Retzlaff
2022-12-05 20:34 ` [PATCH 0/3] eal: rte_ctrl_thread_create API replacement Morten Brørup
2022-12-06 17:28 ` [PATCH v2 " Tyler Retzlaff
2022-12-06 17:28 ` [PATCH v2 1/3] eal: add rte control thread create API Tyler Retzlaff
2022-12-07 9:13 ` Mattias Rönnblom [this message]
2022-12-07 16:38 ` Tyler Retzlaff
2022-12-07 20:37 ` Tyler Retzlaff
2022-12-08 21:59 ` Mattias Rönnblom
2022-12-08 22:15 ` Tyler Retzlaff
2022-12-09 1:09 ` Stephen Hemminger
2022-12-09 19:49 ` Tyler Retzlaff
2022-12-06 17:28 ` [PATCH v2 2/3] test: add rte control thread create API test Tyler Retzlaff
2022-12-06 17:28 ` [PATCH v2 3/3] eal: deprecate pthread control thread create API Tyler Retzlaff
2022-12-07 20:31 ` [PATCH v3 0/3] eal: rte_ctrl_thread_create API replacement Tyler Retzlaff
2022-12-07 20:31 ` [PATCH v3 1/3] eal: add rte control thread create API Tyler Retzlaff
2022-12-07 20:31 ` [PATCH v3 2/3] test: add rte control thread create API test Tyler Retzlaff
2022-12-07 20:31 ` [PATCH v3 3/3] eal: deprecate pthread control thread create API Tyler Retzlaff
2022-12-08 23:19 ` [PATCH v4 0/3] eal: rte_ctrl_thread_create API replacement Tyler Retzlaff
2022-12-08 23:19 ` [PATCH v4 1/3] eal: add rte control thread create API Tyler Retzlaff
2022-12-08 23:19 ` [PATCH v4 2/3] test: add rte control thread create API test Tyler Retzlaff
2022-12-08 23:19 ` [PATCH v4 3/3] eal: deprecate pthread control thread create API Tyler Retzlaff
2022-12-10 14:31 ` [PATCH v4 0/3] eal: rte_ctrl_thread_create API replacement Mattias Rönnblom
2023-01-31 19:46 ` [PATCH v5 0/3] eal: deprecate last use of pthread_t in public API Tyler Retzlaff
2023-01-31 19:46 ` [PATCH v5 1/3] eal: add rte control thread create API Tyler Retzlaff
2023-01-31 19:46 ` [PATCH v5 2/3] doc: add missing index entry for thread Tyler Retzlaff
2023-01-31 19:46 ` [PATCH v5 3/3] doc: announce deprecation of thread ctrl create function Tyler Retzlaff
2023-01-31 20:30 ` [PATCH v5 0/3] eal: deprecate last use of pthread_t in public API Tyler Retzlaff
2023-01-31 20:30 ` [PATCH v5 1/3] eal: add rte control thread create API Tyler Retzlaff
2023-02-07 13:12 ` David Marchand
2023-02-07 21:41 ` Tyler Retzlaff
2023-01-31 20:30 ` [PATCH v5 2/3] doc: add missing index entry for thread Tyler Retzlaff
2023-02-07 15:39 ` David Marchand
2023-02-07 21:39 ` Tyler Retzlaff
2023-01-31 20:30 ` [PATCH v5 3/3] doc: announce deprecation of thread ctrl create function Tyler Retzlaff
2023-02-07 15:40 ` David Marchand
2023-01-31 20:33 ` [PATCH v5 0/3] eal: deprecate last use of pthread_t in public API Tyler Retzlaff
2023-02-08 21:26 ` [PATCH v6 " Tyler Retzlaff
2023-02-08 21:26 ` [PATCH v6 1/3] eal: add rte thread create control API Tyler Retzlaff
2023-02-24 5:52 ` Li, WeiyuanX
2023-02-24 8:13 ` David Marchand
2023-02-24 19:04 ` Tyler Retzlaff
2023-02-24 20:52 ` Tyler Retzlaff
2023-02-08 21:26 ` [PATCH v6 2/3] doc: add missing index entry for thread Tyler Retzlaff
2023-02-08 21:26 ` [PATCH v6 3/3] doc: announce deprecation of thread ctrl create function Tyler Retzlaff
2023-02-09 8:59 ` Bruce Richardson
2023-02-09 8:51 ` [PATCH v6 0/3] eal: deprecate last use of pthread_t in public API David Marchand
2023-02-09 11:55 ` David Marchand
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=5b080d72-f7a8-232d-86a1-9164a005fcfb@lysator.liu.se \
--to=hofors@lysator.liu.se \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=mb@smartsharesystems.com \
--cc=olivier.matz@6wind.com \
--cc=roretzla@linux.microsoft.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
/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).