[v5] enhance NUMA affinity heuristic

Message ID 20230201122048.1283392-1-kaisenx.you@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v5] enhance NUMA affinity heuristic |

Checks

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

Commit Message

Kaisen You Feb. 1, 2023, 12:20 p.m. UTC
  Trying to allocate memory on the first detected numa node has less
chance to find some memory actually available rather than on the main
lcore numa node (especially when the DPDK application is started only
on one numa node).

Fixes: 705356f0811f ("eal: simplify control thread creation")
Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Kaisen You <kaisenx.you@intel.com>
---
Changes since v4:
- mod the patch title,

Changes since v3:
- add the assignment of socket_id in thread initialization,

Changes since v2:
- add uncommitted local change and fix compilation,

Changes since v1:
- accomodate for configurations with main lcore running on multiples
  physical cores belonging to different numa,
---
 lib/eal/common/eal_common_thread.c | 1 +
 lib/eal/common/malloc_heap.c       | 4 ++++
 2 files changed, 5 insertions(+)
  

Comments

Jiale, SongX Feb. 1, 2023, 10:52 a.m. UTC | #1
> -----Original Message-----
> From: Kaisen You <kaisenx.you@intel.com>
> Sent: Wednesday, February 1, 2023 8:21 PM
> To: dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; thomas@monjalon.net;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; You, KaisenX <kaisenx.you@intel.com>;
> zhoumin@loongson.cn; Burakov, Anatoly <anatoly.burakov@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v5] enhance NUMA affinity heuristic
> 
> Trying to allocate memory on the first detected numa node has less chance
> to find some memory actually available rather than on the main lcore numa
> node (especially when the DPDK application is started only on one numa
> node).
> 
> Fixes: 705356f0811f ("eal: simplify control thread creation")
> Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> ---
Tested-by: Song Jiale <songx.jiale@intel.com>
  
Burakov, Anatoly Feb. 15, 2023, 2:22 p.m. UTC | #2
On 2/1/2023 12:20 PM, Kaisen You wrote:
> Trying to allocate memory on the first detected numa node has less
> chance to find some memory actually available rather than on the main
> lcore numa node (especially when the DPDK application is started only
> on one numa node).
> 
> Fixes: 705356f0811f ("eal: simplify control thread creation")
> Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> ---
> Changes since v4:
> - mod the patch title,
> 
> Changes since v3:
> - add the assignment of socket_id in thread initialization,
> 
> Changes since v2:
> - add uncommitted local change and fix compilation,
> 
> Changes since v1:
> - accomodate for configurations with main lcore running on multiples
>    physical cores belonging to different numa,
> ---
>   lib/eal/common/eal_common_thread.c | 1 +
>   lib/eal/common/malloc_heap.c       | 4 ++++
>   2 files changed, 5 insertions(+)
> 
> diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
> index 38d83a6885..21bff971f8 100644
> --- a/lib/eal/common/eal_common_thread.c
> +++ b/lib/eal/common/eal_common_thread.c
> @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
>   	void *routine_arg = params->arg;
>   
>   	__rte_thread_init(rte_lcore_id(), cpuset);
> +	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
>   	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(), cpuset);
>   	if (params->ret != 0) {
>   		__atomic_store_n(&params->ctrl_thread_status,
> diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
> index d7c410b786..3ee19aee15 100644
> --- a/lib/eal/common/malloc_heap.c
> +++ b/lib/eal/common/malloc_heap.c
> @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
>   			return socket_id;
>   	}
>   
> +	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
> +	if (socket_id != (unsigned int)SOCKET_ID_ANY)
> +		return socket_id;
> +
>   	return rte_socket_id_by_idx(0);
>   }
>   

I may be lacking context, but I don't quite get the suggested change. 
 From what I understand, the original has to do with assigning lcore 
cpusets in such a way that an lcore ends up having two socket ID's 
(because it's been assigned to CPU's on different sockets). Why is this 
allowed in the first place? It seems like a user error to me, as it 
breaks many of the fundamental assumptions DPDK makes.

I'm fine with using main lcore socket for control threads, I just don't 
think the `socket_id != SOCKET_ID_ANY` thing should be checked here, 
because it apparently tries to compensate for a problem with cpuset of 
the main thread, which shouldn't have happened to begin with.
  
