From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BC82A42534; Thu, 7 Sep 2023 10:28:46 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B3997402AF; Thu, 7 Sep 2023 10:28:46 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 19DF64026C for ; Thu, 7 Sep 2023 10:28:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694075324; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7KpbPyoNw8MJjvpYuRwnVPGYRZrC6Lf2tAeXnuIabw8=; b=KHzwTPP9VftP4C3qFN4jF9izXQOSbpzMPnGnRrEPjqz69L0ZHUYBJECHTyFKMGRKJEyNH3 tdr8pTEGCJ6w/mkcxE7OC243XafEMgEaeJ1FxkJ9gkR7/+5BdkpfFxxWDeSbVUb+BEmfuJ IldzIaQnXxwMpYNod1sN5WVnhYJCL3U= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-122-x3pA6xLrO6WSFjiP-f3Kfw-1; Thu, 07 Sep 2023 04:28:43 -0400 X-MC-Unique: x3pA6xLrO6WSFjiP-f3Kfw-1 Received: by mail-lj1-f197.google.com with SMTP id 38308e7fff4ca-2bd132e84b2so7874471fa.0 for ; Thu, 07 Sep 2023 01:28:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1694075322; x=1694680122; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7KpbPyoNw8MJjvpYuRwnVPGYRZrC6Lf2tAeXnuIabw8=; b=VaSwFgd6hEArubDoBnsiZAapVwv4oHHuUiomX2AmKvPpSgrFzKgdyGTNuqZ1Tg93XF rM+MLaKNNmxFJLTyKJf5pJR0HV8ducfHqyvEJjBI/OkD9HRiafeSW5eXOSuYE2ZkwWcA bSRmB78HnZbl4tEugRRZRiu/WctO1bTyq8X7VvhsPl9otXlwLaUguL0dN7/ZqGALM7h5 SQZ/XqEiOhX3NBADu/vFN/sx/wPrDbtcfLkkQYV4yNbfEIAAMPLngFMlXDg9N8Q5S41O 4ESpaeOKR+wLN2UCgtm3mJAzxJ6Aew/Zu2SkuVF2RsJw+cIAVD4yqYj0/QvssE6zpBYC RyOw== X-Gm-Message-State: AOJu0YyuPiwWZ90nd+TxyN5Xk9OCIbC7p7QJ+c96HyamhUPDyJ8ev8iG DtMNxStys/AUPd8He0KymUiXi5bLBIk8H7rAOLsEDMO9HhCRmLKCDnVatTNm9lzlNqVFbH5CvZP +n0qEubDW8kVhfLE3z28= X-Received: by 2002:a05:6512:3e0f:b0:500:7a23:720b with SMTP id i15-20020a0565123e0f00b005007a23720bmr5393296lfv.55.1694075321712; Thu, 07 Sep 2023 01:28:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG/tvMb1jOU5epKAlNtb6NMcf5LTbWavePKjQIW2tTL1fzCP/EY/ILF4Y6f3Zh354XyFP1ko8PNjIr5BB3SOXs= X-Received: by 2002:a05:6512:3e0f:b0:500:7a23:720b with SMTP id i15-20020a0565123e0f00b005007a23720bmr5393284lfv.55.1694075321309; Thu, 07 Sep 2023 01:28:41 -0700 (PDT) MIME-Version: 1.0 References: <20230906162226.1618088-1-thomas@monjalon.net> <20230906162226.1618088-6-thomas@monjalon.net> In-Reply-To: <20230906162226.1618088-6-thomas@monjalon.net> From: David Marchand Date: Thu, 7 Sep 2023 10:28:29 +0200 Message-ID: Subject: Re: [PATCH 05/11] eal: force prefix for internal threads To: Thomas Monjalon Cc: dev@dpdk.org, Tyler Retzlaff , Ferruh Yigit X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Sep 6, 2023 at 6:23=E2=80=AFPM Thomas Monjalon wrote: > > In order to make sure all threads created in DPDK drivers and libraries > have the same prefix in their name, some wrapper functions are added > for internal use when creating a control thread or setting a thread name: > - rte_thread_create_internal_control > - rte_thread_set_prefixed_name > > The equivalent public functions are then forbidden for internal use: > - rte_thread_create_control > - rte_thread_set_name > > Note: the libraries and drivers conversion is done in next patches, > while doing other thread-related changes. > > Signed-off-by: Thomas Monjalon > --- > devtools/checkpatches.sh | 8 +++++ > lib/eal/common/eal_common_thread.c | 30 ++++++++++++++++ > lib/eal/include/rte_thread.h | 57 +++++++++++++++++++++++++++++- > lib/eal/version.map | 2 ++ > 4 files changed, 96 insertions(+), 1 deletion(-) > > diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh > index 131ffbcebe..18ad6fbb7f 100755 > --- a/devtools/checkpatches.sh > +++ b/devtools/checkpatches.sh > @@ -159,6 +159,14 @@ check_forbidden_additions() { # > -f $(dirname $(readlink -f $0))/check-forbidden-tokens.aw= k \ > "$1" || res=3D1 > > + # forbid non-internal thread in drivers and libs > + awk -v FOLDERS=3D'lib drivers' \ > + -v EXPRESSIONS=3D"rte_thread_(set_name|create_control)\\\= (" \ > + -v RET_ON_FAIL=3D1 \ > + -v MESSAGE=3D'Prefer rte_thread_(set_prefixed_name|create= _internal_control) in lib & drivers' \ > + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.aw= k \ > + "$1" || res=3D1 > + > # forbid inclusion of driver specific headers in apps and example= s > awk -v FOLDERS=3D'app examples' \ > -v EXPRESSIONS=3D'include.*_driver\\.h include.*_pmd\\.h'= \ > diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_comm= on_thread.c > index 07ac721da1..31c37e3102 100644 > --- a/lib/eal/common/eal_common_thread.c > +++ b/lib/eal/common/eal_common_thread.c > @@ -392,6 +392,36 @@ rte_thread_create_control(rte_thread_t *thread, cons= t char *name, > return ret; > } > > +static void > +add_internal_prefix(char *prefixed_name, const char *name, size_t size) > +{ > + const char *prefix =3D "dpdk-"; Can you add some BUILD_BUG_ON to check RTE_THREAD_INTERNAL_NAME_SIZE is still relevant to the prefix? > + size_t prefixlen; > + > + prefixlen =3D strlen(prefix); > + strlcpy(prefixed_name, prefix, size); > + strlcpy(prefixed_name + prefixlen, name, size - prefixlen); > +} > + > +int > +rte_thread_create_internal_control(rte_thread_t *id, const char *name, > + rte_thread_func func, void *arg) > +{ > + char prefixed_name[RTE_THREAD_NAME_SIZE]; > + > + add_internal_prefix(prefixed_name, name, sizeof(prefixed_name)); > + return rte_thread_create_control(id, prefixed_name, func, arg); > +} > + > +void > +rte_thread_set_prefixed_name(rte_thread_t id, const char *name) > +{ > + char prefixed_name[RTE_THREAD_NAME_SIZE]; > + > + add_internal_prefix(prefixed_name, name, sizeof(prefixed_name)); > + rte_thread_set_name(id, prefixed_name); > +} > + > int > rte_thread_register(void) > { > diff --git a/lib/eal/include/rte_thread.h b/lib/eal/include/rte_thread.h > index dd1f62523f..4b1135df4f 100644 > --- a/lib/eal/include/rte_thread.h > +++ b/lib/eal/include/rte_thread.h > @@ -28,6 +28,9 @@ extern "C" { > /* Old definition, aliased for compatibility. */ > #define RTE_MAX_THREAD_NAME_LEN RTE_THREAD_NAME_SIZE > > +/** Maximum internal thread name length (including '\0'). */ > +#define RTE_THREAD_INTERNAL_NAME_SIZE 11 > + > /** > * Thread id descriptor. > */ > @@ -112,7 +115,7 @@ int rte_thread_create(rte_thread_t *thread_id, > * @param thread_func > * Function to be executed by the new thread. > * @param arg > - * Argument passed to start_routine. > + * Argument passed to thread_func. > * @return > * On success, returns 0; on error, it returns a negative value > * corresponding to the error number. > @@ -121,6 +124,36 @@ int > rte_thread_create_control(rte_thread_t *thread, const char *name, > rte_thread_func thread_func, void *arg); > > +/** > + * Create an internal control thread. > + * > + * Creates a control thread with the given name prefixed with "dpdk-". I don't like having a hardcoded comment here. Plus try to grep dpdk- and you will see it is likely we will miss some if we change the prefix. > + * If setting the name of the thread fails, the error is ignored and log= ged. > + * > + * The affinity of the new thread is based on the CPU affinity retrieved > + * at the time rte_eal_init() was called, the EAL threads are then exclu= ded. > + * > + * @param id > + * Filled with the thread ID of the new created thread. > + * @param name > + * The name of the control thread > + * (maximum 10 characters excluding terminating '\0'). This 10 value in the comment is easy to miss if some change with the prefix is done. Mentionning RTE_THREAD_INTERNAL_NAME_SIZE is enough. > + * See RTE_THREAD_INTERNAL_NAME_SIZE. > + * The name of the driver or library should be first, > + * then followed by a hyphen and more details. > + * It will be prefixed with "dpdk-" by this function. > + * @param func > + * Function to be executed by the new thread. > + * @param arg > + * Argument passed to func. > + * @return > + * On success, returns 0; a negative value otherwise. > + */ > +__rte_internal > +int > +rte_thread_create_internal_control(rte_thread_t *id, const char *name, > + rte_thread_func func, void *arg); > + > /** > * Waits for the thread identified by 'thread_id' to terminate > * > @@ -159,6 +192,7 @@ rte_thread_t rte_thread_self(void); > > /** > * Set the name of the thread. > + * > * This API is a noop if the underlying platform does not > * support setting the thread name or the platform-specific > * API used to set the thread name fails. > @@ -173,6 +207,27 @@ rte_thread_t rte_thread_self(void); > void > rte_thread_set_name(rte_thread_t thread_id, const char *thread_name); > > +/** > + * Set the name of an internal thread with adding a "dpdk-" prefix. > + * > + * This API is a noop if the underlying platform does not support > + * setting the thread name, or if it fails. > + * > + * @param id > + * The ID of the thread to set name. > + * > + * @param name > + * The name to set after being prefixed > + * (maximum 10 characters excluding terminating '\0'). > + * See RTE_THREAD_INTERNAL_NAME_SIZE. > + * The name of the driver or library should be first, > + * then followed by a hyphen and more details. > + * It will be prefixed with "dpdk-" by this function. > + */ > +__rte_internal > +void > +rte_thread_set_prefixed_name(rte_thread_t id, const char *name); > + > /** > * Check if 2 thread ids are equal. > * > diff --git a/lib/eal/version.map b/lib/eal/version.map > index 33b853d7be..6d32c19286 100644 > --- a/lib/eal/version.map > +++ b/lib/eal/version.map > @@ -462,4 +462,6 @@ INTERNAL { > rte_mem_map; > rte_mem_page_size; > rte_mem_unmap; > + rte_thread_create_internal_control; > + rte_thread_set_prefixed_name; > }; > -- > 2.42.0 > --=20 David Marchand