> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz at 6wind.com]
> Sent: Monday, February 09, 2015 4:00 AM
> To: Liang, Cunming; dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 04/17] eal: add support parsing socket_id
> from cpuset
>
> Hi,
>
> On 02/02/2015 03:02 AM, Cunming Liang wrote:
> > It returns the socket_id if all cpus in the cpuset belongs
> > to the same NUMA node, otherwise it will return SOCKET_ID_ANY.
> >
> > Signed-off-by: Cunming Liang <cunming.liang at intel.com>
> > ---
> > lib/librte_eal/bsdapp/eal/eal_lcore.c | 7 +++++
> > lib/librte_eal/common/eal_thread.h | 52
> +++++++++++++++++++++++++++++++++
> > lib/librte_eal/linuxapp/eal/eal_lcore.c | 7 +++++
> > 3 files changed, 66 insertions(+)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal_lcore.c
> b/lib/librte_eal/bsdapp/eal/eal_lcore.c
> > index 72f8ac2..162fb4f 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal_lcore.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal_lcore.c
> > @@ -41,6 +41,7 @@
> > #include <rte_debug.h>
> >
> > #include "eal_private.h"
> > +#include "eal_thread.h"
> >
> > /* No topology information available on FreeBSD including NUMA info */
> > #define cpu_core_id(X) 0
> > @@ -112,3 +113,9 @@ rte_eal_cpu_init(void)
> >
> > return 0;
> > }
> > +
> > +unsigned
> > +eal_cpu_socket_id(__rte_unused unsigned cpu_id)
> > +{
> > + return cpu_socket_id(cpu_id);
> > +}
> > diff --git a/lib/librte_eal/common/eal_thread.h
> b/lib/librte_eal/common/eal_thread.h
> > index b53b84d..a25ee86 100644
> > --- a/lib/librte_eal/common/eal_thread.h
> > +++ b/lib/librte_eal/common/eal_thread.h
> > @@ -34,6 +34,10 @@
> > #ifndef EAL_THREAD_H
> > #define EAL_THREAD_H
> >
> > +#include <sched.h>
> > +
> > +#include <rte_debug.h>
> > +
> > /**
> > * basic loop of thread, called for each thread by eal_init().
> > *
> > @@ -50,4 +54,52 @@ __attribute__((noreturn)) void *eal_thread_loop(void
> *arg);
> > */
> > void eal_thread_init_master(unsigned lcore_id);
> >
> > +/**
> > + * Get the NUMA socket id from cpu id.
> > + * This function is private to EAL.
> > + *
> > + * @param cpu_id
> > + * The logical process id.
> > + * @return
> > + * socket_id or SOCKET_ID_ANY
> > + */
> > +unsigned eal_cpu_socket_id(unsigned cpu_id);
>
> Wouldn't it be better to rename the existing function cpu_socket_id()
> in eal_cpu_socket_id() and export it in eal_thread.h?
>
> In case of bsd where cpu_socket_id() is implemented using a #define,
> a new function should be created returning 0.
[LCM] In eal_lcore.c, the cpu_socket_id()/cpu_core_id() defined as static and
only used in rte_eal_cpu_init().
I suppose the purpose of origin design is to make the sysfs parsing only
visible in the file.
No matter remove the 'static' prefix of cpu_core_id() or add a new wrap
eal_cpu_socket_id(), it results in a new extern EAL API.
So I prefer not change the visibility of the origin static function but have
one as extern interface.
>
>
> > +
> > +/**
> > + * Get the NUMA socket id from cpuset.
> > + * This function is private to EAL.
> > + *
> > + * @param cpusetp
> > + * The point to a valid cpu set.
> > + * @return
> > + * socket_id or SOCKET_ID_ANY
> > + */
> > +static inline int
> > +eal_cpuset_socket_id(rte_cpuset_t *cpusetp)
> > +{
> > + unsigned cpu = 0;
> > + int socket_id = SOCKET_ID_ANY;
> > + int sid;
> > +
> > + if (cpusetp == NULL)
> > + return SOCKET_ID_ANY;
>
> SOCKET_ID_ANY is not defined, maybe <rte_lcore.h> should be included
> somewhere.
[LCM] Agree with you, eal_cpuset_socket_id() can move into eal_common_thread.c.
And add rte_memory.h for SOCKET_ID_ANY reference.
>
> > +
> > + do {
> > + if (!CPU_ISSET(cpu, cpusetp))
> > + continue;
> > +
> > + if (socket_id == SOCKET_ID_ANY)
> > + socket_id = eal_cpu_socket_id(cpu);
> > +
> > + sid = eal_cpu_socket_id(cpu);
> > + if (socket_id != sid) {
> > + socket_id = SOCKET_ID_ANY;
> > + break;
> > + }
> > +
> > + } while (++cpu < RTE_MAX_LCORE);
> > +
> > + return socket_id;
> > +}
>
>
> I don't think this function should be inlined.
>
> As this function is not used, it could be interesting for reviewers
> to understand when
[LCM] It's used in eal_thread_set_affinity() of eal_thread.c.
>
> > +
> > #endif /* EAL_THREAD_H */
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_lcore.c
> b/lib/librte_eal/linuxapp/eal/eal_lcore.c
> > index 29615f8..922af6d 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_lcore.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_lcore.c
> > @@ -45,6 +45,7 @@
> >
> > #include "eal_private.h"
> > #include "eal_filesystem.h"
> > +#include "eal_thread.h"
> >
> > #define SYS_CPU_DIR "/sys/devices/system/cpu/cpu%u"
> > #define CORE_ID_FILE "topology/core_id"
> > @@ -197,3 +198,9 @@ rte_eal_cpu_init(void)
> >
> > return 0;
> > }
> > +
> > +unsigned
> > +eal_cpu_socket_id(unsigned cpu_id)
> > +{
> > + return cpu_socket_id(cpu_id);
> > +}
> >
