From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 4026C58E8 for ; Mon, 9 Feb 2015 13:26:59 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga103.fm.intel.com with ESMTP; 09 Feb 2015 04:19:59 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,543,1418112000"; d="scan'208";a="675081886" Received: from pgsmsx102.gar.corp.intel.com ([10.221.44.80]) by fmsmga002.fm.intel.com with ESMTP; 09 Feb 2015 04:26:55 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by PGSMSX102.gar.corp.intel.com (10.221.44.80) with Microsoft SMTP Server (TLS) id 14.3.195.1; Mon, 9 Feb 2015 20:26:54 +0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.62]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.192]) with mapi id 14.03.0195.001; Mon, 9 Feb 2015 20:26:54 +0800 From: "Liang, Cunming" To: Olivier MATZ , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v4 04/17] eal: add support parsing socket_id from cpuset Thread-Index: AQHQPoxjmS7iOyuo10GDNsBUrdnMEJzmsHAAgAGR96A= Date: Mon, 9 Feb 2015 12:26:53 +0000 Message-ID: References: <1422491072-5114-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-1-git-send-email-cunming.liang@intel.com> <1422842559-13617-5-git-send-email-cunming.liang@intel.com> <54D7C042.6060104@6wind.com> In-Reply-To: <54D7C042.6060104@6wind.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v4 04/17] eal: add support parsing socket_id from cpuset X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Feb 2015 12:27:00 -0000 > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Monday, February 09, 2015 4:00 AM > To: Liang, Cunming; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 04/17] eal: add support parsing socket_= id > from cpuset >=20 > Hi, >=20 > 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 > > --- > > 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 > > > > #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 > > + > > +#include > > + > > /** > > * 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); >=20 > 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? >=20 > 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 a= nd only used in rte_eal_cpu_init(). I suppose the purpose of origin design is to make the sysfs parsing only vi= sible 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 hav= e one as extern interface. >=20 >=20 > > + > > +/** > > + * 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 =3D 0; > > + int socket_id =3D SOCKET_ID_ANY; > > + int sid; > > + > > + if (cpusetp =3D=3D NULL) > > + return SOCKET_ID_ANY; >=20 > SOCKET_ID_ANY is not defined, maybe should be included > somewhere. [LCM] Agree with you, eal_cpuset_socket_id() can move into eal_common_threa= d.c. And add rte_memory.h for SOCKET_ID_ANY reference. >=20 > > + > > + do { > > + if (!CPU_ISSET(cpu, cpusetp)) > > + continue; > > + > > + if (socket_id =3D=3D SOCKET_ID_ANY) > > + socket_id =3D eal_cpu_socket_id(cpu); > > + > > + sid =3D eal_cpu_socket_id(cpu); > > + if (socket_id !=3D sid) { > > + socket_id =3D SOCKET_ID_ANY; > > + break; > > + } > > + > > + } while (++cpu < RTE_MAX_LCORE); > > + > > + return socket_id; > > +} >=20 >=20 > I don't think this function should be inlined. >=20 > 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. >=20 > > + > > #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); > > +} > >