On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov <anatoly.bura...@intel.com>
wrote:
> Currently, each EAL will update internal/shared config in their
> own way at init, resulting in needless duplication of code and
> OS-dependent behavior. Move the functions to a common file and
> add missing FreeBSD steps.
>
> Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
> ---
> lib/librte_eal/common/eal_common_mcfg.c | 18 ++++++++++++++++++
> lib/librte_eal/common/eal_memcfg.h | 8 ++++++++
> lib/librte_eal/freebsd/eal/eal.c | 2 ++
> lib/librte_eal/linux/eal/eal.c | 22 ++--------------------
> 4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_common_mcfg.c
> b/lib/librte_eal/common/eal_common_mcfg.c
> index dc6665d6a..fe8d2b726 100644
> --- a/lib/librte_eal/common/eal_common_mcfg.c
> +++ b/lib/librte_eal/common/eal_common_mcfg.c
> @@ -31,6 +31,24 @@ eal_mcfg_wait_complete(void)
> rte_pause();
> }
>
> +void
> +eal_mcfg_update_internal(void)
> +{
> + struct rte_mem_config *mcfg =
> rte_eal_get_configuration()->mem_config;
> +
> + internal_config.legacy_mem = mcfg->legacy_mem;
> + internal_config.single_file_segments = mcfg->single_file_segments;
> +}
> +
> +void
> +eal_mcfg_update_from_internal(void)
> +{
> + struct rte_mem_config *mcfg =
> rte_eal_get_configuration()->mem_config;
> +
> + mcfg->legacy_mem = internal_config.legacy_mem;
> + mcfg->single_file_segments = internal_config.single_file_segments;
> +}
> +
> void
> rte_mcfg_mem_read_lock(void)
> {
> diff --git a/lib/librte_eal/common/eal_memcfg.h
> b/lib/librte_eal/common/eal_memcfg.h
> index a2434417e..d02ac1621 100644
> --- a/lib/librte_eal/common/eal_memcfg.h
> +++ b/lib/librte_eal/common/eal_memcfg.h
> @@ -68,6 +68,14 @@ struct rte_mem_config {
> uint8_t dma_maskbits;
> };
>
> +/* update internal config from shared mem config */
> +void
> +eal_mcfg_update_internal(void);
> +
> +/* update shared mem config from internal config */
> +void
> +eal_mcfg_update_from_internal(void);
> +
> /* wait until primary process initialization is complete */
> void
> eal_mcfg_wait_complete(void);
> diff --git a/lib/librte_eal/freebsd/eal/eal.c
> b/lib/librte_eal/freebsd/eal/eal.c
> index 13e230fc8..6bfe203fd 100644
> --- a/lib/librte_eal/freebsd/eal/eal.c
> +++ b/lib/librte_eal/freebsd/eal/eal.c
> @@ -379,6 +379,7 @@ rte_config_init(void)
> case RTE_PROC_PRIMARY:
> if (rte_eal_config_create() < 0)
> return -1;
> + eal_mcfg_update_internal();
> break;
> case RTE_PROC_SECONDARY:
> if (rte_eal_config_attach() < 0)
> @@ -386,6 +387,7 @@ rte_config_init(void)
> eal_mcfg_wait_complete();
> if (rte_eal_config_reattach() < 0)
> return -1;
> + eal_mcfg_update_from_internal();
> break;
> case RTE_PROC_AUTO:
> case RTE_PROC_INVALID:
>
Hum, you swapped eal_mcfg_update_internal and eal_mcfg_update_from_internal.
The names are a bit ambiguous, and I wonder if we really need those
separate helpers.
rte_eal_config_create and rte_eal_config_attach are already awfully close
in linux and freebsd implementation.
Can't we have them as common code and put those helpers bits direct in them
?
> diff --git a/lib/librte_eal/linux/eal/eal.c
> b/lib/librte_eal/linux/eal/eal.c
> index 4fd18b15f..fa205fd29 100644
> --- a/lib/librte_eal/linux/eal/eal.c
> +++ b/lib/librte_eal/linux/eal/eal.c
> @@ -472,24 +472,6 @@ eal_proc_type_detect(void)
> return ptype;
> }
>
> -/* copies data from internal config to shared config */
> -static void
> -eal_update_mem_config(void)
> -{
> - struct rte_mem_config *mcfg =
> rte_eal_get_configuration()->mem_config;
> - mcfg->legacy_mem = internal_config.legacy_mem;
> - mcfg->single_file_segments = internal_config.single_file_segments;
> -}
> -
> -/* copies data from shared config to internal config */
> -static void
> -eal_update_internal_config(void)
> -{
> - struct rte_mem_config *mcfg =
> rte_eal_get_configuration()->mem_config;
> - internal_config.legacy_mem = mcfg->legacy_mem;
> - internal_config.single_file_segments = mcfg->single_file_segments;
> -}
> -
> /* Sets up rte_config structure with the pointer to shared memory
> config.*/
> static int
> rte_config_init(void)
> @@ -500,7 +482,7 @@ rte_config_init(void)
> case RTE_PROC_PRIMARY:
> if (rte_eal_config_create() < 0)
> return -1;
> - eal_update_mem_config();
> + eal_mcfg_update_from_internal();
> break;
> case RTE_PROC_SECONDARY:
> if (rte_eal_config_attach() < 0)
> @@ -508,7 +490,7 @@ rte_config_init(void)
> eal_mcfg_wait_complete();
> if (rte_eal_config_reattach() < 0)
> return -1;
> - eal_update_internal_config();
> + eal_mcfg_update_internal();
> break;
> case RTE_PROC_AUTO:
> case RTE_PROC_INVALID:
> --
> 2.17.1
>
--
David Marchand
