Re: [dpdk-dev] [PATCH 4/7] Move EAL common functions

[Neil Horman <nhorman@tuxdriver.com>]

On Thu, Dec 25, 2014 at 10:33:14AM -0500, Ravi Kerur wrote:
> Move common functions in eal.c to librte_eal/common directory.
> Use RTE_EXEC_ENV_BSDAPP to differentiate minor differences in
> common functions.
> Makefile changes to reflect new file.
> 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.c       | 233 +-----------------------
>  lib/librte_eal/common/eal_common.c    | 328 ++++++++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_externs.h   |  42 +++++
>  lib/librte_eal/common/eal_hugepages.h |   1 +
>  lib/librte_eal/common/eal_private.h   |  47 +++++
>  lib/librte_eal/linuxapp/eal/Makefile  |   1 +
>  lib/librte_eal/linuxapp/eal/eal.c     | 246 ++-----------------------
>  8 files changed, 439 insertions(+), 460 deletions(-)
>  create mode 100644 lib/librte_eal/common/eal_common.c
>  create mode 100644 lib/librte_eal/common/eal_externs.h
> 
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index 92dd9a6..050d70b 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -58,6 +58,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_interrupts.c
>  SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_alarm.c
>  
>  # from common dir
> +SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common.c
>  SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_memzone.c
>  SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_log.c
>  SRCS-$(CONFIG_RTE_LIBRTE_EAL_BSDAPP) += eal_common_launch.c
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 69f3c03..f925da7 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -80,30 +80,10 @@
>  #include "eal_filesystem.h"
>  #include "eal_hugepages.h"
>  #include "eal_options.h"
> +#include "eal_externs.h"
>  
>  #define MEMSIZE_IF_NO_HUGE_PAGE (64ULL * 1024ULL * 1024ULL)
>  
> -/* Allow the application to print its usage message too if set */
> -static rte_usage_hook_t	rte_application_usage_hook = NULL;
> -/* early configuration structure, when memory config is not mmapped */
> -static struct rte_mem_config early_mem_config;
> -
> -/* define fd variable here, because file needs to be kept open for the
> - * duration of the program, as we hold a write lock on it in the primary proc */
> -static int mem_cfg_fd = -1;
> -
> -static struct flock wr_lock = {
> -		.l_type = F_WRLCK,
> -		.l_whence = SEEK_SET,
> -		.l_start = offsetof(struct rte_mem_config, memseg),
> -		.l_len = sizeof(early_mem_config.memseg),
> -};
> -
> -/* Address of global and public configuration */
> -static struct rte_config rte_config = {
> -		.mem_config = &early_mem_config,
> -};
> -
>  /* internal configuration (per-core) */
>  struct lcore_config lcore_config[RTE_MAX_LCORE];
>  
> @@ -113,93 +93,14 @@ struct internal_config internal_config;
>  /* used by rte_rdtsc() */
>  int rte_cycles_vmware_tsc_map;
>  
> -/* Return a pointer to the configuration structure */
> -struct rte_config *
> -rte_eal_get_configuration(void)
> +inline void *
> +rte_eal_get_mem_cfg_addr(void)
>  {
> -	return &rte_config;
> -}
> -
> -/* parse a sysfs (or other) file containing one integer value */
> -int
> -eal_parse_sysfs_value(const char *filename, unsigned long *val)
> -{
> -	FILE *f;
> -	char buf[BUFSIZ];
> -	char *end = NULL;
> -
> -	if ((f = fopen(filename, "r")) == NULL) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
> -			__func__, filename);
> -		return -1;
> -	}
> -
> -	if (fgets(buf, sizeof(buf), f) == NULL) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
> -			__func__, filename);
> -		fclose(f);
> -		return -1;
> -	}
> -	*val = strtoul(buf, &end, 0);
> -	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
> -		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
> -				__func__, filename);
> -		fclose(f);
> -		return -1;
> -	}
> -	fclose(f);
> -	return 0;
> -}
> -
> -
> -/* create memory configuration in shared/mmap memory. Take out
> - * a write lock on the memsegs, so we can auto-detect primary/secondary.
> - * This means we never close the file while running (auto-close on exit).
> - * We also don't lock the whole file, so that in future we can use read-locks
> - * on other parts, e.g. memzones, to detect if there are running secondary
> - * processes. */
> -static void
> -rte_eal_config_create(void)
> -{
> -	void *rte_mem_cfg_addr;
> -	int retval;
> -
> -	const char *pathname = eal_runtime_config_path();
> -
> -	if (internal_config.no_shconf)
> -		return;
> -
> -	if (mem_cfg_fd < 0){
> -		mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
> -		if (mem_cfg_fd < 0)
> -			rte_panic("Cannot open '%s' for rte_mem_config\n", pathname);
> -	}
> -
> -	retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
> -	if (retval < 0){
> -		close(mem_cfg_fd);
> -		rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
> -	}
> -
> -	retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
> -	if (retval < 0){
> -		close(mem_cfg_fd);
> -		rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. Is another primary "
> -				"process running?\n", pathname);
> -	}
> -
> -	rte_mem_cfg_addr = mmap(NULL, sizeof(*rte_config.mem_config),
> -				PROT_READ | PROT_WRITE, MAP_SHARED, mem_cfg_fd, 0);
> -
> -	if (rte_mem_cfg_addr == MAP_FAILED){
> -		rte_panic("Cannot mmap memory for rte_config\n");
> -	}
> -	memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
> -	rte_config.mem_config = (struct rte_mem_config *) rte_mem_cfg_addr;
> +	return NULL;
>  }
>  
>  /* attach to an existing shared memory config */
> -static void
> +void
>  rte_eal_config_attach(void)
>  {
>  	void *rte_mem_cfg_addr;
> @@ -223,44 +124,11 @@ rte_eal_config_attach(void)
>  	rte_config.mem_config = (struct rte_mem_config *) rte_mem_cfg_addr;
>  }
>  
> -/* Detect if we are a primary or a secondary process */
> -enum rte_proc_type_t
> -eal_proc_type_detect(void)
> +/* NOP for BSD */
> +void
> +rte_eal_config_reattach(void)
>  {
> -	enum rte_proc_type_t ptype = RTE_PROC_PRIMARY;
> -	const char *pathname = eal_runtime_config_path();
> -
> -	/* if we can open the file but not get a write-lock we are a secondary
> -	 * process. NOTE: if we get a file handle back, we keep that open
> -	 * and don't close it to prevent a race condition between multiple opens */
> -	if (((mem_cfg_fd = open(pathname, O_RDWR)) >= 0) &&
> -			(fcntl(mem_cfg_fd, F_SETLK, &wr_lock) < 0))
> -		ptype = RTE_PROC_SECONDARY;
> -
> -	RTE_LOG(INFO, EAL, "Auto-detected process type: %s\n",
> -			ptype == RTE_PROC_PRIMARY ? "PRIMARY" : "SECONDARY");
>  
> -	return ptype;
> -}
> -
> -/* Sets up rte_config structure with the pointer to shared memory config.*/
> -static void
> -rte_config_init(void)
> -{
> -	rte_config.process_type = internal_config.process_type;
> -
> -	switch (rte_config.process_type){
> -	case RTE_PROC_PRIMARY:
> -		rte_eal_config_create();
> -		break;
> -	case RTE_PROC_SECONDARY:
> -		rte_eal_config_attach();
> -		rte_eal_mcfg_wait_complete(rte_config.mem_config);
> -		break;
> -	case RTE_PROC_AUTO:
> -	case RTE_PROC_INVALID:
> -		rte_panic("Invalid process type\n");
> -	}
>  }
>  
>  /* display usage */
> @@ -276,37 +144,6 @@ eal_usage(const char *prgname)
>  	}
>  }
>  
> -/* Set a per-application usage message */
> -rte_usage_hook_t
> -rte_set_application_usage_hook( rte_usage_hook_t usage_func )
> -{
> -	rte_usage_hook_t	old_func;
> -
> -	/* Will be NULL on the first call to denote the last usage routine. */
> -	old_func					= rte_application_usage_hook;
> -	rte_application_usage_hook	= usage_func;
> -
> -	return old_func;
> -}
> -
> -static inline size_t
> -eal_get_hugepage_mem_size(void)
> -{
> -	uint64_t size = 0;
> -	unsigned i, j;
> -
> -	for (i = 0; i < internal_config.num_hugepage_sizes; i++) {
> -		struct hugepage_info *hpi = &internal_config.hugepage_info[i];
> -		if (hpi->hugedir != NULL) {
> -			for (j = 0; j < RTE_MAX_NUMA_NODES; j++) {
> -				size += hpi->hugepage_sz * hpi->num_pages[j];
> -			}
> -		}
> -	}
> -
> -	return (size < SIZE_MAX) ? (size_t)(size) : SIZE_MAX;
> -}
> -
>  /* Parse the argument given in the command line of the application */
>  static int
>  eal_parse_args(int argc, char **argv)
> @@ -374,45 +211,6 @@ eal_parse_args(int argc, char **argv)
>  	return ret;
>  }
>  
> -static void
> -eal_check_mem_on_local_socket(void)
> -{
> -	const struct rte_memseg *ms;
> -	int i, socket_id;
> -
> -	socket_id = rte_lcore_to_socket_id(rte_config.master_lcore);
> -
> -	ms = rte_eal_get_physmem_layout();
> -
> -	for (i = 0; i < RTE_MAX_MEMSEG; i++)
> -		if (ms[i].socket_id == socket_id &&
> -				ms[i].len > 0)
> -			return;
> -
> -	RTE_LOG(WARNING, EAL, "WARNING: Master core has no "
> -			"memory on local socket!\n");
> -}
> -
> -static int
> -sync_func(__attribute__((unused)) void *arg)
> -{
> -	return 0;
> -}
> -
> -inline static void
> -rte_eal_mcfg_complete(void)
> -{
> -	/* ALL shared mem_config related INIT DONE */
> -	if (rte_config.process_type == RTE_PROC_PRIMARY)
> -		rte_config.mem_config->magic = RTE_MAGIC;
> -}
> -
> -/* return non-zero if hugepages are enabled. */
> -int rte_eal_has_hugepages(void)
> -{
> -	return !internal_config.no_hugetlbfs;
> -}
> -
>  /* Abstraction for port I/0 privilege */
>  int
>  rte_eal_iopl_init(void)
> @@ -476,7 +274,7 @@ rte_eal_init(int argc, char **argv)
>  
>  	rte_srand(rte_rdtsc());
>  
> -	rte_config_init();
> +	rte_eal_config_init();
>  
>  	if (rte_eal_memory_init() < 0)
>  		rte_panic("Cannot init memory\n");
> @@ -548,16 +346,3 @@ rte_eal_init(int argc, char **argv)
>  	return fctret;
>  }
>  
> -/* get core role */
> -enum rte_lcore_role_t
> -rte_eal_lcore_role(unsigned lcore_id)
> -{
> -	return (rte_config.lcore_role[lcore_id]);
> -}
> -
> -enum rte_proc_type_t
> -rte_eal_process_type(void)
> -{
> -	return (rte_config.process_type);
> -}
> -
> diff --git a/lib/librte_eal/common/eal_common.c b/lib/librte_eal/common/eal_common.c
> new file mode 100644
> index 0000000..becf9c8
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common.c
> @@ -0,0 +1,328 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2014 6WIND S.A.
> + *   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 <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <string.h>
> +#include <stdarg.h>
> +#include <unistd.h>
> +#include <pthread.h>
> +#include <syslog.h>
> +#include <getopt.h>
> +#include <sys/file.h>
> +#include <stddef.h>
> +#include <errno.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include <sys/mman.h>
> +#include <sys/queue.h>
> +
> +#include <rte_common.h>
> +#include <rte_debug.h>
> +#include <rte_memory.h>
> +#include <rte_memzone.h>
> +#include <rte_launch.h>
> +#include <rte_tailq.h>
> +#include <rte_eal.h>
> +#include <rte_eal_memconfig.h>
> +#include <rte_per_lcore.h>
> +#include <rte_lcore.h>
> +#include <rte_log.h>
> +#include <rte_random.h>
> +#include <rte_cycles.h>
> +#include <rte_string_fns.h>
> +#include <rte_cpuflags.h>
> +#include <rte_interrupts.h>
> +#include <rte_pci.h>
> +#include <rte_dev.h>
> +#include <rte_devargs.h>
> +#include <rte_common.h>
> +#include <rte_version.h>
> +#include <rte_atomic.h>
> +#include <malloc_heap.h>
> +#include <rte_eth_ring.h>
> +
> +#include "eal_private.h"
> +#include "eal_thread.h"
> +#include "eal_internal_cfg.h"
> +#include "eal_filesystem.h"
> +#include "eal_hugepages.h"
> +#include "eal_options.h"
> +
> +/* Allow the application to print its usage message too if set */
> +rte_usage_hook_t rte_application_usage_hook = NULL;
> +
> +/* define fd variable here, because file needs to be kept open for the
> + * duration of the program, as we hold a write lock on it in the primary proc */
> +int mem_cfg_fd = -1;
> +
> +/* early configuration structure, when memory config is not mmapped */
> +static struct rte_mem_config early_mem_config;
> +
> +/* Address of global and public configuration */
> +struct rte_config rte_config = {
> +		.mem_config = &early_mem_config,
> +};
> +
> +static struct flock wr_lock = {
> +		.l_type = F_WRLCK,
> +		.l_whence = SEEK_SET,
> +		.l_start = offsetof(struct rte_mem_config, memseg),
> +		.l_len = sizeof(early_mem_config.memseg),
> +};
> +
> +/* Return a pointer to the configuration structure */
> +struct rte_config *
> +rte_eal_get_configuration(void)
> +{
> +	return &rte_config;
> +}
> +
> +/* parse a sysfs (or other) file containing one integer value */
> +int
> +eal_parse_sysfs_value(const char *filename, unsigned long *val)
> +{
> +	FILE *f;
> +	char buf[BUFSIZ];
> +	char *end = NULL;
> +
> +	f = fopen(filename, "r");
> +	if (f == NULL) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
> +			__func__, filename);
> +		return -1;
> +	}
> +
> +	if (fgets(buf, sizeof(buf), f) == NULL) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
> +			__func__, filename);
> +		fclose(f);
> +		return -1;
> +	}
> +	*val = strtoul(buf, &end, 0);
> +	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
> +		RTE_LOG(ERR, EAL, "%s(): cannot parse sysfs value %s\n",
> +				__func__, filename);
> +		fclose(f);
> +		return -1;
> +	}
> +	fclose(f);
> +	return 0;
> +}
> +
> +
> +/* create memory configuration in shared/mmap memory. Take out
> + * a write lock on the memsegs, so we can auto-detect primary/secondary.
> + * This means we never close the file while running (auto-close on exit).
> + * We also don't lock the whole file, so that in future we can use read-locks
> + * on other parts, e.g. memzones, to detect if there are running secondary
> + * processes. */
> +static void
> +rte_eal_config_create(void)
> +{
> +	void *rte_mem_cfg_addr;
> +	int retval;
> +
> +	const char *pathname = eal_runtime_config_path();
> +
> +	if (internal_config.no_shconf)
> +		return;
> +
> +	rte_mem_cfg_addr = rte_eal_get_mem_cfg_addr();
> +
> +	if (mem_cfg_fd < 0) {
> +		mem_cfg_fd = open(pathname, O_RDWR | O_CREAT, 0660);
> +		if (mem_cfg_fd < 0)
> +			rte_panic("Cannot open '%s' for rte_mem_config\n",
> +					pathname);
> +	}
> +
> +	retval = ftruncate(mem_cfg_fd, sizeof(*rte_config.mem_config));
> +	if (retval < 0) {
> +		close(mem_cfg_fd);
> +		rte_panic("Cannot resize '%s' for rte_mem_config\n", pathname);
> +	}
> +
> +	retval = fcntl(mem_cfg_fd, F_SETLK, &wr_lock);
> +	if (retval < 0) {
> +		close(mem_cfg_fd);
> +		rte_exit(EXIT_FAILURE, "Cannot create lock on '%s'. "
> +			"Is another primary process running?\n", pathname);
> +	}
> +
> +	rte_mem_cfg_addr = mmap(rte_mem_cfg_addr,
> +			sizeof(*rte_config.mem_config), PROT_READ | PROT_WRITE,
> +			MAP_SHARED, mem_cfg_fd, 0);
> +
> +	if (rte_mem_cfg_addr == MAP_FAILED)
> +		rte_panic("Cannot mmap memory for rte_config\n");
> +
> +	memcpy(rte_mem_cfg_addr, &early_mem_config, sizeof(early_mem_config));
> +	rte_config.mem_config = (struct rte_mem_config *) rte_mem_cfg_addr;
> +
> +#ifndef RTE_EXEC_ENV_BSDAPP
> +	/* store address of the config in the config itself so that secondary
> +	 * processes could later map the config into this exact location
> +	 */
> +	rte_config.mem_config->mem_cfg_addr = (uintptr_t) rte_mem_cfg_addr;
> +#endif /* RTE_EXEC_ENV_BSDAPP */
Why is this a BSD/Linux Differentiator?
Neil