Burakov, Anatoly Feb. 15, 2023, 2:47 p.m. UTC | #3
On 2/15/2023 2:22 PM, Burakov, Anatoly wrote:
> On 2/1/2023 12:20 PM, Kaisen You wrote:
>> Trying to allocate memory on the first detected numa node has less
>> chance to find some memory actually available rather than on the main
>> lcore numa node (especially when the DPDK application is started only
>> on one numa node).
>>
>> Fixes: 705356f0811f ("eal: simplify control thread creation")
>> Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Kaisen You <kaisenx.you@intel.com>
>> ---
>> Changes since v4:
>> - mod the patch title,
>>
>> Changes since v3:
>> - add the assignment of socket_id in thread initialization,
>>
>> Changes since v2:
>> - add uncommitted local change and fix compilation,
>>
>> Changes since v1:
>> - accomodate for configurations with main lcore running on multiples
>>    physical cores belonging to different numa,
>> ---
>>   lib/eal/common/eal_common_thread.c | 1 +
>>   lib/eal/common/malloc_heap.c       | 4 ++++
>>   2 files changed, 5 insertions(+)
>>
>> diff --git a/lib/eal/common/eal_common_thread.c 
>> b/lib/eal/common/eal_common_thread.c
>> index 38d83a6885..21bff971f8 100644
>> --- a/lib/eal/common/eal_common_thread.c
>> +++ b/lib/eal/common/eal_common_thread.c
>> @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
>>       void *routine_arg = params->arg;
>>       __rte_thread_init(rte_lcore_id(), cpuset);
>> +    RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
>>       params->ret = rte_thread_set_affinity_by_id(rte_thread_self(), 
>> cpuset);
>>       if (params->ret != 0) {
>>           __atomic_store_n(&params->ctrl_thread_status,
>> diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
>> index d7c410b786..3ee19aee15 100644
>> --- a/lib/eal/common/malloc_heap.c
>> +++ b/lib/eal/common/malloc_heap.c
>> @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
>>               return socket_id;
>>       }
>> +    socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
>> +    if (socket_id != (unsigned int)SOCKET_ID_ANY)
>> +        return socket_id;
>> +
>>       return rte_socket_id_by_idx(0);
>>   }
> 
> I may be lacking context, but I don't quite get the suggested change. 
>  From what I understand, the original has to do with assigning lcore 
> cpusets in such a way that an lcore ends up having two socket ID's 
> (because it's been assigned to CPU's on different sockets). Why is this 
> allowed in the first place? It seems like a user error to me, as it 
> breaks many of the fundamental assumptions DPDK makes.
> 
> I'm fine with using main lcore socket for control threads, I just don't 
> think the `socket_id != SOCKET_ID_ANY` thing should be checked here, 
> because it apparently tries to compensate for a problem with cpuset of 
> the main thread, which shouldn't have happened to begin with.
> 
Just to be clear: I don't have any objections to this patch otherwise.

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
  
Kaisen You Feb. 16, 2023, 2:50 a.m. UTC | #4
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: 2023年2月15日 22:23
> To: You, KaisenX <kaisenx.you@intel.com>; dev@dpdk.org
> Cc: Zhou, YidingX <yidingx.zhou@intel.com>; thomas@monjalon.net;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org
> Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> 
> On 2/1/2023 12:20 PM, Kaisen You wrote:
> > Trying to allocate memory on the first detected numa node has less
> > chance to find some memory actually available rather than on the main
> > lcore numa node (especially when the DPDK application is started only
> > on one numa node).
> >
> > Fixes: 705356f0811f ("eal: simplify control thread creation")
> > Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> > ---
> > Changes since v4:
> > - mod the patch title,
> >
> > Changes since v3:
> > - add the assignment of socket_id in thread initialization,
> >
> > Changes since v2:
> > - add uncommitted local change and fix compilation,
> >
> > Changes since v1:
> > - accomodate for configurations with main lcore running on multiples
> >    physical cores belonging to different numa,
> > ---
> >   lib/eal/common/eal_common_thread.c | 1 +
> >   lib/eal/common/malloc_heap.c       | 4 ++++
> >   2 files changed, 5 insertions(+)
> >
> > diff --git a/lib/eal/common/eal_common_thread.c
> > b/lib/eal/common/eal_common_thread.c
> > index 38d83a6885..21bff971f8 100644
> > --- a/lib/eal/common/eal_common_thread.c
> > +++ b/lib/eal/common/eal_common_thread.c
> > @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
> >   	void *routine_arg = params->arg;
> >
> >   	__rte_thread_init(rte_lcore_id(), cpuset);
> > +	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
> >   	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(),
> cpuset);
> >   	if (params->ret != 0) {
> >   		__atomic_store_n(&params->ctrl_thread_status,
> > diff --git a/lib/eal/common/malloc_heap.c
> > b/lib/eal/common/malloc_heap.c index d7c410b786..3ee19aee15 100644
> > --- a/lib/eal/common/malloc_heap.c
> > +++ b/lib/eal/common/malloc_heap.c
> > @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
> >   			return socket_id;
> >   	}
> >
> > +	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
> > +	if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > +		return socket_id;
> > +
> >   	return rte_socket_id_by_idx(0);
> >   }
> >
> 
> I may be lacking context, but I don't quite get the suggested change.
>  From what I understand, the original has to do with assigning lcore cpusets in
> such a way that an lcore ends up having two socket ID's (because it's been
> assigned to CPU's on different sockets). Why is this allowed in the first place?
> It seems like a user error to me, as it breaks many of the fundamental
> assumptions DPDK makes.
> 
In a dual socket system, if all used cores are in socket 1 and the NIC is in socket 1, 
no memory is allocated for socket 0. This is to optimize memory consumption.

I agree with you. If the startup parameters can ensure that both sockets 
allocate memory, there will be no problem.
However, due to the different CPU topologies of different systems, 
It is difficult for users to ensure that the startup parameter contains two cpu nodes.

> I'm fine with using main lcore socket for control threads, I just don't think the
> `socket_id != SOCKET_ID_ANY` thing should be checked here, because it
> apparently tries to compensate for a problem with cpuset of the main thread,
> which shouldn't have happened to begin with.
> 
This issue has been explained in detail in the discussion of the patch v1 version. 
I will forward the previous email to you. The content of the email will also better 
let you know the purpose of submitting this patch.

> --
> Thanks,
> Anatoly
  
Thomas Monjalon March 3, 2023, 2:07 p.m. UTC | #5
I'm not comfortable with this patch.

First, there is no comment in the code which helps to understand the logic.
Second, I'm afraid changing the value of the per-core variable _socket_id
may have an impact on some applications.


