[1/7] eal: move OS common functions to single file

Message ID 20200422072747.15960-2-talshn@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Windows bus/pci support |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail apply issues

Commit Message

Tal Shnaiderman April 22, 2020, 7:27 a.m. UTC
  From: Tal Shnaiderman <talshn@mellanox.com>

Move common functions between Unix and Windows to eal_config.c.
Those simple functions are getter functions for IOVA, configuration, Multi-process.

Move rte_config and runtime_dir to be defined in a common file.

Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
---
 lib/librte_eal/common/eal_config.c  | 34 ++++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_private.h | 11 +++++++++++
 lib/librte_eal/common/meson.build   |  2 ++
 lib/librte_eal/freebsd/eal.c        | 34 ----------------------------------
 lib/librte_eal/linux/eal.c          | 33 ---------------------------------
 lib/librte_eal/windows/eal.c        | 36 ------------------------------------
 6 files changed, 47 insertions(+), 103 deletions(-)
 create mode 100644 lib/librte_eal/common/eal_config.c
  

Comments

Menon, Ranjit April 22, 2020, 11:51 p.m. UTC | #1
On 4/22/2020 12:27 AM, talshn@mellanox.com wrote:
> From: Tal Shnaiderman <talshn@mellanox.com>
> 
> Move common functions between Unix and Windows to eal_config.c.

Like other files in common, we should call this eal_common_config.c

