[dpdk-dev,v6,12/19] malloc: fix the issue of SOCKET_ID_ANY

  Add check for rte_socket_id(), avoid get unexpected return like (-1).

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_malloc/malloc_heap.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
  

On Fri, Feb 13, 2015 at 09:38:14AM +0800, Cunming Liang wrote:
> Add check for rte_socket_id(), avoid get unexpected return like (-1).
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> ---
>  lib/librte_malloc/malloc_heap.h | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_malloc/malloc_heap.h b/lib/librte_malloc/malloc_heap.h
> index b4aec45..a47136d 100644
> --- a/lib/librte_malloc/malloc_heap.h
> +++ b/lib/librte_malloc/malloc_heap.h
> @@ -44,7 +44,12 @@ extern "C" {
>  static inline unsigned
>  malloc_get_numa_socket(void)
>  {
> -	return rte_socket_id();
> +	unsigned socket_id = rte_socket_id();
> +
> +	if (socket_id == (unsigned)SOCKET_ID_ANY)
> +		return 0;
> +
> +	return socket_id;
Why is -1 unexpected?  Isn't it reasonable to assume that some memory is
equidistant from all cpu numa nodes?

Neil

>  }
>  
>  void *
> -- 
> 1.8.1.4
> 
>
  

Hi,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Saturday, February 14, 2015 1:57 AM
> To: Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 12/19] malloc: fix the issue of SOCKET_ID_ANY
> 
> On Fri, Feb 13, 2015 at 09:38:14AM +0800, Cunming Liang wrote:
> > Add check for rte_socket_id(), avoid get unexpected return like (-1).
> >
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> >  lib/librte_malloc/malloc_heap.h | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_malloc/malloc_heap.h b/lib/librte_malloc/malloc_heap.h
> > index b4aec45..a47136d 100644
> > --- a/lib/librte_malloc/malloc_heap.h
> > +++ b/lib/librte_malloc/malloc_heap.h
> > @@ -44,7 +44,12 @@ extern "C" {
> >  static inline unsigned
> >  malloc_get_numa_socket(void)
> >  {
> > -	return rte_socket_id();
> > +	unsigned socket_id = rte_socket_id();
> > +
> > +	if (socket_id == (unsigned)SOCKET_ID_ANY)
> > +		return 0;
> > +
> > +	return socket_id;
> Why is -1 unexpected?  Isn't it reasonable to assume that some memory is
> equidistant from all cpu numa nodes?
[LCM] One piece of memory will be whole allocated from one specific NUMA node. But won't be like some part from one and the other part from another.
If no specific NUMA node assigned(SOCKET_ID_ANY/-1), it firstly asks for the current NUMA node where current core belongs to.
'malloc_get_numa_socket()' is called on that time. When the time 1:1 thread/core mapping is assumed and the default value is 0, it always will return a none (-1) value.
Now rte_socket_id() may return -1 in the case the pthread runs on multi-cores which are not belongs to one NUMA node, or in the case _socket_id is not yet assigned and the default value is (-1). So if current _socket_id is -1, then just pick up the first node as the candidate. Probably I shall add more comments for this.
> 
> Neil
> 
> >  }
> >
> >  void *
> > --
> > 1.8.1.4
> >
> >
  

On Sun, Feb 15, 2015 at 12:43:03AM +0000, Liang, Cunming wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > Sent: Saturday, February 14, 2015 1:57 AM
> > To: Liang, Cunming
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v6 12/19] malloc: fix the issue of SOCKET_ID_ANY
> > 
> > On Fri, Feb 13, 2015 at 09:38:14AM +0800, Cunming Liang wrote:
> > > Add check for rte_socket_id(), avoid get unexpected return like (-1).
> > >
> > > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > > ---
> > >  lib/librte_malloc/malloc_heap.h | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/librte_malloc/malloc_heap.h b/lib/librte_malloc/malloc_heap.h
> > > index b4aec45..a47136d 100644
> > > --- a/lib/librte_malloc/malloc_heap.h
> > > +++ b/lib/librte_malloc/malloc_heap.h
> > > @@ -44,7 +44,12 @@ extern "C" {
> > >  static inline unsigned
> > >  malloc_get_numa_socket(void)
> > >  {
> > > -	return rte_socket_id();
> > > +	unsigned socket_id = rte_socket_id();
> > > +
> > > +	if (socket_id == (unsigned)SOCKET_ID_ANY)
> > > +		return 0;
> > > +
> > > +	return socket_id;
> > Why is -1 unexpected?  Isn't it reasonable to assume that some memory is
> > equidistant from all cpu numa nodes?
> [LCM] One piece of memory will be whole allocated from one specific NUMA node. But won't be like some part from one and the other part from another.
> If no specific NUMA node assigned(SOCKET_ID_ANY/-1), it firstly asks for the current NUMA node where current core belongs to.
> 'malloc_get_numa_socket()' is called on that time. When the time 1:1 thread/core mapping is assumed and the default value is 0, it always will return a none (-1) value.
> Now rte_socket_id() may return -1 in the case the pthread runs on multi-cores which are not belongs to one NUMA node, or in the case _socket_id is not yet assigned and the default value is (-1). So if current _socket_id is -1, then just pick up the first node as the candidate. Probably I shall add more comments for this.
> > 
Ok, but doesn't that provide an abnormal bias for node 0?  I was thinking it
might be better to be honest with the application so that it can choose a node
according to its own policy.

Neil

> > Neil
> > 
> > >  }
> > >
> > >  void *
> > > --
> > > 1.8.1.4
> > >
> > >
>
  

Hi,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman@tuxdriver.com]
> Sent: Sunday, February 15, 2015 10:09 PM
> To: Liang, Cunming
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 12/19] malloc: fix the issue of SOCKET_ID_ANY
> 
> On Sun, Feb 15, 2015 at 12:43:03AM +0000, Liang, Cunming wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Neil Horman [mailto:nhorman@tuxdriver.com]
> > > Sent: Saturday, February 14, 2015 1:57 AM
> > > To: Liang, Cunming
> > > Cc: dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v6 12/19] malloc: fix the issue of
> SOCKET_ID_ANY
> > >
> > > On Fri, Feb 13, 2015 at 09:38:14AM +0800, Cunming Liang wrote:
> > > > Add check for rte_socket_id(), avoid get unexpected return like (-1).
> > > >
> > > > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > > > ---
> > > >  lib/librte_malloc/malloc_heap.h | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/librte_malloc/malloc_heap.h
> b/lib/librte_malloc/malloc_heap.h
> > > > index b4aec45..a47136d 100644
> > > > --- a/lib/librte_malloc/malloc_heap.h
> > > > +++ b/lib/librte_malloc/malloc_heap.h
> > > > @@ -44,7 +44,12 @@ extern "C" {
> > > >  static inline unsigned
> > > >  malloc_get_numa_socket(void)
> > > >  {
> > > > -	return rte_socket_id();
> > > > +	unsigned socket_id = rte_socket_id();
> > > > +
> > > > +	if (socket_id == (unsigned)SOCKET_ID_ANY)
> > > > +		return 0;
> > > > +
> > > > +	return socket_id;
> > > Why is -1 unexpected?  Isn't it reasonable to assume that some memory is
> > > equidistant from all cpu numa nodes?
> > [LCM] One piece of memory will be whole allocated from one specific NUMA
> node. But won't be like some part from one and the other part from another.
> > If no specific NUMA node assigned(SOCKET_ID_ANY/-1), it firstly asks for the
> current NUMA node where current core belongs to.
> > 'malloc_get_numa_socket()' is called on that time. When the time 1:1
> thread/core mapping is assumed and the default value is 0, it always will return a
> none (-1) value.
> > Now rte_socket_id() may return -1 in the case the pthread runs on multi-cores
> which are not belongs to one NUMA node, or in the case _socket_id is not yet
> assigned and the default value is (-1). So if current _socket_id is -1, then just pick
> up the first node as the candidate. Probably I shall add more comments for this.
> > >
> Ok, but doesn't that provide an abnormal bias for node 0?  I was thinking it
> might be better to be honest with the application so that it can choose a node
> according to its own policy.
[LCM] Personally I like the idea grant application to make the decision.
Either add a simple default configure or defines the more flexible policy of SOCKET_ID_ANY like
1) use the assigned default socket_id; 2) use current socket_id, if fail goto 1);
3) (weight)round robin across the malloc_heaps; 4) use current socket_id, if fail goto 3); and etc.
But on another side, the well-tuned application are usually NUMA friendly. Instead of using SOCKET_ID_ANY, it most often assigned the expected socket_id.
Except getting the real current valid socket_id, The policy won't help on the affinity but mainly helps on the memory utilization.
I guess the worry comes from the case, after lots of memory allocation happens on socket 0, a new memzone_reserve fails when it definitely has to do it on socket 0 as well.
In this case, either changes the default NUMA node or balance the allocation won't solve the problem, but respite it happening.
It's because the explicit assignment allocation (memzone_reserve, malloc with a specified socket_id) may not average balanced.
In reverse, if reserving all necessary memzone first, even malloc fails on default socket, it will try to get allocation from other NUMA node.
I think it's out of the scope of this patch series. On current moment, using the simplest way taking node 0 as default socket_id is not bad.
For more, we can post on separate patch and involved more on the discussion.
Thanks.

> 
> Neil
> 
> > > Neil
> > >
> > > >  }
> > > >
> > > >  void *
> > > > --
> > > > 1.8.1.4
> > > >
> > > >
> >