16/02/2023 03:50, You, KaisenX:
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> > On 2/1/2023 12:20 PM, Kaisen You wrote:
> > > Trying to allocate memory on the first detected numa node has less
> > > chance to find some memory actually available rather than on the main
> > > lcore numa node (especially when the DPDK application is started only
> > > on one numa node).
> > >
> > > Fixes: 705356f0811f ("eal: simplify control thread creation")
> > > Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> > > ---
> > > Changes since v4:
> > > - mod the patch title,
> > >
> > > Changes since v3:
> > > - add the assignment of socket_id in thread initialization,
> > >
> > > Changes since v2:
> > > - add uncommitted local change and fix compilation,
> > >
> > > Changes since v1:
> > > - accomodate for configurations with main lcore running on multiples
> > >    physical cores belonging to different numa,
> > > ---
> > >   lib/eal/common/eal_common_thread.c | 1 +
> > >   lib/eal/common/malloc_heap.c       | 4 ++++
> > >   2 files changed, 5 insertions(+)
> > >
> > > diff --git a/lib/eal/common/eal_common_thread.c
> > > b/lib/eal/common/eal_common_thread.c
> > > index 38d83a6885..21bff971f8 100644
> > > --- a/lib/eal/common/eal_common_thread.c
> > > +++ b/lib/eal/common/eal_common_thread.c
> > > @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
> > >   	void *routine_arg = params->arg;
> > >
> > >   	__rte_thread_init(rte_lcore_id(), cpuset);
> > > +	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
> > >   	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(),
> > cpuset);
> > >   	if (params->ret != 0) {
> > >   		__atomic_store_n(&params->ctrl_thread_status,
> > > diff --git a/lib/eal/common/malloc_heap.c
> > > b/lib/eal/common/malloc_heap.c index d7c410b786..3ee19aee15 100644
> > > --- a/lib/eal/common/malloc_heap.c
> > > +++ b/lib/eal/common/malloc_heap.c
> > > @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
> > >   			return socket_id;
> > >   	}
> > >
> > > +	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
> > > +	if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > +		return socket_id;
> > > +
> > >   	return rte_socket_id_by_idx(0);
> > >   }
> > >
> > 
> > I may be lacking context, but I don't quite get the suggested change.
> >  From what I understand, the original has to do with assigning lcore cpusets in
> > such a way that an lcore ends up having two socket ID's (because it's been
> > assigned to CPU's on different sockets). Why is this allowed in the first place?
> > It seems like a user error to me, as it breaks many of the fundamental
> > assumptions DPDK makes.
> > 
> In a dual socket system, if all used cores are in socket 1 and the NIC is in socket 1, 
> no memory is allocated for socket 0. This is to optimize memory consumption.
> 
> I agree with you. If the startup parameters can ensure that both sockets 
> allocate memory, there will be no problem.
> However, due to the different CPU topologies of different systems, 
> It is difficult for users to ensure that the startup parameter contains two cpu nodes.
> 
> > I'm fine with using main lcore socket for control threads, I just don't think the
> > `socket_id != SOCKET_ID_ANY` thing should be checked here, because it
> > apparently tries to compensate for a problem with cpuset of the main thread,
> > which shouldn't have happened to begin with.
> > 
> This issue has been explained in detail in the discussion of the patch v1 version. 
> I will forward the previous email to you. The content of the email will also better 
> let you know the purpose of submitting this patch.
  
