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

[Liang, Cunming]

Hi,

> -----Original Message-----
> From: Neil Horman [mailto:nhorman at tuxdriver.com]
> Sent: Sunday, February 15, 2015 10:09 PM
> To: Liang, Cunming
> Cc: dev at 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 at tuxdriver.com]
> > > Sent: Saturday, February 14, 2015 1:57 AM
> > > To: Liang, Cunming
> > > Cc: dev at 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 at 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
> > > >
> > > >
> >