On 04-Jul-19 11:54 AM, David Marchand wrote:
On Thu, Jul 4, 2019 at 12:50 PM Burakov, Anatoly <[email protected] <mailto:[email protected]>> wrote:On 04-Jul-19 8:56 AM, David Marchand wrote: > On Thu, Jul 4, 2019 at 9:50 AM David Marchand <[email protected] <mailto:[email protected]>> > wrote: > >> >> >> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov <[email protected] <mailto:[email protected]>> >> 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 <[email protected] <mailto:[email protected]>> >>> --- >>> 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. >> > > Replying to myself... at least those helpers try to do something > explicitly: synchronising the local copy of the configuration with the > shared mem config. > Keeping them separate documents this step. > > Anyway, your choice, but you must fix freebsd :-) > Sorry, i don't follow - what needs to be fixed in FreeBSD? eal_mcfg_update_from_internal() must be called in primary process. eal_mcfg_update_internal() must be called in secondary processes.
Oh, right, good catch, thanks!
-- David Marchand
-- Thanks, Anatoly