Kaisen You March 9, 2023, 1:58 a.m. UTC | #6
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2023年3月3日 22:07
> To: Burakov, Anatoly <anatoly.burakov@intel.com>; You, KaisenX
> <kaisenx.you@intel.com>
> Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com
> Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> 
> I'm not comfortable with this patch.
> 
> First, there is no comment in the code which helps to understand the logic.
> Second, I'm afraid changing the value of the per-core variable _socket_id
> may have an impact on some applications.
> 
Thank you for your reply.
First, about comments, I can submit a new patch to add comments to help understand.
Second, if you do not change the value of the per-core variable_ socket_ id,
/lib/eal/common/malloc_heap.c
malloc_get_numa_socket(void)
{
        const struct internal_config *conf = eal_get_internal_configuration();
        unsigned int socket_id = rte_socket_id();   // The return value of "rte_socket_id()" is 1
        unsigned int idx;

        if (socket_id != (unsigned int)SOCKET_ID_ANY)
                return socket_id;    //so return here 

This will cause return here, This function returns the socket_id of unallocated memory.

If you have a better solution, I can modify it.
> 16/02/2023 03:50, You, KaisenX:
> > From: Burakov, Anatoly <anatoly.burakov@intel.com>
> > > On 2/1/2023 12:20 PM, Kaisen You wrote:
> > > > Trying to allocate memory on the first detected numa node has less
> > > > chance to find some memory actually available rather than on the
> > > > main lcore numa node (especially when the DPDK application is
> > > > started only on one numa node).
> > > >
> > > > Fixes: 705356f0811f ("eal: simplify control thread creation")
> > > > Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> > > > ---
> > > > Changes since v4:
> > > > - mod the patch title,
> > > >
> > > > Changes since v3:
> > > > - add the assignment of socket_id in thread initialization,
> > > >
> > > > Changes since v2:
> > > > - add uncommitted local change and fix compilation,
> > > >
> > > > Changes since v1:
> > > > - accomodate for configurations with main lcore running on multiples
> > > >    physical cores belonging to different numa,
> > > > ---
> > > >   lib/eal/common/eal_common_thread.c | 1 +
> > > >   lib/eal/common/malloc_heap.c       | 4 ++++
> > > >   2 files changed, 5 insertions(+)
> > > >
> > > > diff --git a/lib/eal/common/eal_common_thread.c
> > > > b/lib/eal/common/eal_common_thread.c
> > > > index 38d83a6885..21bff971f8 100644
> > > > --- a/lib/eal/common/eal_common_thread.c
> > > > +++ b/lib/eal/common/eal_common_thread.c
> > > > @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
> > > >   	void *routine_arg = params->arg;
> > > >
> > > >   	__rte_thread_init(rte_lcore_id(), cpuset);
> > > > +	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
> > > >   	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(),
> > > cpuset);
> > > >   	if (params->ret != 0) {
> > > >   		__atomic_store_n(&params->ctrl_thread_status,
> > > > diff --git a/lib/eal/common/malloc_heap.c
> > > > b/lib/eal/common/malloc_heap.c index d7c410b786..3ee19aee15
> 100644
> > > > --- a/lib/eal/common/malloc_heap.c
> > > > +++ b/lib/eal/common/malloc_heap.c
> > > > @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
> > > >   			return socket_id;
> > > >   	}
> > > >
> > > > +	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
> > > > +	if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > > +		return socket_id;
> > > > +
> > > >   	return rte_socket_id_by_idx(0);
> > > >   }
> > > >
> > >
> > > I may be lacking context, but I don't quite get the suggested change.
> > >  From what I understand, the original has to do with assigning lcore
> > > cpusets in such a way that an lcore ends up having two socket ID's
> > > (because it's been assigned to CPU's on different sockets). Why is this
> allowed in the first place?
> > > It seems like a user error to me, as it breaks many of the
> > > fundamental assumptions DPDK makes.
> > >
> > In a dual socket system, if all used cores are in socket 1 and the NIC
> > is in socket 1, no memory is allocated for socket 0. This is to optimize
> memory consumption.
> >
> > I agree with you. If the startup parameters can ensure that both
> > sockets allocate memory, there will be no problem.
> > However, due to the different CPU topologies of different systems, It
> > is difficult for users to ensure that the startup parameter contains two cpu
> nodes.
> >
> > > I'm fine with using main lcore socket for control threads, I just
> > > don't think the `socket_id != SOCKET_ID_ANY` thing should be checked
> > > here, because it apparently tries to compensate for a problem with
> > > cpuset of the main thread, which shouldn't have happened to begin with.
> > >
> > This issue has been explained in detail in the discussion of the patch v1
> version.
> > I will forward the previous email to you. The content of the email
> > will also better let you know the purpose of submitting this patch.
> 
>
  
Kaisen You April 13, 2023, 12:56 a.m. UTC | #7
> -----Original Message-----
> From: You, KaisenX
> Sent: 2023年3月9日 9:58
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [PATCH v5] enhance NUMA affinity heuristic
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: 2023年3月3日 22:07
> > To: Burakov, Anatoly <anatoly.burakov@intel.com>; You, KaisenX
> > <kaisenx.you@intel.com>
> > Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> > david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> > ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com
> > Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> >
> > I'm not comfortable with this patch.
> >
> > First, there is no comment in the code which helps to understand the logic.
> > Second, I'm afraid changing the value of the per-core variable
> > _socket_id may have an impact on some applications.
> >
Hi Thomas, I'm sorry to bother you again, but we can't think of a better solution for now,
would you please give me some suggestion, and then I will modify it accordingly.