> Those simple functions are getter functions for IOVA, configuration, Multi-process.
> 
> Move rte_config and runtime_dir to be defined in a common file.
> 
> Signed-off-by: Tal Shnaiderman <talshn@mellanox.com>
> ---
>   lib/librte_eal/common/eal_config.c  | 34 ++++++++++++++++++++++++++++++++++
>   lib/librte_eal/common/eal_private.h | 11 +++++++++++
>   lib/librte_eal/common/meson.build   |  2 ++
>   lib/librte_eal/freebsd/eal.c        | 34 ----------------------------------
>   lib/librte_eal/linux/eal.c          | 33 ---------------------------------
>   lib/librte_eal/windows/eal.c        | 36 ------------------------------------
>   6 files changed, 47 insertions(+), 103 deletions(-)
>   create mode 100644 lib/librte_eal/common/eal_config.c
> 
> diff --git a/lib/librte_eal/common/eal_config.c b/lib/librte_eal/common/eal_config.c
> new file mode 100644
> index 000000000..c28080a76
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_config.c
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Mellanox Technologies, Ltd
> + */
> +#include <eal_private.h>
> +
> +#include <rte_os.h>
> +
> +/* platform-specific runtime dir */
> +static char runtime_dir[PATH_MAX];
> +
> +const char *
> +rte_eal_get_runtime_dir(void)
> +{
> +	return runtime_dir;
> +}
> +
> +/* Return a pointer to the configuration structure */
> +struct rte_config *
> +rte_eal_get_configuration(void)
> +{
> +	return &rte_config;
> +}
> +
> +enum rte_iova_mode
> +rte_eal_iova_mode(void)
> +{
> +	return rte_eal_get_configuration()->iova_mode;
> +}
> +
> +enum rte_proc_type_t
> +rte_eal_process_type(void)
> +{
> +	return rte_config.process_type;
> +}
> \ No newline at end of file
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 735813d0c..dab9cac1d 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -13,6 +13,8 @@
>   #include <rte_lcore.h>
>   #include <rte_memory.h>
>   
> +#include <eal_memcfg.h>
> +
>   /**
>    * Structure storing internal configuration (per-lcore)
>    */
> @@ -60,6 +62,15 @@ struct rte_config {
>   	struct rte_mem_config *mem_config;
>   } __rte_packed;
>   
> +
> +/* early configuration structure, when memory config is not mmapped */
> +static struct rte_mem_config early_mem_config;
> +
> +/* Address of global and public configuration */
> +static struct rte_config rte_config = {
> +		.mem_config = &early_mem_config,
> +};
> +
>   /**
>    * Get the global configuration structure.
>    *
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
> index 6dcdcc890..f53a35d0e 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -20,6 +20,7 @@ if is_windows
>   		'eal_common_options.c',
>   		'eal_common_tailqs.c',
>   		'eal_common_thread.c',
> +		'eal_config.c',
>   		'malloc_elem.c',
>   		'malloc_heap.c',
>   		'rte_malloc.c',
> @@ -52,6 +53,7 @@ sources += files(
>   	'eal_common_thread.c',
>   	'eal_common_timer.c',
>   	'eal_common_uuid.c',
> +	'eal_common.c',

Typo. But, better to rename to eal_common_config.c

>   	'hotplug_mp.c',
>   	'malloc_elem.c',
>   	'malloc_heap.c',

ranjit m.
  
Thomas Monjalon April 23, 2020, 7:27 a.m. UTC | #2
23/04/2020 01:51, Ranjit Menon:
> On 4/22/2020 12:27 AM, talshn@mellanox.com wrote:
> > From: Tal Shnaiderman <talshn@mellanox.com>
> > 
> > Move common functions between Unix and Windows to eal_config.c.
> 
> Like other files in common, we should call this eal_common_config.c

I am not sure about the interest of repeating the directory name
in the file name in general.
Do you see a real benefit?

Note: the naming in lib/librte_eal/common/ is not uniform.
  
Dmitry Kozlyuk April 23, 2020, 9:06 a.m. UTC | #3
On 2020-04-23 09:27 GMT+0200 Thomas Monjalon wrote:
> 23/04/2020 01:51, Ranjit Menon:
> > On 4/22/2020 12:27 AM, talshn@mellanox.com wrote:  
> > > From: Tal Shnaiderman <talshn@mellanox.com>
> > > 
> > > Move common functions between Unix and Windows to eal_config.c.  
> > 
> > Like other files in common, we should call this eal_common_config.c  
> 
> I am not sure about the interest of repeating the directory name
> in the file name in general.
> Do you see a real benefit?

It allows using VPATH in Makefile. If filenames are identical in different
VPATH directories, make can't pick both. Makefiles are being deprecated, but
they'll be around for some more time.
  
Thomas Monjalon April 23, 2020, 10:48 a.m. UTC | #4
23/04/2020 11:06, Dmitry Kozlyuk:
> On 2020-04-23 09:27 GMT+0200 Thomas Monjalon wrote:
> > 23/04/2020 01:51, Ranjit Menon:
> > > On 4/22/2020 12:27 AM, talshn@mellanox.com wrote:  
> > > > From: Tal Shnaiderman <talshn@mellanox.com>
> > > > 
> > > > Move common functions between Unix and Windows to eal_config.c.  
> > > 
> > > Like other files in common, we should call this eal_common_config.c  
> > 
> > I am not sure about the interest of repeating the directory name
> > in the file name in general.
> > Do you see a real benefit?
> 
> It allows using VPATH in Makefile. If filenames are identical in different
> VPATH directories, make can't pick both. Makefiles are being deprecated, but
> they'll be around for some more time.

Makefile will be removed in 20.11
  
Menon, Ranjit April 23, 2020, 4:31 p.m. UTC | #5
On 4/23/2020 3:48 AM, Thomas Monjalon wrote:
> 23/04/2020 11:06, Dmitry Kozlyuk:
>> On 2020-04-23 09:27 GMT+0200 Thomas Monjalon wrote:
>>> 23/04/2020 01:51, Ranjit Menon:
>>>> On 4/22/2020 12:27 AM, talshn@mellanox.com wrote:
>>>>> From: Tal Shnaiderman <talshn@mellanox.com>
>>>>>
>>>>> Move common functions between Unix and Windows to eal_config.c.
>>>>
>>>> Like other files in common, we should call this eal_common_config.c
>>>
>>> I am not sure about the interest of repeating the directory name
>>> in the file name in general.
>>> Do you see a real benefit?

In general, no. But in this case, it does make a difference, IMO.
When seeing all the files in EAL together, it clearly stands out that 
these are common/shared files and any change therein will affect others 
beyond Windows. IMO, I prefer it the way it is.

>>
>> It allows using VPATH in Makefile. If filenames are identical in different
>> VPATH directories, make can't pick both. Makefiles are being deprecated, but
>> they'll be around for some more time.
> 
> Makefile will be removed in 20.11
> 
> 


ranjit m.
  

Patch

diff --git a/lib/librte_eal/common/eal_config.c b/lib/librte_eal/common/eal_config.c
new file mode 100644
index 000000000..c28080a76
--- /dev/null
+++ b/lib/librte_eal/common/eal_config.c
@@ -0,0 +1,34 @@ 
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2020 Mellanox Technologies, Ltd
+ */
+#include <eal_private.h>
+
+#include <rte_os.h>
+
+/* platform-specific runtime dir */
+static char runtime_dir[PATH_MAX];
+
+const char *
+rte_eal_get_runtime_dir(void)
+{
+	return runtime_dir;
+}
+
+/* Return a pointer to the configuration structure */
+struct rte_config *
+rte_eal_get_configuration(void)
+{
+	return &rte_config;
+}
+
+enum rte_iova_mode
+rte_eal_iova_mode(void)
+{
+	return rte_eal_get_configuration()->iova_mode;
+}
+
+enum rte_proc_type_t
+rte_eal_process_type(void)
+{
+	return rte_config.process_type;
+}
\ No newline at end of file
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 735813d0c..dab9cac1d 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -13,6 +13,8 @@ 
 #include <rte_lcore.h>
 #include <rte_memory.h>
 
