On Mon, Jan 6, 2020 at 9:56 PM Stephen Hemminger
<step...@networkplumber.org> wrote:
>
> The loop to unwind existing mmaps was only unmapping the
> first segment.
>
> Also, remove obvious redundant assignment.
>
> Fixes: 66cc45e293ed ("mem: replace memseg with memseg lists")
> Cc: anatoly.bura...@intel.com
> Cc: sta...@dpdk.org
> Signed-off-by: Stephen Hemminger <step...@networkplumber.org>
> ---
> lib/librte_eal/linux/eal/eal_memory.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/lib/librte_eal/linux/eal/eal_memory.c
> b/lib/librte_eal/linux/eal/eal_memory.c
> index 43e4ffc757bd..cf5b2433614b 100644
> --- a/lib/librte_eal/linux/eal/eal_memory.c
> +++ b/lib/librte_eal/linux/eal/eal_memory.c
> @@ -1967,9 +1967,8 @@ eal_legacy_hugepage_attach(void)
> close(fd);
> error:
> /* map all segments into memory to make sure we get the addrs */
This looks like a copy of the same loop from a few lines before in the
same function.
The comment can be removed.
> - cur_seg = 0;
> for (cur_seg = 0; cur_seg < i; cur_seg++) {
> - struct hugepage_file *hf = &hp[i];
> + struct hugepage_file *hf = &hp[cur_seg];
> size_t map_sz = hf->size;
> void *map_addr = hf->final_va;
>
map_addr and map_sz are unnecessary.
We should be safe with dereferencing hp if i != 0, but still how about:
error:
if (hp != NULL && hp != MAP_FAILED) {
unsigned int cur_seg;
for (cur_seg = 0; cur_seg < i; cur_seg++)
munmap(hp[cur_seg].final_va, hp[cur_seg].size);
munmap(hp, size);
}
if (fd_hugepage >= 0)
close(fd_hugepage);
return -1;
--
David Marchand