> Thank you for your reply.
> First, about comments, I can submit a new patch to add comments to help
> understand.
> Second, if you do not change the value of the per-core variable_ socket_ id,
> /lib/eal/common/malloc_heap.c
> malloc_get_numa_socket(void)
> {
>         const struct internal_config *conf = eal_get_internal_configuration();
>         unsigned int socket_id = rte_socket_id();   // The return value of
> "rte_socket_id()" is 1
>         unsigned int idx;
> 
>         if (socket_id != (unsigned int)SOCKET_ID_ANY)
>                 return socket_id;    //so return here
> 
> This will cause return here, This function returns the socket_id of unallocated
> memory.
> 
> If you have a better solution, I can modify it.
> > 16/02/2023 03:50, You, KaisenX:
> > > From: Burakov, Anatoly <anatoly.burakov@intel.com>
> > > > On 2/1/2023 12:20 PM, Kaisen You wrote:
> > > > > Trying to allocate memory on the first detected numa node has
> > > > > less chance to find some memory actually available rather than
> > > > > on the main lcore numa node (especially when the DPDK
> > > > > application is started only on one numa node).
> > > > >
> > > > > Fixes: 705356f0811f ("eal: simplify control thread creation")
> > > > > Fixes: bb0bd346d5c1 ("eal: suggest using --lcores option")
> > > > > Cc: stable@dpdk.org
> > > > >
> > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > > Signed-off-by: Kaisen You <kaisenx.you@intel.com>
> > > > > ---
> > > > > Changes since v4:
> > > > > - mod the patch title,
> > > > >
> > > > > Changes since v3:
> > > > > - add the assignment of socket_id in thread initialization,
> > > > >
> > > > > Changes since v2:
> > > > > - add uncommitted local change and fix compilation,
> > > > >
> > > > > Changes since v1:
> > > > > - accomodate for configurations with main lcore running on multiples
> > > > >    physical cores belonging to different numa,
> > > > > ---
> > > > >   lib/eal/common/eal_common_thread.c | 1 +
> > > > >   lib/eal/common/malloc_heap.c       | 4 ++++
> > > > >   2 files changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/lib/eal/common/eal_common_thread.c
> > > > > b/lib/eal/common/eal_common_thread.c
> > > > > index 38d83a6885..21bff971f8 100644
> > > > > --- a/lib/eal/common/eal_common_thread.c
> > > > > +++ b/lib/eal/common/eal_common_thread.c
> > > > > @@ -251,6 +251,7 @@ static void *ctrl_thread_init(void *arg)
> > > > >   	void *routine_arg = params->arg;
> > > > >
> > > > >   	__rte_thread_init(rte_lcore_id(), cpuset);
> > > > > +	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
> > > > >   	params->ret =
> > > > > rte_thread_set_affinity_by_id(rte_thread_self(),
> > > > cpuset);
> > > > >   	if (params->ret != 0) {
> > > > >   		__atomic_store_n(&params->ctrl_thread_status,
> > > > > diff --git a/lib/eal/common/malloc_heap.c
> > > > > b/lib/eal/common/malloc_heap.c index d7c410b786..3ee19aee15
> > 100644
> > > > > --- a/lib/eal/common/malloc_heap.c
> > > > > +++ b/lib/eal/common/malloc_heap.c
> > > > > @@ -717,6 +717,10 @@ malloc_get_numa_socket(void)
> > > > >   			return socket_id;
> > > > >   	}
> > > > >
> > > > > +	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
> > > > > +	if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > > > +		return socket_id;
> > > > > +
> > > > >   	return rte_socket_id_by_idx(0);
> > > > >   }
> > > > >
> > > >
> > > > I may be lacking context, but I don't quite get the suggested change.
> > > >  From what I understand, the original has to do with assigning
> > > > lcore cpusets in such a way that an lcore ends up having two
> > > > socket ID's (because it's been assigned to CPU's on different
> > > > sockets). Why is this
> > allowed in the first place?
> > > > It seems like a user error to me, as it breaks many of the
> > > > fundamental assumptions DPDK makes.
> > > >
> > > In a dual socket system, if all used cores are in socket 1 and the
> > > NIC is in socket 1, no memory is allocated for socket 0. This is to
> > > optimize
> > memory consumption.
> > >
> > > I agree with you. If the startup parameters can ensure that both
> > > sockets allocate memory, there will be no problem.
> > > However, due to the different CPU topologies of different systems,
> > > It is difficult for users to ensure that the startup parameter
> > > contains two cpu
> > nodes.
> > >
> > > > I'm fine with using main lcore socket for control threads, I just
> > > > don't think the `socket_id != SOCKET_ID_ANY` thing should be
> > > > checked here, because it apparently tries to compensate for a
> > > > problem with cpuset of the main thread, which shouldn't have
> happened to begin with.
> > > >
> > > This issue has been explained in detail in the discussion of the
> > > patch v1
> > version.
> > > I will forward the previous email to you. The content of the email
> > > will also better let you know the purpose of submitting this patch.
> >
> >
  
Thomas Monjalon April 19, 2023, 12:16 p.m. UTC | #8
13/04/2023 02:56, You, KaisenX:
> From: You, KaisenX
> > From: Thomas Monjalon <thomas@monjalon.net>
> > >
> > > I'm not comfortable with this patch.
> > >
> > > First, there is no comment in the code which helps to understand the logic.
> > > Second, I'm afraid changing the value of the per-core variable
> > > _socket_id may have an impact on some applications.
> > >
> Hi Thomas, I'm sorry to bother you again, but we can't think of a better solution for now,
> would you please give me some suggestion, and then I will modify it accordingly.

You need to better explain the logic
both in the commit message and in code comments.
When it will be done, it will be easier to have a discussion
with other maintainers and community experts.
Thank you

