[ovs-dev] dpif-netdev: Add core id in the PMD thread name.
diff mbox series

Message ID 20190813163757.473-1-i.maximets@samsung.com
State New
Headers show
Series
  • [ovs-dev] dpif-netdev: Add core id in the PMD thread name.
Related show

Commit Message

Ilya Maximets Aug. 13, 2019, 4:37 p.m. UTC
This is highly useful to see on which core PMD is running by
only looking at the thread name. Thread Id still allows to
distinguish different threads running on the same core over the time:

   |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
   |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
   |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>

In gdb, top or any other utility it's useful to quickly catch up
needed thread without parsing logs, memory or matching threads by port
names they're handling.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/dpif-netdev.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Eelco Chaudron Aug. 13, 2019, 4:46 p.m. UTC | #1
On 13 Aug 2019, at 18:37, Ilya Maximets wrote:

> This is highly useful to see on which core PMD is running by
> only looking at the thread name. Thread Id still allows to
> distinguish different threads running on the same core over the time:
>
>    |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
>    |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
>    |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
>
> In gdb, top or any other utility it's useful to quickly catch up
> needed thread without parsing logs, memory or matching threads by port
> names they're handling.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  lib/dpif-netdev.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index d0a1c58ad..34ba03836 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>      FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>          pmd = dp_netdev_get_pmd(dp, core->core_id);
>          if (!pmd) {
> +            struct ds name = DS_EMPTY_INITIALIZER;
> +
>              pmd = xzalloc(sizeof *pmd);
>              dp_netdev_configure_pmd(pmd, dp, core->core_id, 
> core->numa_id);
> -            pmd->thread = ovs_thread_create("pmd", pmd_thread_main, 
> pmd);
> +
> +            ds_put_format(&name, "pmd-c%02d/id:", core->core_id);

This is a really good idea :) One remark should we make it %03d?

> +            pmd->thread = ovs_thread_create(ds_cstr(&name),
> +                                            pmd_thread_main, pmd);
> +            ds_destroy(&name);
> +
>              VLOG_INFO("PMD thread on numa_id: %d, core id: %2d 
> created.",
>                        pmd->numa_id, pmd->core_id);
>              changed = true;
> -- 
> 2.17.1
Ilya Maximets Aug. 14, 2019, 7:45 a.m. UTC | #2
On 13.08.2019 19:46, Eelco Chaudron wrote:
> 
> 
> On 13 Aug 2019, at 18:37, Ilya Maximets wrote:
> 
>> This is highly useful to see on which core PMD is running by
>> only looking at the thread name. Thread Id still allows to
>> distinguish different threads running on the same core over the time:
>>
>>    |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
>>    |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
>>    |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
>>
>> In gdb, top or any other utility it's useful to quickly catch up
>> needed thread without parsing logs, memory or matching threads by port
>> names they're handling.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  lib/dpif-netdev.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index d0a1c58ad..34ba03836 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>      FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>>          pmd = dp_netdev_get_pmd(dp, core->core_id);
>>          if (!pmd) {
>> +            struct ds name = DS_EMPTY_INITIALIZER;
>> +
>>              pmd = xzalloc(sizeof *pmd);
>>              dp_netdev_configure_pmd(pmd, dp, core->core_id, core->numa_id);
>> -            pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
>> +
>> +            ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
> 
> This is a really good idea :) One remark should we make it %03d?

There is a hard limit for the thread name. It's 15 meaningful chars excluding the
terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes for the
thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining for id.
Thread ids could easily become big ( > 1000) for a long running process, that is
why %02d was chosen, to save some space.

BTW, even on a systems with 100+ CPUs I'm usually placing OVS threads on a lower
cores. Anyway, this is only the matter of a bit more visual beauty in the logs.

What do you think?

> 
>> +            pmd->thread = ovs_thread_create(ds_cstr(&name),
>> +                                            pmd_thread_main, pmd);
>> +            ds_destroy(&name);
>> +
>>              VLOG_INFO("PMD thread on numa_id: %d, core id: %2d created.",
>>                        pmd->numa_id, pmd->core_id);
>>              changed = true;
>> -- 
>> 2.17.1
> 
>
Eelco Chaudron Aug. 14, 2019, 7:53 a.m. UTC | #3
On 14 Aug 2019, at 9:45, Ilya Maximets wrote:

