> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Olivier MATZ > Sent: Monday, February 09, 2015 5:07 PM > To: Liang, Cunming; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v4 01/17] eal: add cpuset into per EAL thread > lcore_config > > Hi, > > On 02/09/2015 12:33 PM, Liang, Cunming wrote: > >> On 02/02/2015 03:02 AM, Cunming Liang wrote: > >>> The patch adds 'cpuset' into per-lcore configure 'lcore_config[]', > >>> as the lcore no longer always 1:1 pinning with physical cpu. > >>> The lcore now stands for a EAL thread rather than a logical cpu. > >>> > >>> It doesn't change the default behavior of 1:1 mapping, but allows to > >>> affinity the EAL thread to multiple cpus. > >>> > >>> [...] > >>> diff --git a/lib/librte_eal/bsdapp/eal/eal_memory.c > >> b/lib/librte_eal/bsdapp/eal/eal_memory.c > >>> index 65ee87d..a34d500 100644 > >>> --- a/lib/librte_eal/bsdapp/eal/eal_memory.c > >>> +++ b/lib/librte_eal/bsdapp/eal/eal_memory.c > >>> @@ -45,6 +45,8 @@ > >>> #include "eal_internal_cfg.h" > >>> #include "eal_filesystem.h" > >>> > >>> +/* avoid re-defined against with freebsd header */ > >>> +#undef PAGE_SIZE > >>> #define PAGE_SIZE (sysconf(_SC_PAGESIZE)) > >> > >> I don't see the link with the patch. Should this go somewhere else? > > Maybe you missed this one. > > > >>> diff --git a/lib/librte_eal/common/include/rte_lcore.h > >> b/lib/librte_eal/common/include/rte_lcore.h > >>> index 49b2c03..4c7d6bb 100644 > >>> --- a/lib/librte_eal/common/include/rte_lcore.h > >>> +++ b/lib/librte_eal/common/include/rte_lcore.h > >>> @@ -50,6 +50,13 @@ extern "C" { > >>> > >>> #define LCORE_ID_ANY -1 /**< Any lcore. */ > >>> > >>> +#if defined(__linux__) > >>> + typedef cpu_set_t rte_cpuset_t; > >>> +#elif defined(__FreeBSD__) > >>> +#include <pthread_np.h> > >>> + typedef cpuset_t rte_cpuset_t; > >>> +#endif > >>> + > >> > >> Should we also define RTE_CPU_SETSIZE? > >> For linux, should <sched.h> be included? > > [LCM] It uses the fix size cpuset, won't use CPU_ALLOC() to get the pointer > > of cpuset. > > The RTE_CPU_SETSIZE always equal to sizeof(rte_cpuset_t). > > The advantage of using CPU_ALLOC() is to avoid issues when the number > of core will be higher than 1024. I agree it's probably a bit early > to think about this, but it could happen soon :)
I personally don't think, we'll hit 1K cpu limit anytime soon... >From other side - fixed size cpuset allows to cleanup and simplify code quite >a bit. So, I'd suggest to stick with fixed size for now. Konstantin > > > >> If I understand well, after the patch series, the user of > >> rte_thread_set_affinity() and rte_thread_get_affinity() are > >> supposed to use the macros from sched.h to access to this > >> cpuset parameter. So I'm wondering if it's not better to > >> use cpu_set_t from libc instead of redefining rte_cpuset_t. > >> > >> To reword my question: what is the purpose of redefining > >> cpu_set_t in rte_cpuset_t if we still need to use all the > >> libc API to access to it? > > [LCM] In linux the type is *cpu_set_t*, but in freebsd it's *cpuset_t*. > > The purpose of *rte_cpuset_t* is to make the consistent type definition in > > EAL, and to avoid lots of #ifdef for this diff. > > In either linux or freebsd, it still can use the MACRO in libc to set the > > rte_cpuset_t. > > OK, it makes sense then. I did not notice the difference between linux > and bsd.
