[dpdk-dev] [PATCH v4 01/17] eal: add cpuset into per EAL thread lcore_config
> -Original Message- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Tuesday, February 10, 2015 1:07 AM > 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. [LCM] Yes, I missed this one. I agree to move to a separate one and remove undef but rename the PAGE_SIZE to EAL_PAGE_SIZE. > > > >>> 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 > >>> + typedef cpuset_t rte_cpuset_t; > >>> +#endif > >>> + > >> > >> Should we also define RTE_CPU_SETSIZE? > >> For linux, should 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 :) > > > >> 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.
[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 >>> + typedef cpuset_t rte_cpuset_t; >>> +#endif >>> + >> >> Should we also define RTE_CPU_SETSIZE? >> For linux, should 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 :) >> 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.
[dpdk-dev] [PATCH v4 01/17] eal: add cpuset into per EAL thread lcore_config
> -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 > >>> + typedef cpuset_t rte_cpuset_t; > >>> +#endif > >>> + > >> > >> Should we also define RTE_CPU_SETSIZE? > >> For linux, should 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.
[dpdk-dev] [PATCH v4 01/17] eal: add cpuset into per EAL thread lcore_config
> -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 01/17] eal: add cpuset into per EAL thread > lcore_config > > Hi, > > 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? > > > > > > /* > > 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 > > + typedef cpuset_t rte_cpuset_t; > > +#endif > > + > > Should we also define RTE_CPU_SETSIZE? > For linux, should 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). > > 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. > > > Regards, > Olivier
[dpdk-dev] [PATCH v4 01/17] eal: add cpuset into per EAL thread lcore_config
Hi, 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? > > /* > 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 > + typedef cpuset_t rte_cpuset_t; > +#endif > + Should we also define RTE_CPU_SETSIZE? For linux, should be included? 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? Regards, Olivier