+#include <eal_memcfg.h>
+
 /**
  * Structure storing internal configuration (per-lcore)
  */
@@ -60,6 +62,15 @@  struct rte_config {
 	struct rte_mem_config *mem_config;
 } __rte_packed;
 
+
+/* early configuration structure, when memory config is not mmapped */
+static struct rte_mem_config early_mem_config;
+
+/* Address of global and public configuration */
+static struct rte_config rte_config = {
+		.mem_config = &early_mem_config,
+};
+
 /**
  * Get the global configuration structure.
  *
diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
index 6dcdcc890..f53a35d0e 100644
--- a/lib/librte_eal/common/meson.build
+++ b/lib/librte_eal/common/meson.build
@@ -20,6 +20,7 @@  if is_windows
 		'eal_common_options.c',
 		'eal_common_tailqs.c',
 		'eal_common_thread.c',
+		'eal_config.c',
 		'malloc_elem.c',
 		'malloc_heap.c',
 		'rte_malloc.c',
@@ -52,6 +53,7 @@  sources += files(
 	'eal_common_thread.c',
 	'eal_common_timer.c',
 	'eal_common_uuid.c',
+	'eal_common.c',
 	'hotplug_mp.c',
 	'malloc_elem.c',
 	'malloc_heap.c',
diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
index 80dc9aa78..a0416a48b 100644
--- a/lib/librte_eal/freebsd/eal.c
+++ b/lib/librte_eal/freebsd/eal.c
@@ -71,11 +71,6 @@  static struct flock wr_lock = {
 		.l_len = sizeof(early_mem_config.memsegs),
 };
 
-/* 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];
 
@@ -85,9 +80,6 @@  struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-/* platform-specific runtime dir */
-static char runtime_dir[PATH_MAX];
-
 static const char *default_runtime_dir = "/var/run";
 
 int
@@ -150,13 +142,6 @@  eal_clean_runtime_dir(void)
 	return 0;
 }
 
-
-const char *
-rte_eal_get_runtime_dir(void)
-{
-	return runtime_dir;
-}
-
 /* Return user provided mbuf pool ops name */
 const char *
 rte_eal_mbuf_user_pool_ops(void)
@@ -164,19 +149,6 @@  rte_eal_mbuf_user_pool_ops(void)
 	return internal_config.user_mbuf_pool_ops_name;
 }
 
