diff mbox series

[v3] eal: update lcore-worker name due to high number of cores

Message ID 20220923070613.697210-1-omer.yamac@ceng.metu.edu.tr (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers show
Series [v3] eal: update lcore-worker name due to high number of cores | expand

Checks

Context Check Description
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/checkpatch success coding style OK

Commit Message

Abdullah Ömer Yamaç Sept. 23, 2022, 7:06 a.m. UTC
In this patch we suggest a new name for lcore-worker.
In case of more than 99 logical cores, name is truncated
(length is restricted to 16 characters, including the
terminating null byte ('\0')) and it makes hard to follow threads.

Signed-off-by: Abdullah Ömer Yamaç <omer.yamac@ceng.metu.edu.tr>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

---
Cc: David Marchand <david.marchand@redhat.com>
---
 lib/eal/freebsd/eal.c | 2 +-
 lib/eal/linux/eal.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

David Marchand Sept. 26, 2022, 9:39 a.m. UTC | #1
Hello,

On Fri, Sep 23, 2022 at 9:06 AM Abdullah Ömer Yamaç
<omer.yamac@ceng.metu.edu.tr> wrote:
>
> In this patch we suggest a new name for lcore-worker.
> In case of more than 99 logical cores, name is truncated
> (length is restricted to 16 characters, including the
> terminating null byte ('\0')) and it makes hard to follow threads.
>
> Signed-off-by: Abdullah Ömer Yamaç <omer.yamac@ceng.metu.edu.tr>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>

- Thanks, good catch.

Some comments though.
What matters is the lcore index, not the core/lcore count.
The commitlog could probably describe this better.

For example, this issue can be triggered with --lcores=0,10@1,100@2
and here we only have three lcores.
$ cat /proc/$(pidof dpdk-testpmd)/task/*/comm
dpdk-testpmd
eal-intr-thread
rte_mp_handle
lcore-worker-10
lcore-worker-10
telemetry-v2
telemetry-v1


And what do you think of following title:
"eal: fix thread names for high order lcores"

Can you update the release notes?


- Question to stable maintainers:
Do you think we should backport this fix?
As you can see below, this change affects EAL thread names, maybe some
users are monitoring them based on the names...


>
> ---
> Cc: David Marchand <david.marchand@redhat.com>
> ---
>  lib/eal/freebsd/eal.c | 2 +-
>  lib/eal/linux/eal.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index ee5c929da8..a1bb5363b1 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -817,7 +817,7 @@ rte_eal_init(int argc, char **argv)
>
>                 /* Set thread_name for aid in debugging. */
>                 snprintf(thread_name, sizeof(thread_name),
> -                               "lcore-worker-%d", i);
> +                               "rte-worker-%d", i);
>                 rte_thread_setname(lcore_config[i].thread_id, thread_name);
>
>                 ret = pthread_setaffinity_np(lcore_config[i].thread_id,
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 46bf52cef0..9a168b7773 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -1248,7 +1248,7 @@ rte_eal_init(int argc, char **argv)
>
>                 /* Set thread_name for aid in debugging. */
>                 snprintf(thread_name, sizeof(thread_name),
> -                       "lcore-worker-%d", i);
> +                       "rte-worker-%d", i);
>                 ret = rte_thread_setname(lcore_config[i].thread_id,
>                                                 thread_name);
>                 if (ret != 0)
> --
> 2.27.0
>

Merci !
Kevin Traynor Sept. 26, 2022, 10:44 a.m. UTC | #2
On 26/09/2022 10:39, David Marchand wrote:
> Hello,
> 
> On Fri, Sep 23, 2022 at 9:06 AM Abdullah Ömer Yamaç
> <omer.yamac@ceng.metu.edu.tr> wrote:
>>
>> In this patch we suggest a new name for lcore-worker.
>> In case of more than 99 logical cores, name is truncated
>> (length is restricted to 16 characters, including the
>> terminating null byte ('\0')) and it makes hard to follow threads.
>>
>> Signed-off-by: Abdullah Ömer Yamaç <omer.yamac@ceng.metu.edu.tr>
>> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> - Thanks, good catch.
> 
> Some comments though.
> What matters is the lcore index, not the core/lcore count.
> The commitlog could probably describe this better.
> 
> For example, this issue can be triggered with --lcores=0,10@1,100@2
> and here we only have three lcores.
> $ cat /proc/$(pidof dpdk-testpmd)/task/*/comm
> dpdk-testpmd
> eal-intr-thread
> rte_mp_handle
> lcore-worker-10
> lcore-worker-10
> telemetry-v2
> telemetry-v1
> 
> 
> And what do you think of following title:
> "eal: fix thread names for high order lcores"
> 
> Can you update the release notes?
> 
> 
> - Question to stable maintainers:
> Do you think we should backport this fix?
> As you can see below, this change affects EAL thread names, maybe some
> users are monitoring them based on the names...
> 

No strong opinion, but backporting will change the name for any user who 
looks at this and only fix an issue for users using lcore names 100+. So 
would be inclined not to backport unless someone complains it is causing 
a real issue for their use case.

> 
>>
>> ---
>> Cc: David Marchand <david.marchand@redhat.com>
>> ---
>>   lib/eal/freebsd/eal.c | 2 +-
>>   lib/eal/linux/eal.c   | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
>> index ee5c929da8..a1bb5363b1 100644
>> --- a/lib/eal/freebsd/eal.c
>> +++ b/lib/eal/freebsd/eal.c
>> @@ -817,7 +817,7 @@ rte_eal_init(int argc, char **argv)
>>
>>                  /* Set thread_name for aid in debugging. */
>>                  snprintf(thread_name, sizeof(thread_name),
>> -                               "lcore-worker-%d", i);
>> +                               "rte-worker-%d", i);
>>                  rte_thread_setname(lcore_config[i].thread_id, thread_name);
>>
>>                  ret = pthread_setaffinity_np(lcore_config[i].thread_id,
>> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
>> index 46bf52cef0..9a168b7773 100644
>> --- a/lib/eal/linux/eal.c
>> +++ b/lib/eal/linux/eal.c
>> @@ -1248,7 +1248,7 @@ rte_eal_init(int argc, char **argv)
>>
>>                  /* Set thread_name for aid in debugging. */
>>                  snprintf(thread_name, sizeof(thread_name),
>> -                       "lcore-worker-%d", i);
>> +                       "rte-worker-%d", i);
>>                  ret = rte_thread_setname(lcore_config[i].thread_id,
>>                                                  thread_name);
>>                  if (ret != 0)
>> --
>> 2.27.0
>>
> 
> Merci !
>
diff mbox series

Patch

diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index ee5c929da8..a1bb5363b1 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -817,7 +817,7 @@  rte_eal_init(int argc, char **argv)
 
 		/* Set thread_name for aid in debugging. */
 		snprintf(thread_name, sizeof(thread_name),
-				"lcore-worker-%d", i);
+				"rte-worker-%d", i);
 		rte_thread_setname(lcore_config[i].thread_id, thread_name);
 
 		ret = pthread_setaffinity_np(lcore_config[i].thread_id,
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 46bf52cef0..9a168b7773 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1248,7 +1248,7 @@  rte_eal_init(int argc, char **argv)
 
 		/* Set thread_name for aid in debugging. */
 		snprintf(thread_name, sizeof(thread_name),
-			"lcore-worker-%d", i);
+			"rte-worker-%d", i);
 		ret = rte_thread_setname(lcore_config[i].thread_id,
 						thread_name);
 		if (ret != 0)