> > Thank you for your reply.
> > First, about comments, I can submit a new patch to add comments to help
> > understand.
> > Second, if you do not change the value of the per-core variable_ socket_ id,
> > /lib/eal/common/malloc_heap.c
> > malloc_get_numa_socket(void)
> > {
> >         const struct internal_config *conf = eal_get_internal_configuration();
> >         unsigned int socket_id = rte_socket_id();   // The return value of
> > "rte_socket_id()" is 1
> >         unsigned int idx;
> > 
> >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> >                 return socket_id;    //so return here
> > 
> > This will cause return here, This function returns the socket_id of unallocated
> > memory.
> > 
> > If you have a better solution, I can modify it.
  
Kaisen You April 21, 2023, 2:34 a.m. UTC | #9
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2023年4月19日 20:17
> To: You, KaisenX <kaisenx.you@intel.com>
> Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> 
> 13/04/2023 02:56, You, KaisenX:
> > From: You, KaisenX
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > > I'm not comfortable with this patch.
> > > >
> > > > First, there is no comment in the code which helps to understand the
> logic.
> > > > Second, I'm afraid changing the value of the per-core variable
> > > > _socket_id may have an impact on some applications.
> > > >
> > Hi Thomas, I'm sorry to bother you again, but we can't think of a
> > better solution for now, would you please give me some suggestion, and
> then I will modify it accordingly.
> 
> You need to better explain the logic
> both in the commit message and in code comments.
> When it will be done, it will be easier to have a discussion with other
> maintainers and community experts.
> Thank you
> 
Thank you for your reply, I'll explain my patch in more detail next.

When a DPDK application is started on only one numa node, memory is allocated 
for only one socket.When interrupt threads use memory, memory may not be found 
on the socket where the interrupt thread is currently located, and memory has to be 
reallocated on the hugepage, this operation can lead to performance degradation.

So my modification is in the function malloc_get_numa_socket to make sure 
that the first socket with memory can be returned.

If you can accept my explanation and modification, I will send the V6 
version to improve the commit message and code comments.

> > > Thank you for your reply.
> > > First, about comments, I can submit a new patch to add comments to
> > > help understand.
> > > Second, if you do not change the value of the per-core variable_
> > > socket_ id, /lib/eal/common/malloc_heap.c
> > > malloc_get_numa_socket(void)
> > > {
> > >         const struct internal_config *conf = eal_get_internal_configuration();
> > >         unsigned int socket_id = rte_socket_id();   // The return value of
> > > "rte_socket_id()" is 1
> > >         unsigned int idx;
> > >
> > >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > >                 return socket_id;    //so return here
> > >
> > > This will cause return here, This function returns the socket_id of
> > > unallocated memory.
> > >
> > > If you have a better solution, I can modify it.
> 
> 
>
  
Thomas Monjalon April 21, 2023, 8:12 a.m. UTC | #10
21/04/2023 04:34, You, KaisenX:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 13/04/2023 02:56, You, KaisenX:
> > > From: You, KaisenX
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > >
> > > > > I'm not comfortable with this patch.
> > > > >
> > > > > First, there is no comment in the code which helps to understand the
> > logic.
> > > > > Second, I'm afraid changing the value of the per-core variable
> > > > > _socket_id may have an impact on some applications.
> > > > >
> > > Hi Thomas, I'm sorry to bother you again, but we can't think of a
> > > better solution for now, would you please give me some suggestion, and
> > then I will modify it accordingly.
> > 
> > You need to better explain the logic
> > both in the commit message and in code comments.
> > When it will be done, it will be easier to have a discussion with other
> > maintainers and community experts.
> > Thank you
> > 
> Thank you for your reply, I'll explain my patch in more detail next.
> 
> When a DPDK application is started on only one numa node,

What do you mean by started on only one node?

> memory is allocated for only one socket.
> When interrupt threads use memory, memory may not be found 
> on the socket where the interrupt thread is currently located,

Why interrupt thread is on a different socket?

> and memory has to be reallocated on the hugepage,
> this operation can lead to performance degradation.
> 
> So my modification is in the function malloc_get_numa_socket to make sure 
> that the first socket with memory can be returned.
> 
> If you can accept my explanation and modification, I will send the V6 
> version to improve the commit message and code comments.
> 
> > > > Thank you for your reply.
> > > > First, about comments, I can submit a new patch to add comments to
> > > > help understand.
> > > > Second, if you do not change the value of the per-core variable_
> > > > socket_ id, /lib/eal/common/malloc_heap.c
> > > > malloc_get_numa_socket(void)
> > > > {
> > > >         const struct internal_config *conf = eal_get_internal_configuration();
> > > >         unsigned int socket_id = rte_socket_id();   // The return value of
> > > > "rte_socket_id()" is 1
> > > >         unsigned int idx;
> > > >
> > > >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > >                 return socket_id;    //so return here
> > > >
> > > > This will cause return here, This function returns the socket_id of
> > > > unallocated memory.
> > > >
> > > > If you have a better solution, I can modify it.
  
Kaisen You April 23, 2023, 6:52 a.m. UTC | #11
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: 2023年4月21日 16:13
> To: You, KaisenX <kaisenx.you@intel.com>
> Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> 
> 21/04/2023 04:34, You, KaisenX:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/04/2023 02:56, You, KaisenX:
> > > > From: You, KaisenX
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > >
> > > > > > I'm not comfortable with this patch.
> > > > > >
> > > > > > First, there is no comment in the code which helps to
> > > > > > understand the
> > > logic.
> > > > > > Second, I'm afraid changing the value of the per-core variable
> > > > > > _socket_id may have an impact on some applications.
> > > > > >
> > > > Hi Thomas, I'm sorry to bother you again, but we can't think of a
> > > > better solution for now, would you please give me some suggestion,
> > > > and
> > > then I will modify it accordingly.
> > >
> > > You need to better explain the logic both in the commit message and
> > > in code comments.
> > > When it will be done, it will be easier to have a discussion with
> > > other maintainers and community experts.
> > > Thank you
> > >
> > Thank you for your reply, I'll explain my patch in more detail next.
> >
> > When a DPDK application is started on only one numa node,
> 
> What do you mean by started on only one node?
When the dpdk application is started with the startup parameter "-l 40-59" 
(this range is on the same node as the system cpu processor).Only memory is 
allocated for this node when the process is initialized.
> 
> > memory is allocated for only one socket.
> > When interrupt threads use memory, memory may not be found on the
> > socket where the interrupt thread is currently located,
> 
> Why interrupt thread is on a different socket?
The above only allocates memory on node1, but the interrupt thread is created on node0.
Interrupt threads are created by rte_ctrl_thread_create() ,rte_ctrl_thread_create()' 
does NOT run on main lcore, it can run on any core except data plane cores. 
So interrupt thread can run on any core.
> > and memory has to be reallocated on the hugepage, this operation can
> > lead to performance degradation.
> >
> > So my modification is in the function malloc_get_numa_socket to make
> > sure that the first socket with memory can be returned.
> >
> > If you can accept my explanation and modification, I will send the V6
> > version to improve the commit message and code comments.
> >
> > > > > Thank you for your reply.
> > > > > First, about comments, I can submit a new patch to add comments
> > > > > to help understand.
> > > > > Second, if you do not change the value of the per-core variable_
> > > > > socket_ id, /lib/eal/common/malloc_heap.c
> > > > > malloc_get_numa_socket(void)
> > > > > {
> > > > >         const struct internal_config *conf =
> eal_get_internal_configuration();
> > > > >         unsigned int socket_id = rte_socket_id();   // The return value of
> > > > > "rte_socket_id()" is 1
> > > > >         unsigned int idx;
> > > > >
> > > > >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > > >                 return socket_id;    //so return here
> > > > >
> > > > > This will cause return here, This function returns the socket_id
> > > > > of unallocated memory.
> > > > >
> > > > > If you have a better solution, I can modify it.
> 
> 
>
  
Kaisen You April 23, 2023, 8:57 a.m. UTC | #12
> -----Original Message-----
> From: You, KaisenX <kaisenx.you@intel.com>
> Sent: 2023年4月23日 14:52
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> Burakov, Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [PATCH v5] enhance NUMA affinity heuristic
> 
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: 2023年4月21日 16:13
> > To: You, KaisenX <kaisenx.you@intel.com>
> > Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> > david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> > ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> > Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> >
> > 21/04/2023 04:34, You, KaisenX:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/04/2023 02:56, You, KaisenX:
> > > > > From: You, KaisenX
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > >
> > > > > > > I'm not comfortable with this patch.
> > > > > > >
> > > > > > > First, there is no comment in the code which helps to
> > > > > > > understand the
> > > > logic.
> > > > > > > Second, I'm afraid changing the value of the per-core
> > > > > > > variable _socket_id may have an impact on some applications.
> > > > > > >
> > > > > Hi Thomas, I'm sorry to bother you again, but we can't think of
> > > > > a better solution for now, would you please give me some
> > > > > suggestion, and
> > > > then I will modify it accordingly.
> > > >
> > > > You need to better explain the logic both in the commit message
> > > > and in code comments.
> > > > When it will be done, it will be easier to have a discussion with
> > > > other maintainers and community experts.
> > > > Thank you
> > > >
> > > Thank you for your reply, I'll explain my patch in more detail next.
> > >
> > > When a DPDK application is started on only one numa node,
> >
> > What do you mean by started on only one node?
> When the dpdk application is started with the startup parameter "-l 40-59"
> (this range is on the same node as the system cpu processor).Only memory is
> allocated for this node when the process is initialized.
> >
> > > memory is allocated for only one socket.
> > > When interrupt threads use memory, memory may not be found on the
> > > socket where the interrupt thread is currently located,
> >
> > Why interrupt thread is on a different socket?
> The above only allocates memory on node1, but the interrupt thread is
> created on node0.
> Interrupt threads are created by
> rte_ctrl_thread_create() ,rte_ctrl_thread_create()'
> does NOT run on main lcore, it can run on any core except data plane cores.
> So interrupt thread can run on any core.
To avoid ambiguity, I'll explain my commet again. Interrupt threads are created by
rte_ctrl_thread_create() , rte_ctrl_thread_create() doesn't control which node the 
thread it creates runs on, interrupt threads can run on any core except data plane 
cores. So interrupt thread can run on any core.
> > > and memory has to be reallocated on the hugepage, this operation can
> > > lead to performance degradation.
> > >
> > > So my modification is in the function malloc_get_numa_socket to make
> > > sure that the first socket with memory can be returned.
> > >
> > > If you can accept my explanation and modification, I will send the
> > > V6 version to improve the commit message and code comments.
> > >
> > > > > > Thank you for your reply.
> > > > > > First, about comments, I can submit a new patch to add
> > > > > > comments to help understand.
> > > > > > Second, if you do not change the value of the per-core
> > > > > > variable_ socket_ id, /lib/eal/common/malloc_heap.c
> > > > > > malloc_get_numa_socket(void)
> > > > > > {
> > > > > >         const struct internal_config *conf =
> > eal_get_internal_configuration();
> > > > > >         unsigned int socket_id = rte_socket_id();   // The return value of
> > > > > > "rte_socket_id()" is 1
> > > > > >         unsigned int idx;
> > > > > >
> > > > > >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > > > >                 return socket_id;    //so return here
> > > > > >
> > > > > > This will cause return here, This function returns the
> > > > > > socket_id of unallocated memory.
> > > > > >
> > > > > > If you have a better solution, I can modify it.
> >
> >
> >
  
Thomas Monjalon April 23, 2023, 1:19 p.m. UTC | #13
OK please send v6 with comments in the code where appropriate.
We'll continue the discussion in v6.
Thanks


23/04/2023 10:57, You, KaisenX:
> 
> > -----Original Message-----
> > From: You, KaisenX <kaisenx.you@intel.com>
> > Sent: 2023年4月23日 14:52
> > To: Thomas Monjalon <thomas@monjalon.net>
> > Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> > david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> > ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> > Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> > Burakov, Anatoly <anatoly.burakov@intel.com>
> > Subject: RE: [PATCH v5] enhance NUMA affinity heuristic
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: 2023年4月21日 16:13
> > > To: You, KaisenX <kaisenx.you@intel.com>
> > > Cc: dev@dpdk.org; Zhou, YidingX <yidingx.zhou@intel.com>;
> > > david.marchand@redhat.com; Matz, Olivier <olivier.matz@6wind.com>;
> > > ferruh.yigit@amd.com; zhoumin@loongson.cn; stable@dpdk.org;
> > > Richardson, Bruce <bruce.richardson@intel.com>; jerinj@marvell.com;
> > > Burakov, Anatoly <anatoly.burakov@intel.com>
> > > Subject: Re: [PATCH v5] enhance NUMA affinity heuristic
> > >
> > > 21/04/2023 04:34, You, KaisenX:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 13/04/2023 02:56, You, KaisenX:
> > > > > > From: You, KaisenX
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > >
> > > > > > > > I'm not comfortable with this patch.
> > > > > > > >
> > > > > > > > First, there is no comment in the code which helps to
> > > > > > > > understand the
> > > > > logic.
> > > > > > > > Second, I'm afraid changing the value of the per-core
> > > > > > > > variable _socket_id may have an impact on some applications.
> > > > > > > >
> > > > > > Hi Thomas, I'm sorry to bother you again, but we can't think of
> > > > > > a better solution for now, would you please give me some
> > > > > > suggestion, and
> > > > > then I will modify it accordingly.
> > > > >
> > > > > You need to better explain the logic both in the commit message
> > > > > and in code comments.
> > > > > When it will be done, it will be easier to have a discussion with
> > > > > other maintainers and community experts.
> > > > > Thank you
> > > > >
> > > > Thank you for your reply, I'll explain my patch in more detail next.
> > > >
> > > > When a DPDK application is started on only one numa node,
> > >
> > > What do you mean by started on only one node?
> > When the dpdk application is started with the startup parameter "-l 40-59"
> > (this range is on the same node as the system cpu processor).Only memory is
> > allocated for this node when the process is initialized.
> > >
> > > > memory is allocated for only one socket.
> > > > When interrupt threads use memory, memory may not be found on the
> > > > socket where the interrupt thread is currently located,
> > >
> > > Why interrupt thread is on a different socket?
> > The above only allocates memory on node1, but the interrupt thread is
> > created on node0.
> > Interrupt threads are created by
> > rte_ctrl_thread_create() ,rte_ctrl_thread_create()'
> > does NOT run on main lcore, it can run on any core except data plane cores.
> > So interrupt thread can run on any core.
> To avoid ambiguity, I'll explain my commet again. Interrupt threads are created by
> rte_ctrl_thread_create() , rte_ctrl_thread_create() doesn't control which node the 
> thread it creates runs on, interrupt threads can run on any core except data plane 
> cores. So interrupt thread can run on any core.
> > > > and memory has to be reallocated on the hugepage, this operation can
> > > > lead to performance degradation.
> > > >
> > > > So my modification is in the function malloc_get_numa_socket to make
> > > > sure that the first socket with memory can be returned.
> > > >
> > > > If you can accept my explanation and modification, I will send the
> > > > V6 version to improve the commit message and code comments.
> > > >
> > > > > > > Thank you for your reply.
> > > > > > > First, about comments, I can submit a new patch to add
> > > > > > > comments to help understand.
> > > > > > > Second, if you do not change the value of the per-core
> > > > > > > variable_ socket_ id, /lib/eal/common/malloc_heap.c
> > > > > > > malloc_get_numa_socket(void)
> > > > > > > {
> > > > > > >         const struct internal_config *conf =
> > > eal_get_internal_configuration();
> > > > > > >         unsigned int socket_id = rte_socket_id();   // The return value of
> > > > > > > "rte_socket_id()" is 1
> > > > > > >         unsigned int idx;
> > > > > > >
> > > > > > >         if (socket_id != (unsigned int)SOCKET_ID_ANY)
> > > > > > >                 return socket_id;    //so return here
> > > > > > >
> > > > > > > This will cause return here, This function returns the
> > > > > > > socket_id of unallocated memory.
> > > > > > >
> > > > > > > If you have a better solution, I can modify it.
> > >
> > >
> > >
> 
>
  

Patch

diff --git a/lib/eal/common/eal_common_thread.c b/lib/eal/common/eal_common_thread.c
index 38d83a6885..21bff971f8 100644
--- a/lib/eal/common/eal_common_thread.c
+++ b/lib/eal/common/eal_common_thread.c
@@ -251,6 +251,7 @@  static void *ctrl_thread_init(void *arg)
 	void *routine_arg = params->arg;
 
 	__rte_thread_init(rte_lcore_id(), cpuset);
+	RTE_PER_LCORE(_socket_id) = SOCKET_ID_ANY;
 	params->ret = rte_thread_set_affinity_by_id(rte_thread_self(), cpuset);
 	if (params->ret != 0) {
 		__atomic_store_n(&params->ctrl_thread_status,
diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
index d7c410b786..3ee19aee15 100644
--- a/lib/eal/common/malloc_heap.c
+++ b/lib/eal/common/malloc_heap.c
@@ -717,6 +717,10 @@  malloc_get_numa_socket(void)
 			return socket_id;
 	}
 
+	socket_id = rte_lcore_to_socket_id(rte_get_main_lcore());
+	if (socket_id != (unsigned int)SOCKET_ID_ANY)
+		return socket_id;
+
 	return rte_socket_id_by_idx(0);
 }