[dpdk-dev,v5,3/8] Move common functions in eal_lcore.c

Message ID 1428608412-31191-4-git-send-email-rkerur@gmail.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Ravi Kerur April 9, 2015, 7:40 p.m. UTC
  Changes in v5
Rebase to latest code.

Changes in v4
Implement cpu_detected() for BSD.
Have common RTE_LOG for Linux and BSD in rte_eal_cpu_init().
Remove RTE_EXEC_ENV_BSDAPP in common file.

Changes in v3
Changed subject to be more explicit on file name inclusion.

Changes in v2
None

Changes in v1
Move common function in eal_lcore.c to librte_eal/common/
eal_common_lcore.c file.

Following function is  moved to eal_common_lcore.c file

int rte_eal_cpu_init(void);

Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
common function.
Makefile changes to reflect new file added.
Fix checkpatch warnings and errors.

Signed-off-by: Ravi Kerur <rkerur@gmail.com>
---
 lib/librte_eal/bsdapp/eal/Makefile       |   1 +
 lib/librte_eal/bsdapp/eal/eal_lcore.c    |  72 +++++----------------
 lib/librte_eal/common/eal_common_lcore.c | 107 +++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_private.h      |  14 ++++
 lib/librte_eal/linuxapp/eal/Makefile     |   2 +
 lib/librte_eal/linuxapp/eal/eal_lcore.c  |  66 ++-----------------
 6 files changed, 143 insertions(+), 119 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_common_lcore.c
  

Comments

Thomas Monjalon April 16, 2015, 8:39 a.m. UTC | #1
2015-04-09 12:40, Ravi Kerur:
> Changes in v5
> Rebase to latest code.
> 
> Changes in v4
> Implement cpu_detected() for BSD.
> Have common RTE_LOG for Linux and BSD in rte_eal_cpu_init().
> Remove RTE_EXEC_ENV_BSDAPP in common file.
> 
> Changes in v3
> Changed subject to be more explicit on file name inclusion.
> 
> Changes in v2
> None
> 
> Changes in v1
> Move common function in eal_lcore.c to librte_eal/common/
> eal_common_lcore.c file.
> 
> Following function is  moved to eal_common_lcore.c file
> 
> int rte_eal_cpu_init(void);
> 
> Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
> common function.
> Makefile changes to reflect new file added.
> Fix checkpatch warnings and errors.
> 
> Signed-off-by: Ravi Kerur <rkerur@gmail.com>
[...]
> --- a/lib/librte_eal/bsdapp/eal/eal_lcore.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_lcore.c
[...]
>  static int
> -get_ncpus(void)
> +eal_get_ncpus(void)

Why not keep it static as it seems to be specific to BSD handling?

>  {
>  	int mib[2] = {CTL_HW, HW_NCPU};
>  	int ncpu;
> @@ -59,63 +62,18 @@ get_ncpus(void)
>  	return ncpu;
>  }
[...]
> -unsigned
> -eal_cpu_socket_id(__rte_unused unsigned cpu_id)
> +/* Check if a cpu is present by the presence of the
> + * cpu information for it.
> + */
> +int
> +eal_cpu_detected(unsigned lcore_id)
>  {
> -	return cpu_socket_id(cpu_id);
> +	const unsigned ncpus = eal_get_ncpus();
> +	return (lcore_id < ncpus);
>  }
  
Ravi Kerur April 16, 2015, 10:10 p.m. UTC | #2
On Thu, Apr 16, 2015 at 1:39 AM, Thomas Monjalon <thomas.monjalon@6wind.com>
wrote:

> 2015-04-09 12:40, Ravi Kerur:
> > Changes in v5
> > Rebase to latest code.
> >
> > Changes in v4
> > Implement cpu_detected() for BSD.
> > Have common RTE_LOG for Linux and BSD in rte_eal_cpu_init().
> > Remove RTE_EXEC_ENV_BSDAPP in common file.
> >
> > Changes in v3
> > Changed subject to be more explicit on file name inclusion.
> >
> > Changes in v2
> > None
> >
> > Changes in v1
> > Move common function in eal_lcore.c to librte_eal/common/
> > eal_common_lcore.c file.
> >
> > Following function is  moved to eal_common_lcore.c file
> >
> > int rte_eal_cpu_init(void);
> >
> > Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
> > common function.
> > Makefile changes to reflect new file added.
> > Fix checkpatch warnings and errors.
> >
> > Signed-off-by: Ravi Kerur <rkerur@gmail.com>
> [...]
> > --- a/lib/librte_eal/bsdapp/eal/eal_lcore.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal_lcore.c
> [...]
> >  static int
> > -get_ncpus(void)
> > +eal_get_ncpus(void)
>
> Why not keep it static as it seems to be specific to BSD handling?
>

It is static with the changes, "name" is changed from "get_ncpus" to
"eal_get_ncpus".

>
> >  {
> >       int mib[2] = {CTL_HW, HW_NCPU};
> >       int ncpu;
> > @@ -59,63 +62,18 @@ get_ncpus(void)
> >       return ncpu;
> >  }
> [...]
> > -unsigned
> > -eal_cpu_socket_id(__rte_unused unsigned cpu_id)
> > +/* Check if a cpu is present by the presence of the
> > + * cpu information for it.
> > + */
> > +int
> > +eal_cpu_detected(unsigned lcore_id)
> >  {
> > -     return cpu_socket_id(cpu_id);
> > +     const unsigned ncpus = eal_get_ncpus();
> > +     return (lcore_id < ncpus);
> >  }
>
>
>
>
  

Patch

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index d0a7c43..a28ead3 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -80,6 +80,7 @@  SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_thread.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_system.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_runtime.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_lcore.c
 
 CFLAGS_eal.o := -D_GNU_SOURCE
 #CFLAGS_eal_thread.o := -D_GNU_SOURCE
diff --git a/lib/librte_eal/bsdapp/eal/eal_lcore.c b/lib/librte_eal/bsdapp/eal/eal_lcore.c
index 162fb4f..b47eb1b 100644
--- a/lib/librte_eal/bsdapp/eal/eal_lcore.c
+++ b/lib/librte_eal/bsdapp/eal/eal_lcore.c
@@ -44,11 +44,14 @@ 
 #include "eal_thread.h"
 
 /* No topology information available on FreeBSD including NUMA info */
-#define cpu_core_id(X) 0
-#define cpu_socket_id(X) 0
+unsigned
+eal_cpu_core_id(__rte_unused unsigned lcore_id)
+{
+	return 0;
+}
 
 static int
-get_ncpus(void)
+eal_get_ncpus(void)
 {
 	int mib[2] = {CTL_HW, HW_NCPU};
 	int ncpu;
@@ -59,63 +62,18 @@  get_ncpus(void)
 	return ncpu;
 }
 
-/*
- * fill the cpu_info structure with as much info as we can get.
- * code is similar to linux version, but sadly available info is less.
- */
-int
-rte_eal_cpu_init(void)
+unsigned
+eal_cpu_socket_id(__rte_unused unsigned cpu_id)
 {
-	/* pointer to global configuration */
-	struct rte_config *config = rte_eal_get_configuration();
-	unsigned lcore_id;
-	unsigned count = 0;
-
-	const unsigned ncpus = get_ncpus();
-	/*
-	 * Parse the maximum set of logical cores, detect the subset of running
-	 * ones and enable them by default.
-	 */
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-		/* init cpuset for per lcore config */
-		CPU_ZERO(&lcore_config[lcore_id].cpuset);
-
-		lcore_config[lcore_id].detected = (lcore_id < ncpus);
-		if (lcore_config[lcore_id].detected == 0) {
-			config->lcore_role[lcore_id] = ROLE_OFF;
-			continue;
-		}
-
-		/* By default, lcore 1:1 map to cpu id */
-		CPU_SET(lcore_id, &lcore_config[lcore_id].cpuset);
-
-		/* By default, each detected core is enabled */
-		config->lcore_role[lcore_id] = ROLE_RTE;
-		lcore_config[lcore_id].core_id = cpu_core_id(lcore_id);
-		lcore_config[lcore_id].socket_id = 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_panic("Socket ID (%u) is greater than "
-				"RTE_MAX_NUMA_NODES (%d)\n",
-				lcore_config[lcore_id].socket_id, RTE_MAX_NUMA_NODES);
-#endif
-		RTE_LOG(DEBUG, EAL, "Detected lcore %u\n",
-				lcore_id);
-		count++;
-	}
-	/* Set the count of enabled logical cores of the EAL configuration */
-	config->lcore_count = count;
-	RTE_LOG(DEBUG, EAL, "Support maximum %u logical core(s) by configuration.\n",
-		RTE_MAX_LCORE);
-	RTE_LOG(DEBUG, EAL, "Detected %u lcore(s)\n", config->lcore_count);
-
 	return 0;
 }
 
