[dpdk-dev] [PATCH v7 10/11] eal: replace rte_panic instances in init sequence
Arnon Warshavsky
arnon at qwilt.com
Wed Apr 25 11:33:52 CEST 2018
<...>
>
> + if (rte_config_init() != 0) {
>> + rte_eal_init_alert("Failed to init configuration");
>> + rte_errno = EFAULT;
>> + return -1;
>> + }
>> +
>> + if (rte_mp_channel_init() < 0) {
>> + rte_eal_init_alert("failed to init mp channel\n");
>> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
>> + rte_errno = EFAULT;
>> + return -1;
>> + }
>> + }
>>
>
> ^^^ this change looks unintended. Rebase artifact?
>
> +
>> /* in secondary processes, memory init may allocate additional
>> fbarrays
>> * not present in primary processes, so to avoid any potential
>> issues,
>> * initialize memzones first.
>> @@ -671,6 +712,7 @@ static void rte_eal_init_alert(const char *msg)
>> */
>> if (pipe(lcore_config[i].pipe_master2slave) < 0)
>> rte_panic("Cannot create pipe\n");
>> +
>> if (pipe(lcore_config[i].pipe_slave2master) < 0)
>> rte_panic("Cannot create pipe\n");
>>
>
> ^^^ this looks unintended as well.
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>> b/lib/librte_eal/linuxapp/eal/eal.c
>> index 5b23bf0..54adaec 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -160,7 +160,7 @@ enum rte_iova_mode
>> * 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
>> +static int
>> rte_eal_config_create(void)
>> {
>> void *rte_mem_cfg_addr;
>> @@ -169,7 +169,7 @@ enum rte_iova_mode
>> const char *pathname = eal_runtime_config_path();
>> if (internal_config.no_shconf)
>> - return;
>>
>
> <...>
>
> }
>> 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");
>> + if (rte_mem_cfg_addr == MAP_FAILED) {
>> + RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for
>> rte_config\n",
>> + __func__);
>> + return -1;
>> }
>>
>
> I think you forgot to close mem_cfg_fd and set it to -1 in case of error
> here.
>
> memcpy(rte_mem_cfg_addr, &early_mem_config,
>> sizeof(early_mem_config));
>> rte_config.mem_config = rte_mem_cfg_addr;
>> @@ -211,10 +221,11 @@ enum rte_iova_mode
>> * processes could later map the config into this exact location
>> */
>> rte_config.mem_config->mem_cfg_addr = (uintptr_t)
>> rte_mem_cfg_addr;
>> + return 0;
>> }
>>
>>
>
> <...>
>
> /* map it as read-only first */
>> mem_config = (struct rte_mem_config *) mmap(NULL,
>> sizeof(*mem_config),
>> PROT_READ, MAP_SHARED, mem_cfg_fd, 0);
>> - if (mem_config == MAP_FAILED)
>> - rte_panic("Cannot mmap memory for rte_config! error %i
>> (%s)\n",
>> - errno, strerror(errno));
>> + if (mem_config == MAP_FAILED) {
>> + mem_cfg_fd = -1;
>>
>
> Forgot close() here, i think.
>
> + RTE_LOG(CRIT, EAL, "%s(): Cannot mmap memory for
>> rte_config! error %i (%s)\n",
>> + __func__, errno, strerror(errno));
>> + return -1;
>> + }
>> rte_config.mem_config = mem_config;
>> +
>> + return 0;
>> }
>> /* reattach the shared config at exact memory location primary
>> process has it */
>>
>
> <...>
>
> + if (rte_config_init() != 0)
>> + return -1;
>> +
>> if (rte_eal_log_init(logid, internal_config.syslog_facility) <
>> 0) {
>> rte_eal_init_alert("Cannot init logging.");
>> rte_errno = ENOMEM;
>> @@ -914,6 +946,7 @@ static void rte_eal_init_alert(const char *msg)
>> */
>> if (pipe(lcore_config[i].pipe_master2slave) < 0)
>> rte_panic("Cannot create pipe\n");
>> +
>> if (pipe(lcore_config[i].pipe_slave2master) < 0)
>>
>
> Again, looks like unintended whitespace change.
>
> rte_panic("Cannot create pipe\n");
>>
>>
>
> Thanks Anatoly
More information about the dev
mailing list