[dpdk-dev,v4,12/17] eal: set _lcore_id and _socket_id to (-1) by default

Message ID 1422842559-13617-13-git-send-email-cunming.liang@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Cunming Liang Feb. 2, 2015, 2:02 a.m. UTC
  For those none EAL thread, *_lcore_id* shall always be LCORE_ID_ANY.
The libraries using *_lcore_id* as index need to take care.
*_socket_id* always be SOCKET_ID_ANY unitl the thread changes the affinity
by rte_thread_set_affinity()

Signed-off-by: Cunming Liang <cunming.liang@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal_thread.c   | 4 ++--
 lib/librte_eal/linuxapp/eal/eal_thread.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
  

Comments

Olivier Matz Feb. 8, 2015, 8:01 p.m. UTC | #1
Hi,

On 02/02/2015 03:02 AM, Cunming Liang wrote:
> For those none EAL thread, *_lcore_id* shall always be LCORE_ID_ANY.
> The libraries using *_lcore_id* as index need to take care.
> *_socket_id* always be SOCKET_ID_ANY unitl the thread changes the affinity

unitl -> until

> by rte_thread_set_affinity()
> 
> Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal_thread.c   | 4 ++--
>  lib/librte_eal/linuxapp/eal/eal_thread.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
> index 5b16302..2b3c9a8 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_thread.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
> @@ -56,8 +56,8 @@
>  #include "eal_private.h"
>  #include "eal_thread.h"
>  
> -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
>  
>  /*
> diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
> index 6eb1525..ab94e20 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
> @@ -57,8 +57,8 @@
>  #include "eal_private.h"
>  #include "eal_thread.h"
>  
> -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);

As far as I understand, now a rte_lcore_id() can return LCORE_ID_ANY.
This should be modified in the rte_lcore_id() API comments.

Same for rte_socket_id().

I also wonder if the API of these functions should be modified to
return an int instead of an unsigned as LCORE_ID_ANY is -1.

Regards,
Olivier
  
Cunming Liang Feb. 9, 2015, 2:24 p.m. UTC | #2
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Monday, February 09, 2015 4:01 AM
> To: Liang, Cunming; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 12/17] eal: set _lcore_id and _socket_id to (-1)
> by default
> 
> Hi,
> 
> On 02/02/2015 03:02 AM, Cunming Liang wrote:
> > For those none EAL thread, *_lcore_id* shall always be LCORE_ID_ANY.
> > The libraries using *_lcore_id* as index need to take care.
> > *_socket_id* always be SOCKET_ID_ANY unitl the thread changes the affinity
> 
> unitl -> until
[LCM] accept.
> 
> > by rte_thread_set_affinity()
> >
> > Signed-off-by: Cunming Liang <cunming.liang@intel.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal_thread.c   | 4 ++--
> >  lib/librte_eal/linuxapp/eal/eal_thread.c | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c
> b/lib/librte_eal/bsdapp/eal/eal_thread.c
> > index 5b16302..2b3c9a8 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal_thread.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
> > @@ -56,8 +56,8 @@
> >  #include "eal_private.h"
> >  #include "eal_thread.h"
> >
> > -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> > -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> > +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> > +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
> >  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
> >
> >  /*
> > diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c
> b/lib/librte_eal/linuxapp/eal/eal_thread.c
> > index 6eb1525..ab94e20 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
> > @@ -57,8 +57,8 @@
> >  #include "eal_private.h"
> >  #include "eal_thread.h"
> >
> > -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> > -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> > +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> > +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
> >  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
> 
> As far as I understand, now a rte_lcore_id() can return LCORE_ID_ANY.
> This should be modified in the rte_lcore_id() API comments.
> 
> Same for rte_socket_id().
[LCM] accept.
> 
> I also wonder if the API of these functions should be modified to
> return an int instead of an unsigned as LCORE_ID_ANY is -1.
[LCM] I prefer not change the API definition. (unsigned)LCORE_ID_ANY already used before.
> 
> Regards,
> Olivier
  
Olivier Matz Feb. 9, 2015, 5:49 p.m. UTC | #3
Hi,

On 02/09/2015 03:24 PM, Liang, Cunming wrote:
>>> --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
>>> @@ -57,8 +57,8 @@
>>>  #include "eal_private.h"
>>>  #include "eal_thread.h"
>>>
>>> -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
>>> -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
>>> +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
>>> +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
>>>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
>>
>> As far as I understand, now a rte_lcore_id() can return LCORE_ID_ANY.
>> This should be modified in the rte_lcore_id() API comments.
>>
>> Same for rte_socket_id().
> [LCM] accept.
>>
>> I also wonder if the API of these functions should be modified to
>> return an int instead of an unsigned as LCORE_ID_ANY is -1.
> [LCM] I prefer not change the API definition. (unsigned)LCORE_ID_ANY already used before.

OK

And what about directly defining the following?

#define LCORE_ID_ANY ((unsigned)-1)


It would avoid the casts.
  
Cunming Liang Feb. 10, 2015, 2:53 a.m. UTC | #4
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Tuesday, February 10, 2015 1:49 AM
> To: Liang, Cunming; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 12/17] eal: set _lcore_id and _socket_id to (-1)
> by default
> 
> Hi,
> 
> On 02/09/2015 03:24 PM, Liang, Cunming wrote:
> >>> --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
> >>> +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
> >>> @@ -57,8 +57,8 @@
> >>>  #include "eal_private.h"
> >>>  #include "eal_thread.h"
> >>>
> >>> -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> >>> -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> >>> +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> >>> +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) =
> (unsigned)SOCKET_ID_ANY;
> >>>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
> >>
> >> As far as I understand, now a rte_lcore_id() can return LCORE_ID_ANY.
> >> This should be modified in the rte_lcore_id() API comments.
> >>
> >> Same for rte_socket_id().
> > [LCM] accept.
> >>
> >> I also wonder if the API of these functions should be modified to
> >> return an int instead of an unsigned as LCORE_ID_ANY is -1.
> > [LCM] I prefer not change the API definition. (unsigned)LCORE_ID_ANY already
> used before.
> 
> OK
> 
> And what about directly defining the following?
> 
> #define LCORE_ID_ANY ((unsigned)-1)
> 
> 
> It would avoid the casts.
[LCM] Good point, will update it.
  
Ananyev, Konstantin Feb. 10, 2015, 11:15 a.m. UTC | #5
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Liang, Cunming
> Sent: Tuesday, February 10, 2015 2:54 AM
> To: Olivier MATZ; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v4 12/17] eal: set _lcore_id and _socket_id to (-1) by default
> 
> 
> 
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Tuesday, February 10, 2015 1:49 AM
> > To: Liang, Cunming; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v4 12/17] eal: set _lcore_id and _socket_id to (-1)
> > by default
> >
> > Hi,
> >
> > On 02/09/2015 03:24 PM, Liang, Cunming wrote:
> > >>> --- a/lib/librte_eal/linuxapp/eal/eal_thread.c
> > >>> +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
> > >>> @@ -57,8 +57,8 @@
> > >>>  #include "eal_private.h"
> > >>>  #include "eal_thread.h"
> > >>>
> > >>> -RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
> > >>> -RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
> > >>> +RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
> > >>> +RTE_DEFINE_PER_LCORE(unsigned, _socket_id) =
> > (unsigned)SOCKET_ID_ANY;
> > >>>  RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
> > >>
> > >> As far as I understand, now a rte_lcore_id() can return LCORE_ID_ANY.
> > >> This should be modified in the rte_lcore_id() API comments.
> > >>
> > >> Same for rte_socket_id().
> > > [LCM] accept.
> > >>
> > >> I also wonder if the API of these functions should be modified to
> > >> return an int instead of an unsigned as LCORE_ID_ANY is -1.
> > > [LCM] I prefer not change the API definition. (unsigned)LCORE_ID_ANY already
> > used before.
> >
> > OK
> >
> > And what about directly defining the following?
> >
> > #define LCORE_ID_ANY ((unsigned)-1)
> >
> >
> > It would avoid the casts.
> [LCM] Good point, will update it.

UINT32_MAX ?
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c
index 5b16302..2b3c9a8 100644
--- a/lib/librte_eal/bsdapp/eal/eal_thread.c
+++ b/lib/librte_eal/bsdapp/eal/eal_thread.c
@@ -56,8 +56,8 @@ 
 #include "eal_private.h"
 #include "eal_thread.h"
 
-RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
-RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
+RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
+RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
 RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
 /*
diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c
index 6eb1525..ab94e20 100644
--- a/lib/librte_eal/linuxapp/eal/eal_thread.c
+++ b/lib/librte_eal/linuxapp/eal/eal_thread.c
@@ -57,8 +57,8 @@ 
 #include "eal_private.h"
 #include "eal_thread.h"
 
-RTE_DEFINE_PER_LCORE(unsigned, _lcore_id);
-RTE_DEFINE_PER_LCORE(unsigned, _socket_id);
+RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = (unsigned)LCORE_ID_ANY;
+RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY;
 RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset);
 
 /*