Re: [PATCH v4] eal: make hugetlb initialization more robust

[David Marchand <david.marchand@6wind.com>]

Hello Sergio,

On Wed, May 18, 2016 at 9:56 AM, Sergio Gonzalez Monroy
<sergio.gonzalez.monroy@intel.com> wrote:
> On 17/05/2016 17:39, David Marchand wrote:
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> index 5b9132c..8c77010 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_memory.c
>>> @@ -417,12 +434,33 @@ map_all_hugepages(struct hugepage_file *hugepg_tbl,
>>>                          hugepg_tbl[i].final_va = virtaddr;
>>>                  }
>>>
>>> +               if (orig && internal_config.huge_trybest) {
>>> +                       /* In linux, hugetlb limitations, like cgroup,
>>> are
>>> +                        * enforced at fault time instead of mmap(), even
>>> +                        * with the option of MAP_POPULATE. Kernel will
>>> send
>>> +                        * a SIGBUS signal. To avoid to be killed, save
>>> stack
>>> +                        * environment here, if SIGBUS happens, we can
>>> jump
>>> +                        * back here.
>>> +                        */
>>> +                       if (wrap_sigsetjmp()) {
>>> +                               RTE_LOG(DEBUG, EAL, "SIGBUS: Cannot mmap
>>> more "
>>> +                                       "hugepages of size %u MB\n",
>>> +                                       (unsigned)(hugepage_sz /
>>> 0x100000));
>>> +                               munmap(virtaddr, hugepage_sz);
>>> +                               close(fd);
>>> +                               unlink(hugepg_tbl[i].filepath);
>>> +                               return i;
>>> +                       }
>>> +                       *(int *)virtaddr = 0;
>>> +               }
>>> +
>>> +
>>>                  /* set shared flock on the file. */
>>>                  if (flock(fd, LOCK_SH | LOCK_NB) == -1) {
>>> -                       RTE_LOG(ERR, EAL, "%s(): Locking file failed:%s
>>> \n",
>>> +                       RTE_LOG(DEBUG, EAL, "%s(): Locking file failed:%s
>>> \n",
>>>                                  __func__, strerror(errno));
>>>                          close(fd);
>>> -                       return -1;
>>> +                       return i;
>>>                  }
>>>
>>>                  close(fd);
>>
>> Maybe I missed something, but we are writing into some hugepage before
>> the flock has been called.
>> Are we sure there is nobody else using this hugepage ?
>>
>> Especially, can't this cause trouble to a primary process running if
>> we start the exact same primary process ?
>>
>
> We lock the hugepage directory during eal_hugepage_info_init(), and we do
> not unlock
> until we have finished eal_memory_init.
>
> I think that takes care of that case.

Yes, thanks.

-- 
David Marchand