-/* Return a pointer to the configuration structure */
-struct rte_config *
-rte_eal_get_configuration(void)
-{
-	return &rte_config;
-}
-
-enum rte_iova_mode
-rte_eal_iova_mode(void)
-{
-	return rte_eal_get_configuration()->iova_mode;
-}
-
 /* parse a sysfs (or other) file containing one integer value */
 int
 eal_parse_sysfs_value(const char *filename, unsigned long *val)
@@ -970,12 +942,6 @@  rte_eal_cleanup(void)
 	return 0;
 }
 
-enum rte_proc_type_t
-rte_eal_process_type(void)
-{
-	return rte_config.process_type;
-}
-
 int rte_eal_has_pci(void)
 {
 	return !internal_config.no_pci;
diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
index d1e532fc1..bc09bfcef 100644
--- a/lib/librte_eal/linux/eal.c
+++ b/lib/librte_eal/linux/eal.c
@@ -85,11 +85,6 @@  static struct flock wr_lock = {
 		.l_len = sizeof(early_mem_config.memsegs),
 };
 
-/* 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];
 
@@ -99,9 +94,6 @@  struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
-/* platform-specific runtime dir */
-static char runtime_dir[PATH_MAX];
-
 static const char *default_runtime_dir = "/var/run";
 
 int
@@ -240,12 +232,6 @@  eal_clean_runtime_dir(void)
 	return -1;
 }
 
-const char *
-rte_eal_get_runtime_dir(void)
-{
-	return runtime_dir;
-}
-
 /* Return user provided mbuf pool ops name */
 const char *
 rte_eal_mbuf_user_pool_ops(void)
@@ -253,19 +239,6 @@  rte_eal_mbuf_user_pool_ops(void)
 	return internal_config.user_mbuf_pool_ops_name;
 }
 
-/* Return a pointer to the configuration structure */
-struct rte_config *
-rte_eal_get_configuration(void)
-{
-	return &rte_config;
-}
-
-enum rte_iova_mode
-rte_eal_iova_mode(void)
-{
-	return rte_eal_get_configuration()->iova_mode;
-}
-
 /* parse a sysfs (or other) file containing one integer value */
 int
 eal_parse_sysfs_value(const char *filename, unsigned long *val)
@@ -1331,12 +1304,6 @@  rte_eal_cleanup(void)
 	return 0;
 }
 
-enum rte_proc_type_t
-rte_eal_process_type(void)
-{
-	return rte_config.process_type;
-}
-
 int rte_eal_has_hugepages(void)
 {
 	return ! internal_config.no_hugetlbfs;
diff --git a/lib/librte_eal/windows/eal.c b/lib/librte_eal/windows/eal.c
index 38f17f09c..57520d51c 100644
--- a/lib/librte_eal/windows/eal.c
+++ b/lib/librte_eal/windows/eal.c
@@ -31,36 +31,12 @@  static rte_usage_hook_t	rte_application_usage_hook;
  */
 static 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 */
-static struct rte_config rte_config = {
-		.mem_config = &early_mem_config,
-};
-
 /* internal configuration (per-core) */
 struct lcore_config lcore_config[RTE_MAX_LCORE];
 
 /* internal configuration */
 struct internal_config internal_config;
 
-/* platform-specific runtime dir */
-static char runtime_dir[PATH_MAX];
-
-const char *
-rte_eal_get_runtime_dir(void)
-{
-	return runtime_dir;
-}
-
-/* Return a pointer to the configuration structure */
-struct rte_config *
-rte_eal_get_configuration(void)
-{
-	return &rte_config;
-}
-
 /* Detect if we are a primary or a secondary process */
 enum rte_proc_type_t
 eal_proc_type_detect(void)
@@ -93,24 +69,12 @@  eal_proc_type_detect(void)
 	return ptype;
 }
 
-enum rte_proc_type_t
-rte_eal_process_type(void)
-{
-	return rte_config.process_type;
-}
-
 int
 rte_eal_has_hugepages(void)
 {
 	return !internal_config.no_hugetlbfs;
 }
 
-enum rte_iova_mode
-rte_eal_iova_mode(void)
-{
-	return rte_config.iova_mode;
-}
-
 /* display usage */
 static void
 eal_usage(const char *prgname)