-unsigned
-eal_cpu_socket_id(__rte_unused unsigned cpu_id)
+/* Check if a cpu is present by the presence of the
+ * cpu information for it.
+ */
+int
+eal_cpu_detected(unsigned lcore_id)
 {
-	return cpu_socket_id(cpu_id);
+	const unsigned ncpus = eal_get_ncpus();
+	return (lcore_id < ncpus);
 }
diff --git a/lib/librte_eal/common/eal_common_lcore.c b/lib/librte_eal/common/eal_common_lcore.c
new file mode 100644
index 0000000..845140b
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_lcore.c
@@ -0,0 +1,107 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <unistd.h>
+#include <limits.h>
+#include <string.h>
+#include <dirent.h>
+
+#include <rte_log.h>
+#include <rte_eal.h>
+#include <rte_lcore.h>
+#include <rte_common.h>
+#include <rte_debug.h>
+
+#include "eal_private.h"
+#include "eal_thread.h"
+
+/*
+ * Parse /sys/devices/system/cpu to get the number of physical and logical
+ * processors on the machine. The function will fill the cpu_info
+ * structure.
+ */
+int
+rte_eal_cpu_init(void)
+{
+	/* pointer to global configuration */
+	struct rte_config *config = rte_eal_get_configuration();
+	unsigned lcore_id;
+	unsigned count = 0;
+
+	/*
+	 * Parse the maximum set of logical cores, detect the subset of running
+	 * ones and enable them by default.
+	 */
+	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
+		/* init cpuset for per lcore config */
+		CPU_ZERO(&lcore_config[lcore_id].cpuset);
+
+		/* 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) {
+			config->lcore_role[lcore_id] = ROLE_OFF;
+			continue;
+		}
+
+		/* By default, lcore 1:1 map to cpu id */
+		CPU_SET(lcore_id, &lcore_config[lcore_id].cpuset);
+
+		/* By default, each detected core is enabled */
+		config->lcore_role[lcore_id] = 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_panic("Socket ID (%u) is greater than "
+				"RTE_MAX_NUMA_NODES (%d)\n",
+				lcore_config[lcore_id].socket_id,
+				RTE_MAX_NUMA_NODES);
+#endif
+
+		RTE_LOG(DEBUG, EAL, "Detected lcore %u as "
+				"core %u on socket %u\n",
+				lcore_id, lcore_config[lcore_id].core_id,
+				lcore_config[lcore_id].socket_id);
+		count++;
+	}
+	/* Set the count of enabled logical cores of the EAL configuration */
+	config->lcore_count = count;
+	RTE_LOG(DEBUG, EAL,
+		"Support maximum %u logical core(s) by configuration.\n",
+		RTE_MAX_LCORE);
+	RTE_LOG(DEBUG, EAL, "Detected %u lcore(s)\n", config->lcore_count);
+
+	return 0;
+}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index cdcf4b9..458e8c1 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -310,4 +310,18 @@  void rte_eal_config_attach(void);
  */
 void rte_eal_config_reattach(void);
 
+/**
+ * This function gets cpu core_id
+ *
+ * This function is private to the EAL.
+ */
+unsigned eal_cpu_core_id(unsigned lcore_id);
+
+/**
+ * This function check if cpu is present
+ *
+ * This function is private to the EAL.
+ */
+int eal_cpu_detected(unsigned lcore_id);
+
 #endif /* _EAL_PRIVATE_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index c36d5f4..8ae23a0 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -92,6 +92,7 @@  SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_thread.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_system.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_runtime.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_common_lcore.c
 
 CFLAGS_eal.o := -D_GNU_SOURCE
 CFLAGS_eal_interrupts.o := -D_GNU_SOURCE