> On 13.08.2019 19:46, Eelco Chaudron wrote:
>>
>>
>> On 13 Aug 2019, at 18:37, Ilya Maximets wrote:
>>
>>> This is highly useful to see on which core PMD is running by
>>> only looking at the thread name. Thread Id still allows to
>>> distinguish different threads running on the same core over the 
>>> time:
>>>
>>>    |dpif_netdev(pmd-c10/id:53)|DBG|Creating 2. subtable <...>
>>>    |dpif_netdev(pmd-c10/id:53)|DBG|flow_add: <...>, actions:2
>>>    |dpif_netdev(pmd-c09/id:70)|DBG|Core 9 processing port <..>
>>>
>>> In gdb, top or any other utility it's useful to quickly catch up
>>> needed thread without parsing logs, memory or matching threads by 
>>> port
>>> names they're handling.
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>> ---
>>>  lib/dpif-netdev.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>>> index d0a1c58ad..34ba03836 100644
>>> --- a/lib/dpif-netdev.c
>>> +++ b/lib/dpif-netdev.c
>>> @@ -4735,9 +4735,16 @@ reconfigure_pmd_threads(struct dp_netdev *dp)
>>>      FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
>>>          pmd = dp_netdev_get_pmd(dp, core->core_id);
>>>          if (!pmd) {
>>> +            struct ds name = DS_EMPTY_INITIALIZER;
>>> +
>>>              pmd = xzalloc(sizeof *pmd);
>>>              dp_netdev_configure_pmd(pmd, dp, 
>>> core->core_id, core->numa_id);
>>> -            pmd->thread = ovs_thread_create("pmd", 
>>> pmd_thread_main, pmd);
>>> +
>>> +            ds_put_format(&name, "pmd-c%02d/id:", 
>>> core->core_id);
>>
>> This is a really good idea :) One remark should we make it %03d?
>
> There is a hard limit for the thread name. It's 15 meaningful chars 
> excluding the
> terminating null byte. 'pmd-c02/id:' is 11 bytes wide keeping 4 bytes 
> for the
> thread id. 'pmd-c002/id:' is 12 bytes wide with only 3 bytes remaining 
> for id.
> Thread ids could easily become big ( > 1000) for a long running 
> process, that is
> why %02d was chosen, to save some space.
>
> BTW, even on a systems with 100+ CPUs I'm usually placing OVS threads 
> on a lower
> cores. Anyway, this is only the matter of a bit more visual beauty in 
> the logs.
>
> What do you think?

Makes sense

Acked-by: Eelco Chaudron <echaudro@redhat.com>

>
>>
>>> +            pmd->thread = 
>>> ovs_thread_create(ds_cstr(&name),
>>> +                                            
>>> pmd_thread_main, pmd);
>>> +            ds_destroy(&name);
>>> +
>>>              VLOG_INFO("PMD thread on numa_id: %d, core 
>>> id: %2d created.",
>>>                        pmd->numa_id, 
>>> pmd->core_id);
>>>              changed = true;
>>> -- 
>>> 2.17.1
>>
>>

Patch
diff mbox series

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index d0a1c58ad..34ba03836 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4735,9 +4735,16 @@  reconfigure_pmd_threads(struct dp_netdev *dp)
     FOR_EACH_CORE_ON_DUMP(core, pmd_cores) {
         pmd = dp_netdev_get_pmd(dp, core->core_id);
         if (!pmd) {
+            struct ds name = DS_EMPTY_INITIALIZER;
+
             pmd = xzalloc(sizeof *pmd);
             dp_netdev_configure_pmd(pmd, dp, core->core_id, core->numa_id);
-            pmd->thread = ovs_thread_create("pmd", pmd_thread_main, pmd);
+
+            ds_put_format(&name, "pmd-c%02d/id:", core->core_id);
+            pmd->thread = ovs_thread_create(ds_cstr(&name),
+                                            pmd_thread_main, pmd);
+            ds_destroy(&name);
+
             VLOG_INFO("PMD thread on numa_id: %d, core id: %2d created.",
                       pmd->numa_id, pmd->core_id);
             changed = true;