On 04-Jul-19 11:54 AM, David Marchand wrote:
On Thu, Jul 4, 2019 at 12:50 PM Burakov, Anatoly
<anatoly.bura...@intel.com <mailto:anatoly.bura...@intel.com>> wrote:
On 04-Jul-19 8:56 AM, David Marchand wrote:
> On Thu, Jul 4, 2019 at 9:50 AM David Marchand
<david.march...@redhat.com <mailto:david.march...@redhat.com>>
> wrote:
>
>>
>>
>> On Thu, Jun 27, 2019 at 1:39 PM Anatoly Burakov
<anatoly.bura...@intel.com <mailto: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
<mailto: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.
>>
>
> 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