@@ -106,6 +107,7 @@  CFLAGS_eal_pci_vfio.o := -D_GNU_SOURCE
 CFLAGS_eal_common_whitelist.o := -D_GNU_SOURCE
 CFLAGS_eal_common_options.o := -D_GNU_SOURCE
 CFLAGS_eal_common_thread.o := -D_GNU_SOURCE
+CFLAGS_eal_common_lcore.o := -D_GNU_SOURCE
 
 # workaround for a gcc bug with noreturn attribute
 # http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12603
diff --git a/lib/librte_eal/linuxapp/eal/eal_lcore.c b/lib/librte_eal/linuxapp/eal/eal_lcore.c
index ef8c433..9af6d38 100644
--- a/lib/librte_eal/linuxapp/eal/eal_lcore.c
+++ b/lib/librte_eal/linuxapp/eal/eal_lcore.c
@@ -52,8 +52,8 @@ 
 #define PHYS_PKG_FILE "topology/physical_package_id"
 
 /* Check if a cpu is present by the presence of the cpu information for it */
-static int
-cpu_detected(unsigned lcore_id)
+int
+eal_cpu_detected(unsigned lcore_id)
 {
 	char path[PATH_MAX];
 	int len = snprintf(path, sizeof(path), SYS_CPU_DIR
@@ -122,8 +122,8 @@  err:
 }
 
 /* Get the cpu core id value from the /sys/.../cpuX core_id value */
-static unsigned
-cpu_core_id(unsigned lcore_id)
+unsigned
+eal_cpu_core_id(unsigned lcore_id)
 {
 	char path[PATH_MAX];
 	unsigned long id;
@@ -140,61 +140,3 @@  err:
 			"for lcore %u - assuming core 0\n", SYS_CPU_DIR, lcore_id);
 	return 0;
 }
-
-/*
- * Parse /sys/devices/system/cpu to get the number of physical and logical
- * processors on the machine. The function will fill the cpu_info
- * structure.
- */
-int
-rte_eal_cpu_init(void)
-{
-	/* pointer to global configuration */
-	struct rte_config *config = rte_eal_get_configuration();
-	unsigned lcore_id;
-	unsigned count = 0;
-
-	/*
-	 * Parse the maximum set of logical cores, detect the subset of running
-	 * ones and enable them by default.
-	 */
-	for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
-		/* init cpuset for per lcore config */
-		CPU_ZERO(&lcore_config[lcore_id].cpuset);
-
-		/* in 1:1 mapping, record related cpu detected state */
-		lcore_config[lcore_id].detected = cpu_detected(lcore_id);
-		if (lcore_config[lcore_id].detected == 0) {
-			config->lcore_role[lcore_id] = ROLE_OFF;
-			continue;
-		}
-
-		/* By default, lcore 1:1 map to cpu id */
-		CPU_SET(lcore_id, &lcore_config[lcore_id].cpuset);
-
-		/* By default, each detected core is enabled */
-		config->lcore_role[lcore_id] = ROLE_RTE;
-		lcore_config[lcore_id].core_id = 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_panic("Socket ID (%u) is greater than "
-				"RTE_MAX_NUMA_NODES (%d)\n",
-				lcore_config[lcore_id].socket_id, RTE_MAX_NUMA_NODES);
-#endif
-		RTE_LOG(DEBUG, EAL, "Detected lcore %u as core %u on socket %u\n",
-				lcore_id,
-				lcore_config[lcore_id].core_id,
-				lcore_config[lcore_id].socket_id);
-		count ++;
-	}
-	/* Set the count of enabled logical cores of the EAL configuration */
-	config->lcore_count = count;
-	RTE_LOG(DEBUG, EAL, "Support maximum %u logical core(s) by configuration.\n",
-		RTE_MAX_LCORE);
-	RTE_LOG(DEBUG, EAL, "Detected %u lcore(s)\n", config->lcore_count);
-
-	return 0;
-}