[dpdk-dev,v2] eal: add function to return number of detected sockets
Checks
Commit Message
During lcore scan, find maximum socket ID and store it.
Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
Notes:
v2:
- checkpatch changes
- check socket before deciding if the core is not to be used
lib/librte_eal/common/eal_common_lcore.c | 37 +++++++++++++++++++++----------
lib/librte_eal/common/include/rte_eal.h | 1 +
lib/librte_eal/common/include/rte_lcore.h | 8 +++++++
lib/librte_eal/rte_eal_version.map | 6 +++++
4 files changed, 40 insertions(+), 12 deletions(-)
Comments
22/12/2017 13:41, Anatoly Burakov:
> During lcore scan, find maximum socket ID and store it.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> struct rte_config {
> uint32_t master_lcore; /**< Id of the master lcore */
> uint32_t lcore_count; /**< Number of available logical cores. */
> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
> uint32_t service_lcore_count;/**< Number of available service cores. */
> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
isn't it breaking the ABI?
On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> 22/12/2017 13:41, Anatoly Burakov:
>> During lcore scan, find maximum socket ID and store it.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> --- a/lib/librte_eal/common/include/rte_eal.h
>> +++ b/lib/librte_eal/common/include/rte_eal.h
>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
>> struct rte_config {
>> uint32_t master_lcore; /**< Id of the master lcore */
>> uint32_t lcore_count; /**< Number of available logical cores. */
>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
>> uint32_t service_lcore_count;/**< Number of available service cores. */
>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>
> isn't it breaking the ABI?
>
>
Yep, you're right, forgot to add that. I didn't expect this to get
merged in 18.02 anyway, so v2 will follow.
12/01/2018 12:44, Burakov, Anatoly:
> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> > 22/12/2017 13:41, Anatoly Burakov:
> >> During lcore scan, find maximum socket ID and store it.
> >>
> >> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >> ---
> >> --- a/lib/librte_eal/common/include/rte_eal.h
> >> +++ b/lib/librte_eal/common/include/rte_eal.h
> >> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> >> struct rte_config {
> >> uint32_t master_lcore; /**< Id of the master lcore */
> >> uint32_t lcore_count; /**< Number of available logical cores. */
> >> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
> >> uint32_t service_lcore_count;/**< Number of available service cores. */
> >> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >
> > isn't it breaking the ABI?
> >
> >
>
> Yep, you're right, forgot to add that. I didn't expect this to get
> merged in 18.02 anyway, so v2 will follow.
Please write 18.05 in the subject to show your expectation.
Thanks
On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> 12/01/2018 12:44, Burakov, Anatoly:
>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>> 22/12/2017 13:41, Anatoly Burakov:
>>>> During lcore scan, find maximum socket ID and store it.
>>>>
>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
>>>> struct rte_config {
>>>> uint32_t master_lcore; /**< Id of the master lcore */
>>>> uint32_t lcore_count; /**< Number of available logical cores. */
>>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
>>>> uint32_t service_lcore_count;/**< Number of available service cores. */
>>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>
>>> isn't it breaking the ABI?
>>>
>>>
>>
>> Yep, you're right, forgot to add that. I didn't expect this to get
>> merged in 18.02 anyway, so v2 will follow.
>
> Please write 18.05 in the subject to show your expectation.
> Thanks
>
Does it have to be an ABI change though? We can put numa_node_count
after pointer to mem_config, in which case it won't be an ABI break.
Would that be better?
16/01/2018 12:56, Burakov, Anatoly:
> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> > 12/01/2018 12:44, Burakov, Anatoly:
> >> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>> 22/12/2017 13:41, Anatoly Burakov:
> >>>> During lcore scan, find maximum socket ID and store it.
> >>>>
> >>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>> ---
> >>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> >>>> struct rte_config {
> >>>> uint32_t master_lcore; /**< Id of the master lcore */
> >>>> uint32_t lcore_count; /**< Number of available logical cores. */
> >>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
> >>>> uint32_t service_lcore_count;/**< Number of available service cores. */
> >>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>
> >>> isn't it breaking the ABI?
> >>>
> >>>
> >>
> >> Yep, you're right, forgot to add that. I didn't expect this to get
> >> merged in 18.02 anyway, so v2 will follow.
> >
> > Please write 18.05 in the subject to show your expectation.
> > Thanks
> >
>
> Does it have to be an ABI change though? We can put numa_node_count
> after pointer to mem_config, in which case it won't be an ABI break.
> Would that be better?
Changing the size of a struct which is allocated by the app,
is an ABI break.
Is your solution changing the size?
On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> 16/01/2018 12:56, Burakov, Anatoly:
>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
>>> 12/01/2018 12:44, Burakov, Anatoly:
>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>>>> 22/12/2017 13:41, Anatoly Burakov:
>>>>>> During lcore scan, find maximum socket ID and store it.
>>>>>>
>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>> ---
>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
>>>>>> struct rte_config {
>>>>>> uint32_t master_lcore; /**< Id of the master lcore */
>>>>>> uint32_t lcore_count; /**< Number of available logical cores. */
>>>>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
>>>>>> uint32_t service_lcore_count;/**< Number of available service cores. */
>>>>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>>>
>>>>> isn't it breaking the ABI?
>>>>>
>>>>>
>>>>
>>>> Yep, you're right, forgot to add that. I didn't expect this to get
>>>> merged in 18.02 anyway, so v2 will follow.
>>>
>>> Please write 18.05 in the subject to show your expectation.
>>> Thanks
>>>
>>
>> Does it have to be an ABI change though? We can put numa_node_count
>> after pointer to mem_config, in which case it won't be an ABI break.
>> Would that be better?
>
> Changing the size of a struct which is allocated by the app,
> is an ABI break.
> Is your solution changing the size?
>
It's not really allocated as such. rte_config is a global static
variable, and we only ever get pointers to it from the user code. If we
add the new value at the end, all of the old data layout would be intact
and work as before, so nothing would change as far as old code is concerned.
However, if that's still considered an ABI break, then OK, break it is.
Some background for why this is needed - for the memory hotplug, we need
to know how many sockets we can allocate memory at, to distinguish
between socket that doesn't exist, and socket that exists but has no
memory allocated on it. I'm OK with trying other approaches (such as
storing numa nodes in a static variable somewhere) if breaking ABI for
this is too much to ask for such a minute change.
16/01/2018 16:05, Burakov, Anatoly:
> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> > 16/01/2018 12:56, Burakov, Anatoly:
> >> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> >>> 12/01/2018 12:44, Burakov, Anatoly:
> >>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>>>> 22/12/2017 13:41, Anatoly Burakov:
> >>>>>> During lcore scan, find maximum socket ID and store it.
> >>>>>>
> >>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>>>> ---
> >>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> >>>>>> struct rte_config {
> >>>>>> uint32_t master_lcore; /**< Id of the master lcore */
> >>>>>> uint32_t lcore_count; /**< Number of available logical cores. */
> >>>>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
> >>>>>> uint32_t service_lcore_count;/**< Number of available service cores. */
> >>>>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>>>
> >>>>> isn't it breaking the ABI?
> >>>>>
> >>>>>
> >>>>
> >>>> Yep, you're right, forgot to add that. I didn't expect this to get
> >>>> merged in 18.02 anyway, so v2 will follow.
> >>>
> >>> Please write 18.05 in the subject to show your expectation.
> >>> Thanks
> >>>
> >>
> >> Does it have to be an ABI change though? We can put numa_node_count
> >> after pointer to mem_config, in which case it won't be an ABI break.
> >> Would that be better?
> >
> > Changing the size of a struct which is allocated by the app,
> > is an ABI break.
> > Is your solution changing the size?
> >
>
> It's not really allocated as such. rte_config is a global static
> variable, and we only ever get pointers to it from the user code. If we
> add the new value at the end, all of the old data layout would be intact
> and work as before, so nothing would change as far as old code is concerned.
>
> However, if that's still considered an ABI break, then OK, break it is.
Maybe that assuming it is never allocated (not copied for instance)
we could consider it is not an ABI break.
> Some background for why this is needed - for the memory hotplug, we need
> to know how many sockets we can allocate memory at, to distinguish
> between socket that doesn't exist, and socket that exists but has no
> memory allocated on it. I'm OK with trying other approaches (such as
> storing numa nodes in a static variable somewhere) if breaking ABI for
> this is too much to ask for such a minute change.
Why is it important for 18.02?
Memory hotplug will be integrated only in 18.05.
I think it is better to just wait (and announce the deprecation).
On 16-Jan-18 5:34 PM, Thomas Monjalon wrote:
> 16/01/2018 16:05, Burakov, Anatoly:
>> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
>>> 16/01/2018 12:56, Burakov, Anatoly:
>>>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
>>>>> 12/01/2018 12:44, Burakov, Anatoly:
>>>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
>>>>>>> 22/12/2017 13:41, Anatoly Burakov:
>>>>>>>> During lcore scan, find maximum socket ID and store it.
>>>>>>>>
>>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>>>>>> ---
>>>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
>>>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
>>>>>>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
>>>>>>>> struct rte_config {
>>>>>>>> uint32_t master_lcore; /**< Id of the master lcore */
>>>>>>>> uint32_t lcore_count; /**< Number of available logical cores. */
>>>>>>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
>>>>>>>> uint32_t service_lcore_count;/**< Number of available service cores. */
>>>>>>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
>>>>>>>
>>>>>>> isn't it breaking the ABI?
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Yep, you're right, forgot to add that. I didn't expect this to get
>>>>>> merged in 18.02 anyway, so v2 will follow.
>>>>>
>>>>> Please write 18.05 in the subject to show your expectation.
>>>>> Thanks
>>>>>
>>>>
>>>> Does it have to be an ABI change though? We can put numa_node_count
>>>> after pointer to mem_config, in which case it won't be an ABI break.
>>>> Would that be better?
>>>
>>> Changing the size of a struct which is allocated by the app,
>>> is an ABI break.
>>> Is your solution changing the size?
>>>
>>
>> It's not really allocated as such. rte_config is a global static
>> variable, and we only ever get pointers to it from the user code. If we
>> add the new value at the end, all of the old data layout would be intact
>> and work as before, so nothing would change as far as old code is concerned.
>>
>> However, if that's still considered an ABI break, then OK, break it is.
>
> Maybe that assuming it is never allocated (not copied for instance)
> we could consider it is not an ABI break.
>
>> Some background for why this is needed - for the memory hotplug, we need
>> to know how many sockets we can allocate memory at, to distinguish
>> between socket that doesn't exist, and socket that exists but has no
>> memory allocated on it. I'm OK with trying other approaches (such as
>> storing numa nodes in a static variable somewhere) if breaking ABI for
>> this is too much to ask for such a minute change.
>
> Why is it important for 18.02?
> Memory hotplug will be integrated only in 18.05.
> I think it is better to just wait (and announce the deprecation).
>
It isn't, i've already marked this patch as deferred. However, we'll
have to have this discussion anyway :)
16/01/2018 18:38, Burakov, Anatoly:
> On 16-Jan-18 5:34 PM, Thomas Monjalon wrote:
> > 16/01/2018 16:05, Burakov, Anatoly:
> >> On 16-Jan-18 12:20 PM, Thomas Monjalon wrote:
> >>> 16/01/2018 12:56, Burakov, Anatoly:
> >>>> On 12-Jan-18 11:50 AM, Thomas Monjalon wrote:
> >>>>> 12/01/2018 12:44, Burakov, Anatoly:
> >>>>>> On 11-Jan-18 10:20 PM, Thomas Monjalon wrote:
> >>>>>>> 22/12/2017 13:41, Anatoly Burakov:
> >>>>>>>> During lcore scan, find maximum socket ID and store it.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >>>>>>>> ---
> >>>>>>>> --- a/lib/librte_eal/common/include/rte_eal.h
> >>>>>>>> +++ b/lib/librte_eal/common/include/rte_eal.h
> >>>>>>>> @@ -83,6 +83,7 @@ enum rte_proc_type_t {
> >>>>>>>> struct rte_config {
> >>>>>>>> uint32_t master_lcore; /**< Id of the master lcore */
> >>>>>>>> uint32_t lcore_count; /**< Number of available logical cores. */
> >>>>>>>> + uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
> >>>>>>>> uint32_t service_lcore_count;/**< Number of available service cores. */
> >>>>>>>> enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
> >>>>>>>
> >>>>>>> isn't it breaking the ABI?
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>> Yep, you're right, forgot to add that. I didn't expect this to get
> >>>>>> merged in 18.02 anyway, so v2 will follow.
> >>>>>
> >>>>> Please write 18.05 in the subject to show your expectation.
> >>>>> Thanks
> >>>>>
> >>>>
> >>>> Does it have to be an ABI change though? We can put numa_node_count
> >>>> after pointer to mem_config, in which case it won't be an ABI break.
> >>>> Would that be better?
> >>>
> >>> Changing the size of a struct which is allocated by the app,
> >>> is an ABI break.
> >>> Is your solution changing the size?
> >>>
> >>
> >> It's not really allocated as such. rte_config is a global static
> >> variable, and we only ever get pointers to it from the user code. If we
> >> add the new value at the end, all of the old data layout would be intact
> >> and work as before, so nothing would change as far as old code is concerned.
> >>
> >> However, if that's still considered an ABI break, then OK, break it is.
> >
> > Maybe that assuming it is never allocated (not copied for instance)
> > we could consider it is not an ABI break.
> >
> >> Some background for why this is needed - for the memory hotplug, we need
> >> to know how many sockets we can allocate memory at, to distinguish
> >> between socket that doesn't exist, and socket that exists but has no
> >> memory allocated on it. I'm OK with trying other approaches (such as
> >> storing numa nodes in a static variable somewhere) if breaking ABI for
> >> this is too much to ask for such a minute change.
> >
> > Why is it important for 18.02?
> > Memory hotplug will be integrated only in 18.05.
> > I think it is better to just wait (and announce the deprecation).
> >
>
> It isn't, i've already marked this patch as deferred. However, we'll
> have to have this discussion anyway :)
To be on the safe side, you announce a deprecation.
And there will be no debate in 18.05 (except if someone has a better idea).
@@ -57,6 +57,7 @@ rte_eal_cpu_init(void)
struct rte_config *config = rte_eal_get_configuration();
unsigned lcore_id;
unsigned count = 0;
+ unsigned int socket_id, max_socket_id = 0;
/*
* Parse the maximum set of logical cores, detect the subset of running
@@ -68,6 +69,19 @@ rte_eal_cpu_init(void)
/* init cpuset for per lcore config */
CPU_ZERO(&lcore_config[lcore_id].cpuset);
+ /* find socket first */
+ socket_id = eal_cpu_socket_id(lcore_id);
+ if (socket_id >= RTE_MAX_NUMA_NODES) {
+#ifdef RTE_EAL_ALLOW_INV_SOCKET_ID
+ socket_id = 0;
+#else
+ RTE_LOG(ERR, EAL, "Socket ID (%u) is greater than RTE_MAX_NUMA_NODES (%d)\n",
+ socket_id, RTE_MAX_NUMA_NODES);
+ return -1;
+#endif
+ }
+ max_socket_id = RTE_MAX(max_socket_id, socket_id);
+
/* in 1:1 mapping, record related cpu detected state */
lcore_config[lcore_id].detected = eal_cpu_detected(lcore_id);
if (lcore_config[lcore_id].detected == 0) {
@@ -83,18 +97,7 @@ rte_eal_cpu_init(void)
config->lcore_role[lcore_id] = ROLE_RTE;
lcore_config[lcore_id].core_role = ROLE_RTE;
lcore_config[lcore_id].core_id = eal_cpu_core_id(lcore_id);
- lcore_config[lcore_id].socket_id = eal_cpu_socket_id(lcore_id);
- if (lcore_config[lcore_id].socket_id >= RTE_MAX_NUMA_NODES) {
-#ifdef RTE_EAL_ALLOW_INV_SOCKET_ID
- lcore_config[lcore_id].socket_id = 0;
-#else
- RTE_LOG(ERR, EAL, "Socket ID (%u) is greater than "
- "RTE_MAX_NUMA_NODES (%d)\n",
- lcore_config[lcore_id].socket_id,
- RTE_MAX_NUMA_NODES);
- return -1;
-#endif
- }
+ lcore_config[lcore_id].socket_id = socket_id;
RTE_LOG(DEBUG, EAL, "Detected lcore %u as "
"core %u on socket %u\n",
lcore_id, lcore_config[lcore_id].core_id,
@@ -108,5 +111,15 @@ rte_eal_cpu_init(void)
RTE_MAX_LCORE);
RTE_LOG(INFO, EAL, "Detected %u lcore(s)\n", config->lcore_count);
+ config->numa_node_count = max_socket_id + 1;
+ RTE_LOG(INFO, EAL, "Detected %u NUMA nodes\n", config->numa_node_count);
+
return 0;
}
+
+unsigned int
+rte_num_sockets(void)
+{
+ const struct rte_config *config = rte_eal_get_configuration();
+ return config->numa_node_count;
+}
@@ -83,6 +83,7 @@ enum rte_proc_type_t {
struct rte_config {
uint32_t master_lcore; /**< Id of the master lcore */
uint32_t lcore_count; /**< Number of available logical cores. */
+ uint32_t numa_node_count; /**< Number of detected NUMA nodes. */
uint32_t service_lcore_count;/**< Number of available service cores. */
enum rte_lcore_role_t lcore_role[RTE_MAX_LCORE]; /**< State of cores. */
@@ -148,6 +148,14 @@ rte_lcore_index(int lcore_id)
unsigned rte_socket_id(void);
/**
+ * Return number of physical sockets on the system.
+ * @return
+ * the number of physical sockets as recognized by EAL
+ *
+ */
+unsigned int rte_num_sockets(void);
+
+/**
* Get the ID of the physical socket of the specified lcore
*
* @param lcore_id
@@ -200,6 +200,12 @@ DPDK_17.11 {
} DPDK_17.08;
+DPDK_18.02 {
+ global:
+
+ rte_num_sockets;
+} DPDK_17.11;
+
EXPERIMENTAL {
global: