| Message ID | 1424874849-8973-1-git-send-email-nhorman@tuxdriver.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
Hi Neil, Thanks for the cleanup. Does it better moving rte_socket_id() to eal_common_thread.c ? As it simply returns _socket_id, it's not necessary to have two copy in both linux and bsd. -Cunming > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Wednesday, February 25, 2015 10:34 PM > To: dev@dpdk.org > Cc: thomas.monjalon@6wind.com; Liang, Cunming; Neil Horman > Subject: [PATCH] eal: Clean up export of per_lcore__socket_id > > Theres no need to export this variable. Its set and queried from an API call > that doesn't exist in the hot path. Instead just export the rte_socket_id > symbol and make the variable private to protect it from type changes. We should > do this with the other exported variables too, but I think its too late in the > release cycle to do that. > > tested using distributor_autotest (which uses rte_socket_id), successfully. > Only tested on linux, as I don't currently have a bsd system spun up, but the > changes are symmetric, and should be fine > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > --- > lib/librte_eal/bsdapp/eal/eal_thread.c | 5 +++++ > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- > lib/librte_eal/common/eal_common_thread.c | 2 ++ > lib/librte_eal/common/include/rte_lcore.h | 7 +------ > lib/librte_eal/linuxapp/eal/eal_thread.c | 5 +++++ > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- > 6 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c > b/lib/librte_eal/bsdapp/eal/eal_thread.c > index ca95c72..5e6eea9 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_thread.c > +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c > @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = > LCORE_ID_ANY; > RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; > RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); > > +unsigned rte_socket_id(void) > +{ > + return RTE_PER_LCORE(_socket_id); > +} > + > /* > * Send a message to a slave lcore identified by slave_id to call a > * function f with argument arg. Once the execution is done, the > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > index 17515a9..d83524d 100644 > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > @@ -10,7 +10,6 @@ DPDK_2.0 { > pci_driver_list; > per_lcore__lcore_id; > per_lcore__rte_errno; > - per_lcore__socket_id; > rte_cpu_check_supported; > rte_cpu_get_flag_enabled; > rte_cycles_vmware_tsc_map; > @@ -82,6 +81,7 @@ DPDK_2.0 { > rte_set_log_level; > rte_set_log_type; > rte_snprintf; > + rte_socket_id; > rte_strerror; > rte_strsplit; > rte_sys_gettid; > diff --git a/lib/librte_eal/common/eal_common_thread.c > b/lib/librte_eal/common/eal_common_thread.c > index f4d9892..4010eab 100644 > --- a/lib/librte_eal/common/eal_common_thread.c > +++ b/lib/librte_eal/common/eal_common_thread.c > @@ -46,6 +46,8 @@ > > #include "eal_thread.h" > > +RTE_DECLARE_PER_LCORE(unsigned , _socket_id); > + > int eal_cpuset_socket_id(rte_cpuset_t *cpusetp) > { > unsigned cpu = 0; > diff --git a/lib/librte_eal/common/include/rte_lcore.h > b/lib/librte_eal/common/include/rte_lcore.h > index 20a58eb..e03264e 100644 > --- a/lib/librte_eal/common/include/rte_lcore.h > +++ b/lib/librte_eal/common/include/rte_lcore.h > @@ -81,7 +81,6 @@ struct lcore_config { > extern struct lcore_config lcore_config[RTE_MAX_LCORE]; > > RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per thread "lcore id". */ > -RTE_DECLARE_PER_LCORE(unsigned, _socket_id); /**< Per thread "socket id". > */ > RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread "cpuset". */ > > /** > @@ -145,11 +144,7 @@ rte_lcore_index(int lcore_id) > * @return > * the ID of current lcoreid's physical socket > */ > -static inline unsigned > -rte_socket_id(void) > -{ > - return RTE_PER_LCORE(_socket_id); > -} > +unsigned rte_socket_id(void); > > /** > * Get the ID of the physical socket of the specified lcore > diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c > b/lib/librte_eal/linuxapp/eal/eal_thread.c > index 5635c7d..9cacd86 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_thread.c > +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c > @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = > LCORE_ID_ANY; > RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; > RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); > > +unsigned rte_socket_id(void) > +{ > + return RTE_PER_LCORE(_socket_id); > +} > + > /* > * Send a message to a slave lcore identified by slave_id to call a > * function f with argument arg. Once the execution is done, the > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > index 17515a9..d83524d 100644 > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > @@ -10,7 +10,6 @@ DPDK_2.0 { > pci_driver_list; > per_lcore__lcore_id; > per_lcore__rte_errno; > - per_lcore__socket_id; > rte_cpu_check_supported; > rte_cpu_get_flag_enabled; > rte_cycles_vmware_tsc_map; > @@ -82,6 +81,7 @@ DPDK_2.0 { > rte_set_log_level; > rte_set_log_type; > rte_snprintf; > + rte_socket_id; > rte_strerror; > rte_strsplit; > rte_sys_gettid; > -- > 2.1.0
On Wed, Feb 25, 2015 at 11:54:51PM +0000, Liang, Cunming wrote: > Hi Neil, > > Thanks for the cleanup. > Does it better moving rte_socket_id() to eal_common_thread.c ? > As it simply returns _socket_id, it's not necessary to have two copy in both linux and bsd. > > -Cunming > Sure, I can respin this in the AM. neil > > -----Original Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: Wednesday, February 25, 2015 10:34 PM > > To: dev@dpdk.org > > Cc: thomas.monjalon@6wind.com; Liang, Cunming; Neil Horman > > Subject: [PATCH] eal: Clean up export of per_lcore__socket_id > > > > Theres no need to export this variable. Its set and queried from an API call > > that doesn't exist in the hot path. Instead just export the rte_socket_id > > symbol and make the variable private to protect it from type changes. We should > > do this with the other exported variables too, but I think its too late in the > > release cycle to do that. > > > > tested using distributor_autotest (which uses rte_socket_id), successfully. > > Only tested on linux, as I don't currently have a bsd system spun up, but the > > changes are symmetric, and should be fine > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com> > > --- > > lib/librte_eal/bsdapp/eal/eal_thread.c | 5 +++++ > > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- > > lib/librte_eal/common/eal_common_thread.c | 2 ++ > > lib/librte_eal/common/include/rte_lcore.h | 7 +------ > > lib/librte_eal/linuxapp/eal/eal_thread.c | 5 +++++ > > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- > > 6 files changed, 15 insertions(+), 8 deletions(-) > > > > diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c > > b/lib/librte_eal/bsdapp/eal/eal_thread.c > > index ca95c72..5e6eea9 100644 > > --- a/lib/librte_eal/bsdapp/eal/eal_thread.c > > +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c > > @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = > > LCORE_ID_ANY; > > RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; > > RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); > > > > +unsigned rte_socket_id(void) > > +{ > > + return RTE_PER_LCORE(_socket_id); > > +} > > + > > /* > > * Send a message to a slave lcore identified by slave_id to call a > > * function f with argument arg. Once the execution is done, the > > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > > b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > > index 17515a9..d83524d 100644 > > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map > > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map > > @@ -10,7 +10,6 @@ DPDK_2.0 { > > pci_driver_list; > > per_lcore__lcore_id; > > per_lcore__rte_errno; > > - per_lcore__socket_id; > > rte_cpu_check_supported; > > rte_cpu_get_flag_enabled; > > rte_cycles_vmware_tsc_map; > > @@ -82,6 +81,7 @@ DPDK_2.0 { > > rte_set_log_level; > > rte_set_log_type; > > rte_snprintf; > > + rte_socket_id; > > rte_strerror; > > rte_strsplit; > > rte_sys_gettid; > > diff --git a/lib/librte_eal/common/eal_common_thread.c > > b/lib/librte_eal/common/eal_common_thread.c > > index f4d9892..4010eab 100644 > > --- a/lib/librte_eal/common/eal_common_thread.c > > +++ b/lib/librte_eal/common/eal_common_thread.c > > @@ -46,6 +46,8 @@ > > > > #include "eal_thread.h" > > > > +RTE_DECLARE_PER_LCORE(unsigned , _socket_id); > > + > > int eal_cpuset_socket_id(rte_cpuset_t *cpusetp) > > { > > unsigned cpu = 0; > > diff --git a/lib/librte_eal/common/include/rte_lcore.h > > b/lib/librte_eal/common/include/rte_lcore.h > > index 20a58eb..e03264e 100644 > > --- a/lib/librte_eal/common/include/rte_lcore.h > > +++ b/lib/librte_eal/common/include/rte_lcore.h > > @@ -81,7 +81,6 @@ struct lcore_config { > > extern struct lcore_config lcore_config[RTE_MAX_LCORE]; > > > > RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per thread "lcore id". */ > > -RTE_DECLARE_PER_LCORE(unsigned, _socket_id); /**< Per thread "socket id". > > */ > > RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread "cpuset". */ > > > > /** > > @@ -145,11 +144,7 @@ rte_lcore_index(int lcore_id) > > * @return > > * the ID of current lcoreid's physical socket > > */ > > -static inline unsigned > > -rte_socket_id(void) > > -{ > > - return RTE_PER_LCORE(_socket_id); > > -} > > +unsigned rte_socket_id(void); > > > > /** > > * Get the ID of the physical socket of the specified lcore > > diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c > > b/lib/librte_eal/linuxapp/eal/eal_thread.c > > index 5635c7d..9cacd86 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_thread.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c > > @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = > > LCORE_ID_ANY; > > RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; > > RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); > > > > +unsigned rte_socket_id(void) > > +{ > > + return RTE_PER_LCORE(_socket_id); > > +} > > + > > /* > > * Send a message to a slave lcore identified by slave_id to call a > > * function f with argument arg. Once the execution is done, the > > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > > b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > > index 17515a9..d83524d 100644 > > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map > > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map > > @@ -10,7 +10,6 @@ DPDK_2.0 { > > pci_driver_list; > > per_lcore__lcore_id; > > per_lcore__rte_errno; > > - per_lcore__socket_id; > > rte_cpu_check_supported; > > rte_cpu_get_flag_enabled; > > rte_cycles_vmware_tsc_map; > > @@ -82,6 +81,7 @@ DPDK_2.0 { > > rte_set_log_level; > > rte_set_log_type; > > rte_snprintf; > > + rte_socket_id; > > rte_strerror; > > rte_strsplit; > > rte_sys_gettid; > > -- > > 2.1.0 > >
diff --git a/lib/librte_eal/bsdapp/eal/eal_thread.c b/lib/librte_eal/bsdapp/eal/eal_thread.c index ca95c72..5e6eea9 100644 --- a/lib/librte_eal/bsdapp/eal/eal_thread.c +++ b/lib/librte_eal/bsdapp/eal/eal_thread.c @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY; RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); +unsigned rte_socket_id(void) +{ + return RTE_PER_LCORE(_socket_id); +} + /* * Send a message to a slave lcore identified by slave_id to call a * function f with argument arg. Once the execution is done, the diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map index 17515a9..d83524d 100644 --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map @@ -10,7 +10,6 @@ DPDK_2.0 { pci_driver_list; per_lcore__lcore_id; per_lcore__rte_errno; - per_lcore__socket_id; rte_cpu_check_supported; rte_cpu_get_flag_enabled; rte_cycles_vmware_tsc_map; @@ -82,6 +81,7 @@ DPDK_2.0 { rte_set_log_level; rte_set_log_type; rte_snprintf; + rte_socket_id; rte_strerror; rte_strsplit; rte_sys_gettid; diff --git a/lib/librte_eal/common/eal_common_thread.c b/lib/librte_eal/common/eal_common_thread.c index f4d9892..4010eab 100644 --- a/lib/librte_eal/common/eal_common_thread.c +++ b/lib/librte_eal/common/eal_common_thread.c @@ -46,6 +46,8 @@ #include "eal_thread.h" +RTE_DECLARE_PER_LCORE(unsigned , _socket_id); + int eal_cpuset_socket_id(rte_cpuset_t *cpusetp) { unsigned cpu = 0; diff --git a/lib/librte_eal/common/include/rte_lcore.h b/lib/librte_eal/common/include/rte_lcore.h index 20a58eb..e03264e 100644 --- a/lib/librte_eal/common/include/rte_lcore.h +++ b/lib/librte_eal/common/include/rte_lcore.h @@ -81,7 +81,6 @@ struct lcore_config { extern struct lcore_config lcore_config[RTE_MAX_LCORE]; RTE_DECLARE_PER_LCORE(unsigned, _lcore_id); /**< Per thread "lcore id". */ -RTE_DECLARE_PER_LCORE(unsigned, _socket_id); /**< Per thread "socket id". */ RTE_DECLARE_PER_LCORE(rte_cpuset_t, _cpuset); /**< Per thread "cpuset". */ /** @@ -145,11 +144,7 @@ rte_lcore_index(int lcore_id) * @return * the ID of current lcoreid's physical socket */ -static inline unsigned -rte_socket_id(void) -{ - return RTE_PER_LCORE(_socket_id); -} +unsigned rte_socket_id(void); /** * Get the ID of the physical socket of the specified lcore diff --git a/lib/librte_eal/linuxapp/eal/eal_thread.c b/lib/librte_eal/linuxapp/eal/eal_thread.c index 5635c7d..9cacd86 100644 --- a/lib/librte_eal/linuxapp/eal/eal_thread.c +++ b/lib/librte_eal/linuxapp/eal/eal_thread.c @@ -60,6 +60,11 @@ RTE_DEFINE_PER_LCORE(unsigned, _lcore_id) = LCORE_ID_ANY; RTE_DEFINE_PER_LCORE(unsigned, _socket_id) = (unsigned)SOCKET_ID_ANY; RTE_DEFINE_PER_LCORE(rte_cpuset_t, _cpuset); +unsigned rte_socket_id(void) +{ + return RTE_PER_LCORE(_socket_id); +} + /* * Send a message to a slave lcore identified by slave_id to call a * function f with argument arg. Once the execution is done, the diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map index 17515a9..d83524d 100644 --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map @@ -10,7 +10,6 @@ DPDK_2.0 { pci_driver_list; per_lcore__lcore_id; per_lcore__rte_errno; - per_lcore__socket_id; rte_cpu_check_supported; rte_cpu_get_flag_enabled; rte_cycles_vmware_tsc_map; @@ -82,6 +81,7 @@ DPDK_2.0 { rte_set_log_level; rte_set_log_type; rte_snprintf; + rte_socket_id; rte_strerror; rte_strsplit; rte_sys_gettid;
Theres no need to export this variable. Its set and queried from an API call that doesn't exist in the hot path. Instead just export the rte_socket_id symbol and make the variable private to protect it from type changes. We should do this with the other exported variables too, but I think its too late in the release cycle to do that. tested using distributor_autotest (which uses rte_socket_id), successfully. Only tested on linux, as I don't currently have a bsd system spun up, but the changes are symmetric, and should be fine Signed-off-by: Neil Horman <nhorman@tuxdriver.com> --- lib/librte_eal/bsdapp/eal/eal_thread.c | 5 +++++ lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- lib/librte_eal/common/eal_common_thread.c | 2 ++ lib/librte_eal/common/include/rte_lcore.h | 7 +------ lib/librte_eal/linuxapp/eal/eal_thread.c | 5 +++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- 6 files changed, 15 insertions(+), 8 deletions(-)