* [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode @ 2023-05-22 12:41 Fengnan Chang 2023-05-22 13:28 ` Burakov, Anatoly 0 siblings, 1 reply; 4+ messages in thread From: Fengnan Chang @ 2023-05-22 12:41 UTC (permalink / raw) To: anatoly.burakov, dev; +Cc: Fengnan Chang, Lin Li Under legacy mode, if the number of continuous memsegs greater than RTE_MAX_MEMSEG_PER_LIST, eal init will failed even though another memseg list is empty, because only one memseg list used to check in remap_needed_hugepages. Fix this by add a argment indicate how many pages mapped in remap_segment, remap_segment try to mapped most pages it can, if exceed it's capbility, remap_needed_hugepages will continue to map other left pages. For example: hugepage configure: cat /sys/devices/system/node/node*/hugepages/hugepages-2048kB/nr_hugepages 10241 10239 startup log: EAL: Detected memory type: socket_id:0 hugepage_sz:2097152 EAL: Detected memory type: socket_id:1 hugepage_sz:2097152 EAL: Creating 4 segment lists: n_segs:8192 socket_id:0 hugepage_sz:2097152 EAL: Creating 4 segment lists: n_segs:8192 socket_id:1 hugepage_sz:2097152 EAL: Requesting 13370 pages of size 2MB from socket 0 EAL: Requesting 7110 pages of size 2MB from socket 1 EAL: Attempting to map 14220M on socket 1 EAL: Allocated 14220M on socket 1 EAL: Attempting to map 26740M on socket 0 EAL: Could not find space for memseg. Please increase 32768 and/or 65536 in configuration. EAL: Couldn't remap hugepage files into memseg lists EAL: FATAL: Cannot init memory EAL: Cannot init memory Signed-off-by: Fengnan Chang <changfengnan@bytedance.com> Signed-off-by: Lin Li <lilintjpu@bytedance.com> --- lib/eal/linux/eal_memory.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c index 60fc8cc6ca..b2e6453fbe 100644 --- a/lib/eal/linux/eal_memory.c +++ b/lib/eal/linux/eal_memory.c @@ -657,12 +657,12 @@ unmap_unneeded_hugepages(struct hugepage_file *hugepg_tbl, } static int -remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) +remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end, int *mapped_seg_len) { struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; struct rte_memseg_list *msl; struct rte_fbarray *arr; - int cur_page, seg_len; + int cur_page, seg_len, cur_len; unsigned int msl_idx; int ms_idx; uint64_t page_sz; @@ -692,8 +692,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) /* leave space for a hole if array is not empty */ empty = arr->count == 0; + cur_len = RTE_MIN((unsigned int)seg_len, arr->len - arr->count - (empty ? 0 : 1)); ms_idx = rte_fbarray_find_next_n_free(arr, 0, - seg_len + (empty ? 0 : 1)); + cur_len + (empty ? 0 : 1)); /* memseg list is full? */ if (ms_idx < 0) @@ -704,12 +705,12 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) */ if (!empty) ms_idx++; + *mapped_seg_len = cur_len; break; } if (msl_idx == RTE_MAX_MEMSEG_LISTS) { - RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n", - RTE_STR(RTE_MAX_MEMSEG_PER_TYPE), - RTE_STR(RTE_MAX_MEM_MB_PER_TYPE)); + RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST " + "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n"); return -1; } @@ -725,6 +726,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) void *addr; int fd; + if (cur_page - seg_start == *mapped_seg_len) + break; fd = open(hfile->filepath, O_RDWR); if (fd < 0) { RTE_LOG(ERR, EAL, "Could not open '%s': %s\n", @@ -986,7 +989,7 @@ prealloc_segments(struct hugepage_file *hugepages, int n_pages) static int remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) { - int cur_page, seg_start_page, new_memseg, ret; + int cur_page, seg_start_page, new_memseg, ret, mapped_seg_len = 0; seg_start_page = 0; for (cur_page = 0; cur_page < n_pages; cur_page++) { @@ -1023,21 +1026,27 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) /* if this isn't the first time, remap segment */ if (cur_page != 0) { ret = remap_segment(hugepages, seg_start_page, - cur_page); + cur_page, &mapped_seg_len); if (ret != 0) return -1; } + cur_page = seg_start_page + mapped_seg_len; /* remember where we started */ seg_start_page = cur_page; + mapped_seg_len = 0; } /* continuation of previous memseg */ } /* we were stopped, but we didn't remap the last segment, do it now */ if (cur_page != 0) { - ret = remap_segment(hugepages, seg_start_page, - cur_page); - if (ret != 0) - return -1; + while (seg_start_page < n_pages) { + ret = remap_segment(hugepages, seg_start_page, + cur_page, &mapped_seg_len); + if (ret != 0) + return -1; + seg_start_page = seg_start_page + mapped_seg_len; + mapped_seg_len = 0; + } } return 0; } -- 2.37.1 (Apple Git-137.1) ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode 2023-05-22 12:41 [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode Fengnan Chang @ 2023-05-22 13:28 ` Burakov, Anatoly 2023-05-25 12:49 ` [External] " Fengnan Chang 0 siblings, 1 reply; 4+ messages in thread From: Burakov, Anatoly @ 2023-05-22 13:28 UTC (permalink / raw) To: Fengnan Chang, dev; +Cc: Lin Li On 5/22/2023 1:41 PM, Fengnan Chang wrote: > Under legacy mode, if the number of continuous memsegs greater > than RTE_MAX_MEMSEG_PER_LIST, eal init will failed even though > another memseg list is empty, because only one memseg list used > to check in remap_needed_hugepages. > Fix this by add a argment indicate how many pages mapped in > remap_segment, remap_segment try to mapped most pages it can, if > exceed it's capbility, remap_needed_hugepages will continue to > map other left pages. > > For example: > hugepage configure: > cat /sys/devices/system/node/node*/hugepages/hugepages-2048kB/nr_hugepages > 10241 > 10239 > > startup log: > EAL: Detected memory type: socket_id:0 hugepage_sz:2097152 > EAL: Detected memory type: socket_id:1 hugepage_sz:2097152 > EAL: Creating 4 segment lists: n_segs:8192 socket_id:0 hugepage_sz:2097152 > EAL: Creating 4 segment lists: n_segs:8192 socket_id:1 hugepage_sz:2097152 > EAL: Requesting 13370 pages of size 2MB from socket 0 > EAL: Requesting 7110 pages of size 2MB from socket 1 > EAL: Attempting to map 14220M on socket 1 > EAL: Allocated 14220M on socket 1 > EAL: Attempting to map 26740M on socket 0 > EAL: Could not find space for memseg. Please increase 32768 and/or 65536 in > configuration. > EAL: Couldn't remap hugepage files into memseg lists > EAL: FATAL: Cannot init memory > EAL: Cannot init memory > > Signed-off-by: Fengnan Chang <changfengnan@bytedance.com> > Signed-off-by: Lin Li <lilintjpu@bytedance.com> > --- > lib/eal/linux/eal_memory.c | 33 +++++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c > index 60fc8cc6ca..b2e6453fbe 100644 > --- a/lib/eal/linux/eal_memory.c > +++ b/lib/eal/linux/eal_memory.c > @@ -657,12 +657,12 @@ unmap_unneeded_hugepages(struct hugepage_file *hugepg_tbl, > } > > static int > -remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > +remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end, int *mapped_seg_len) > { > struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; > struct rte_memseg_list *msl; > struct rte_fbarray *arr; > - int cur_page, seg_len; > + int cur_page, seg_len, cur_len; > unsigned int msl_idx; > int ms_idx; > uint64_t page_sz; > @@ -692,8 +692,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > > /* leave space for a hole if array is not empty */ > empty = arr->count == 0; > + cur_len = RTE_MIN((unsigned int)seg_len, arr->len - arr->count - (empty ? 0 : 1)); > ms_idx = rte_fbarray_find_next_n_free(arr, 0, > - seg_len + (empty ? 0 : 1)); > + cur_len + (empty ? 0 : 1)); > > /* memseg list is full? */ > if (ms_idx < 0) > @@ -704,12 +705,12 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > */ > if (!empty) > ms_idx++; > + *mapped_seg_len = cur_len; > break; > } > if (msl_idx == RTE_MAX_MEMSEG_LISTS) { > - RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n", > - RTE_STR(RTE_MAX_MEMSEG_PER_TYPE), > - RTE_STR(RTE_MAX_MEM_MB_PER_TYPE)); > + RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST " > + "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n"); > return -1; > } > > @@ -725,6 +726,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > void *addr; > int fd; > > + if (cur_page - seg_start == *mapped_seg_len) > + break; > fd = open(hfile->filepath, O_RDWR); > if (fd < 0) { > RTE_LOG(ERR, EAL, "Could not open '%s': %s\n", > @@ -986,7 +989,7 @@ prealloc_segments(struct hugepage_file *hugepages, int n_pages) > static int > remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) > { > - int cur_page, seg_start_page, new_memseg, ret; > + int cur_page, seg_start_page, new_memseg, ret, mapped_seg_len = 0; > > seg_start_page = 0; > for (cur_page = 0; cur_page < n_pages; cur_page++) { > @@ -1023,21 +1026,27 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) > /* if this isn't the first time, remap segment */ > if (cur_page != 0) { > ret = remap_segment(hugepages, seg_start_page, > - cur_page); > + cur_page, &mapped_seg_len); > if (ret != 0) > return -1; > } > + cur_page = seg_start_page + mapped_seg_len; > /* remember where we started */ > seg_start_page = cur_page; > + mapped_seg_len = 0; > } > /* continuation of previous memseg */ > } > /* we were stopped, but we didn't remap the last segment, do it now */ > if (cur_page != 0) { > - ret = remap_segment(hugepages, seg_start_page, > - cur_page); > - if (ret != 0) > - return -1; > + while (seg_start_page < n_pages) { > + ret = remap_segment(hugepages, seg_start_page, > + cur_page, &mapped_seg_len); > + if (ret != 0) > + return -1; > + seg_start_page = seg_start_page + mapped_seg_len; > + mapped_seg_len = 0; > + } > } > return 0; > } This works, but I feel like it's overcomplicated - the same logic you're trying to use can just be implemented using `find_biggest_free()` + `find_contig_free()` and returning `seg_len` from `remap_segment()`? Something like the following: --- diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c index 60fc8cc6ca..08acc787fc 100644 --- a/lib/eal/linux/eal_memory.c +++ b/lib/eal/linux/eal_memory.c @@ -681,6 +681,7 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) /* find free space in memseg lists */ for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS; msl_idx++) { + int free_len; bool empty; msl = &mcfg->memsegs[msl_idx]; arr = &msl->memseg_arr; @@ -692,18 +693,27 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) /* leave space for a hole if array is not empty */ empty = arr->count == 0; - ms_idx = rte_fbarray_find_next_n_free(arr, 0, - seg_len + (empty ? 0 : 1)); - /* memseg list is full? */ - if (ms_idx < 0) - continue; + /* find start of the biggest contiguous block and its size */ + ms_idx = rte_fbarray_find_biggest_free(arr, 0); + free_len = rte_fbarray_find_contig_free(arr, ms_idx); /* leave some space between memsegs, they are not IOVA * contiguous, so they shouldn't be VA contiguous either. */ - if (!empty) + if (!empty) { ms_idx++; + free_len--; + } + + /* memseg list is full? */ + if (free_len < 1) + continue; + + /* we might not get all of the space we wanted */ + free_len = RTE_MIN(seg_len, free_len); + seg_end = seg_start + free_len; + seg_len = seg_end - seg_start; break; } if (msl_idx == RTE_MAX_MEMSEG_LISTS) { @@ -787,7 +797,7 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) } RTE_LOG(DEBUG, EAL, "Allocated %" PRIu64 "M on socket %i\n", (seg_len * page_sz) >> 20, socket_id); - return 0; + return seg_len; } static uint64_t @@ -1022,10 +1032,17 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) if (new_memseg) { /* if this isn't the first time, remap segment */ if (cur_page != 0) { - ret = remap_segment(hugepages, seg_start_page, - cur_page); - if (ret != 0) - return -1; + int n_remapped = 0; + int n_needed = cur_page - seg_start_page; + + while (n_remapped < n_needed) { + ret = remap_segment(hugepages, + seg_start_page, + cur_page); + if (ret < 0) + return -1; + n_remapped += ret; + } } /* remember where we started */ seg_start_page = cur_page; @@ -1034,10 +1051,16 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) } /* we were stopped, but we didn't remap the last segment, do it now */ if (cur_page != 0) { - ret = remap_segment(hugepages, seg_start_page, - cur_page); - if (ret != 0) - return -1; + int n_remapped = 0; + int n_needed = cur_page - seg_start_page; + + while (n_remapped < n_needed) { + ret = remap_segment( + hugepages, seg_start_page, cur_page); + if (ret < 0) + return -1; + n_remapped += ret; + } } return 0; } --- This should do the trick? Also, this probably needs to be duplicated for Windows and FreeBSD init as well, because AFAIK they follow the legacy mem init logic. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode 2023-05-22 13:28 ` Burakov, Anatoly @ 2023-05-25 12:49 ` Fengnan Chang 2023-05-25 14:26 ` Burakov, Anatoly 0 siblings, 1 reply; 4+ messages in thread From: Fengnan Chang @ 2023-05-25 12:49 UTC (permalink / raw) To: Burakov, Anatoly; +Cc: dev, Lin Li Burakov, Anatoly <anatoly.burakov@intel.com> 于2023年5月22日周一 21:28写道： > > On 5/22/2023 1:41 PM, Fengnan Chang wrote: > > Under legacy mode, if the number of continuous memsegs greater > > than RTE_MAX_MEMSEG_PER_LIST, eal init will failed even though > > another memseg list is empty, because only one memseg list used > > to check in remap_needed_hugepages. > > Fix this by add a argment indicate how many pages mapped in > > remap_segment, remap_segment try to mapped most pages it can, if > > exceed it's capbility, remap_needed_hugepages will continue to > > map other left pages. > > > > For example: > > hugepage configure: > > cat /sys/devices/system/node/node*/hugepages/hugepages-2048kB/nr_hugepages > > 10241 > > 10239 > > > > startup log: > > EAL: Detected memory type: socket_id:0 hugepage_sz:2097152 > > EAL: Detected memory type: socket_id:1 hugepage_sz:2097152 > > EAL: Creating 4 segment lists: n_segs:8192 socket_id:0 hugepage_sz:2097152 > > EAL: Creating 4 segment lists: n_segs:8192 socket_id:1 hugepage_sz:2097152 > > EAL: Requesting 13370 pages of size 2MB from socket 0 > > EAL: Requesting 7110 pages of size 2MB from socket 1 > > EAL: Attempting to map 14220M on socket 1 > > EAL: Allocated 14220M on socket 1 > > EAL: Attempting to map 26740M on socket 0 > > EAL: Could not find space for memseg. Please increase 32768 and/or 65536 in > > configuration. > > EAL: Couldn't remap hugepage files into memseg lists > > EAL: FATAL: Cannot init memory > > EAL: Cannot init memory > > > > Signed-off-by: Fengnan Chang <changfengnan@bytedance.com> > > Signed-off-by: Lin Li <lilintjpu@bytedance.com> > > --- > > lib/eal/linux/eal_memory.c | 33 +++++++++++++++++++++------------ > > 1 file changed, 21 insertions(+), 12 deletions(-) > > > > diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c > > index 60fc8cc6ca..b2e6453fbe 100644 > > --- a/lib/eal/linux/eal_memory.c > > +++ b/lib/eal/linux/eal_memory.c > > @@ -657,12 +657,12 @@ unmap_unneeded_hugepages(struct hugepage_file *hugepg_tbl, > > } > > > > static int > > -remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > > +remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end, int *mapped_seg_len) > > { > > struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; > > struct rte_memseg_list *msl; > > struct rte_fbarray *arr; > > - int cur_page, seg_len; > > + int cur_page, seg_len, cur_len; > > unsigned int msl_idx; > > int ms_idx; > > uint64_t page_sz; > > @@ -692,8 +692,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > > > > /* leave space for a hole if array is not empty */ > > empty = arr->count == 0; > > + cur_len = RTE_MIN((unsigned int)seg_len, arr->len - arr->count - (empty ? 0 : 1)); > > ms_idx = rte_fbarray_find_next_n_free(arr, 0, > > - seg_len + (empty ? 0 : 1)); > > + cur_len + (empty ? 0 : 1)); > > > > /* memseg list is full? */ > > if (ms_idx < 0) > > @@ -704,12 +705,12 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > > */ > > if (!empty) > > ms_idx++; > > + *mapped_seg_len = cur_len; > > break; > > } > > if (msl_idx == RTE_MAX_MEMSEG_LISTS) { > > - RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n", > > - RTE_STR(RTE_MAX_MEMSEG_PER_TYPE), > > - RTE_STR(RTE_MAX_MEM_MB_PER_TYPE)); > > + RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST " > > + "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n"); > > return -1; > > } > > > > @@ -725,6 +726,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) > > void *addr; > > int fd; > > > > + if (cur_page - seg_start == *mapped_seg_len) > > + break; > > fd = open(hfile->filepath, O_RDWR); > > if (fd < 0) { > > RTE_LOG(ERR, EAL, "Could not open '%s': %s\n", > > @@ -986,7 +989,7 @@ prealloc_segments(struct hugepage_file *hugepages, int n_pages) > > static int > > remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) > > { > > - int cur_page, seg_start_page, new_memseg, ret; > > + int cur_page, seg_start_page, new_memseg, ret, mapped_seg_len = 0; > > > > seg_start_page = 0; > > for (cur_page = 0; cur_page < n_pages; cur_page++) { > > @@ -1023,21 +1026,27 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) > > /* if this isn't the first time, remap segment */ > > if (cur_page != 0) { > > ret = remap_segment(hugepages, seg_start_page, > > - cur_page); > > + cur_page, &mapped_seg_len); > > if (ret != 0) > > return -1; > > } > > + cur_page = seg_start_page + mapped_seg_len; > > /* remember where we started */ > > seg_start_page = cur_page; > > + mapped_seg_len = 0; > > } > > /* continuation of previous memseg */ > > } > > /* we were stopped, but we didn't remap the last segment, do it now */ > > if (cur_page != 0) { > > - ret = remap_segment(hugepages, seg_start_page, > > - cur_page); > > - if (ret != 0) > > - return -1; > > + while (seg_start_page < n_pages) { > > + ret = remap_segment(hugepages, seg_start_page, > > + cur_page, &mapped_seg_len); > > + if (ret != 0) > > + return -1; > > + seg_start_page = seg_start_page + mapped_seg_len; > > + mapped_seg_len = 0; > > + } > > } > > return 0; > > } > > This works, but I feel like it's overcomplicated - the same logic you're > trying to use can just be implemented using `find_biggest_free()` + > `find_contig_free()` and returning `seg_len` from `remap_segment()`? > > Something like the following: > > --- > > diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c > index 60fc8cc6ca..08acc787fc 100644 > --- a/lib/eal/linux/eal_memory.c > +++ b/lib/eal/linux/eal_memory.c > @@ -681,6 +681,7 @@ remap_segment(struct hugepage_file *hugepages, int > seg_start, int seg_end) > > /* find free space in memseg lists */ > for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS; msl_idx++) { > + int free_len; > bool empty; > msl = &mcfg->memsegs[msl_idx]; > arr = &msl->memseg_arr; > @@ -692,18 +693,27 @@ remap_segment(struct hugepage_file *hugepages, int > seg_start, int seg_end) > > /* leave space for a hole if array is not empty */ > empty = arr->count == 0; > - ms_idx = rte_fbarray_find_next_n_free(arr, 0, > - seg_len + (empty ? 0 : 1)); > > - /* memseg list is full? */ > - if (ms_idx < 0) > - continue; > + /* find start of the biggest contiguous block and its size */ > + ms_idx = rte_fbarray_find_biggest_free(arr, 0); > + free_len = rte_fbarray_find_contig_free(arr, ms_idx); > > /* leave some space between memsegs, they are not IOVA > * contiguous, so they shouldn't be VA contiguous either. > */ > - if (!empty) > + if (!empty) { > ms_idx++; > + free_len--; > + } > + > + /* memseg list is full? */ > + if (free_len < 1) > + continue; > + > + /* we might not get all of the space we wanted */ > + free_len = RTE_MIN(seg_len, free_len); > + seg_end = seg_start + free_len; > + seg_len = seg_end - seg_start; > break; > } > if (msl_idx == RTE_MAX_MEMSEG_LISTS) { > @@ -787,7 +797,7 @@ remap_segment(struct hugepage_file *hugepages, int > seg_start, int seg_end) > } > RTE_LOG(DEBUG, EAL, "Allocated %" PRIu64 "M on socket %i\n", > (seg_len * page_sz) >> 20, socket_id); > - return 0; > + return seg_len; > } > > static uint64_t > @@ -1022,10 +1032,17 @@ remap_needed_hugepages(struct hugepage_file > *hugepages, int n_pages) > if (new_memseg) { > /* if this isn't the first time, remap segment */ > if (cur_page != 0) { > - ret = remap_segment(hugepages, seg_start_page, > - cur_page); > - if (ret != 0) > - return -1; > + int n_remapped = 0; > + int n_needed = cur_page - seg_start_page; > + > + while (n_remapped < n_needed) { > + ret = remap_segment(hugepages, > + seg_start_page, > + cur_page); > + if (ret < 0) > + return -1; > + n_remapped += ret; > + } > } > /* remember where we started */ > seg_start_page = cur_page; > @@ -1034,10 +1051,16 @@ remap_needed_hugepages(struct hugepage_file > *hugepages, int n_pages) > } > /* we were stopped, but we didn't remap the last segment, do it now */ > if (cur_page != 0) { > - ret = remap_segment(hugepages, seg_start_page, > - cur_page); > - if (ret != 0) > - return -1; > + int n_remapped = 0; > + int n_needed = cur_page - seg_start_page; > + > + while (n_remapped < n_needed) { > + ret = remap_segment( > + hugepages, seg_start_page, cur_page); > + if (ret < 0) > + return -1; > + n_remapped += ret; > + } > } > return 0; > } > > --- > > This should do the trick? Also, this probably needs to be duplicated for > Windows and FreeBSD init as well, because AFAIK they follow the legacy > mem init logic. I take a simple look at FreeBSD and Windows code, FreeBSD don't have this problem, FreeBSD map hugepage is one by one, not mapp multiple at once and windows call eal_dynmem_hugepage_init when have hugepage. So It seems just modify linux is ok. > > -- > Thanks, > Anatoly > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [External] Re: [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode 2023-05-25 12:49 ` [External] " Fengnan Chang @ 2023-05-25 14:26 ` Burakov, Anatoly 0 siblings, 0 replies; 4+ messages in thread From: Burakov, Anatoly @ 2023-05-25 14:26 UTC (permalink / raw) To: Fengnan Chang; +Cc: dev, Lin Li On 5/25/2023 1:49 PM, Fengnan Chang wrote: > Burakov, Anatoly <anatoly.burakov@intel.com> 于2023年5月22日周一 21:28写道： >> >> On 5/22/2023 1:41 PM, Fengnan Chang wrote: >>> Under legacy mode, if the number of continuous memsegs greater >>> than RTE_MAX_MEMSEG_PER_LIST, eal init will failed even though >>> another memseg list is empty, because only one memseg list used >>> to check in remap_needed_hugepages. >>> Fix this by add a argment indicate how many pages mapped in >>> remap_segment, remap_segment try to mapped most pages it can, if >>> exceed it's capbility, remap_needed_hugepages will continue to >>> map other left pages. >>> >>> For example: >>> hugepage configure: >>> cat /sys/devices/system/node/node*/hugepages/hugepages-2048kB/nr_hugepages >>> 10241 >>> 10239 >>> >>> startup log: >>> EAL: Detected memory type: socket_id:0 hugepage_sz:2097152 >>> EAL: Detected memory type: socket_id:1 hugepage_sz:2097152 >>> EAL: Creating 4 segment lists: n_segs:8192 socket_id:0 hugepage_sz:2097152 >>> EAL: Creating 4 segment lists: n_segs:8192 socket_id:1 hugepage_sz:2097152 >>> EAL: Requesting 13370 pages of size 2MB from socket 0 >>> EAL: Requesting 7110 pages of size 2MB from socket 1 >>> EAL: Attempting to map 14220M on socket 1 >>> EAL: Allocated 14220M on socket 1 >>> EAL: Attempting to map 26740M on socket 0 >>> EAL: Could not find space for memseg. Please increase 32768 and/or 65536 in >>> configuration. >>> EAL: Couldn't remap hugepage files into memseg lists >>> EAL: FATAL: Cannot init memory >>> EAL: Cannot init memory >>> >>> Signed-off-by: Fengnan Chang <changfengnan@bytedance.com> >>> Signed-off-by: Lin Li <lilintjpu@bytedance.com> >>> --- >>> lib/eal/linux/eal_memory.c | 33 +++++++++++++++++++++------------ >>> 1 file changed, 21 insertions(+), 12 deletions(-) >>> >>> diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c >>> index 60fc8cc6ca..b2e6453fbe 100644 >>> --- a/lib/eal/linux/eal_memory.c >>> +++ b/lib/eal/linux/eal_memory.c >>> @@ -657,12 +657,12 @@ unmap_unneeded_hugepages(struct hugepage_file *hugepg_tbl, >>> } >>> >>> static int >>> -remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) >>> +remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end, int *mapped_seg_len) >>> { >>> struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config; >>> struct rte_memseg_list *msl; >>> struct rte_fbarray *arr; >>> - int cur_page, seg_len; >>> + int cur_page, seg_len, cur_len; >>> unsigned int msl_idx; >>> int ms_idx; >>> uint64_t page_sz; >>> @@ -692,8 +692,9 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) >>> >>> /* leave space for a hole if array is not empty */ >>> empty = arr->count == 0; >>> + cur_len = RTE_MIN((unsigned int)seg_len, arr->len - arr->count - (empty ? 0 : 1)); >>> ms_idx = rte_fbarray_find_next_n_free(arr, 0, >>> - seg_len + (empty ? 0 : 1)); >>> + cur_len + (empty ? 0 : 1)); >>> >>> /* memseg list is full? */ >>> if (ms_idx < 0) >>> @@ -704,12 +705,12 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) >>> */ >>> if (!empty) >>> ms_idx++; >>> + *mapped_seg_len = cur_len; >>> break; >>> } >>> if (msl_idx == RTE_MAX_MEMSEG_LISTS) { >>> - RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase %s and/or %s in configuration.\n", >>> - RTE_STR(RTE_MAX_MEMSEG_PER_TYPE), >>> - RTE_STR(RTE_MAX_MEM_MB_PER_TYPE)); >>> + RTE_LOG(ERR, EAL, "Could not find space for memseg. Please increase RTE_MAX_MEMSEG_PER_LIST " >>> + "RTE_MAX_MEMSEG_PER_TYPE and/or RTE_MAX_MEM_MB_PER_TYPE in configuration.\n"); >>> return -1; >>> } >>> >>> @@ -725,6 +726,8 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end) >>> void *addr; >>> int fd; >>> >>> + if (cur_page - seg_start == *mapped_seg_len) >>> + break; >>> fd = open(hfile->filepath, O_RDWR); >>> if (fd < 0) { >>> RTE_LOG(ERR, EAL, "Could not open '%s': %s\n", >>> @@ -986,7 +989,7 @@ prealloc_segments(struct hugepage_file *hugepages, int n_pages) >>> static int >>> remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) >>> { >>> - int cur_page, seg_start_page, new_memseg, ret; >>> + int cur_page, seg_start_page, new_memseg, ret, mapped_seg_len = 0; >>> >>> seg_start_page = 0; >>> for (cur_page = 0; cur_page < n_pages; cur_page++) { >>> @@ -1023,21 +1026,27 @@ remap_needed_hugepages(struct hugepage_file *hugepages, int n_pages) >>> /* if this isn't the first time, remap segment */ >>> if (cur_page != 0) { >>> ret = remap_segment(hugepages, seg_start_page, >>> - cur_page); >>> + cur_page, &mapped_seg_len); >>> if (ret != 0) >>> return -1; >>> } >>> + cur_page = seg_start_page + mapped_seg_len; >>> /* remember where we started */ >>> seg_start_page = cur_page; >>> + mapped_seg_len = 0; >>> } >>> /* continuation of previous memseg */ >>> } >>> /* we were stopped, but we didn't remap the last segment, do it now */ >>> if (cur_page != 0) { >>> - ret = remap_segment(hugepages, seg_start_page, >>> - cur_page); >>> - if (ret != 0) >>> - return -1; >>> + while (seg_start_page < n_pages) { >>> + ret = remap_segment(hugepages, seg_start_page, >>> + cur_page, &mapped_seg_len); >>> + if (ret != 0) >>> + return -1; >>> + seg_start_page = seg_start_page + mapped_seg_len; >>> + mapped_seg_len = 0; >>> + } >>> } >>> return 0; >>> } >> >> This works, but I feel like it's overcomplicated - the same logic you're >> trying to use can just be implemented using `find_biggest_free()` + >> `find_contig_free()` and returning `seg_len` from `remap_segment()`? >> >> Something like the following: >> >> --- >> >> diff --git a/lib/eal/linux/eal_memory.c b/lib/eal/linux/eal_memory.c >> index 60fc8cc6ca..08acc787fc 100644 >> --- a/lib/eal/linux/eal_memory.c >> +++ b/lib/eal/linux/eal_memory.c >> @@ -681,6 +681,7 @@ remap_segment(struct hugepage_file *hugepages, int >> seg_start, int seg_end) >> >> /* find free space in memseg lists */ >> for (msl_idx = 0; msl_idx < RTE_MAX_MEMSEG_LISTS; msl_idx++) { >> + int free_len; >> bool empty; >> msl = &mcfg->memsegs[msl_idx]; >> arr = &msl->memseg_arr; >> @@ -692,18 +693,27 @@ remap_segment(struct hugepage_file *hugepages, int >> seg_start, int seg_end) >> >> /* leave space for a hole if array is not empty */ >> empty = arr->count == 0; >> - ms_idx = rte_fbarray_find_next_n_free(arr, 0, >> - seg_len + (empty ? 0 : 1)); >> >> - /* memseg list is full? */ >> - if (ms_idx < 0) >> - continue; >> + /* find start of the biggest contiguous block and its size */ >> + ms_idx = rte_fbarray_find_biggest_free(arr, 0); >> + free_len = rte_fbarray_find_contig_free(arr, ms_idx); >> >> /* leave some space between memsegs, they are not IOVA >> * contiguous, so they shouldn't be VA contiguous either. >> */ >> - if (!empty) >> + if (!empty) { >> ms_idx++; >> + free_len--; >> + } >> + >> + /* memseg list is full? */ >> + if (free_len < 1) >> + continue; >> + >> + /* we might not get all of the space we wanted */ >> + free_len = RTE_MIN(seg_len, free_len); >> + seg_end = seg_start + free_len; >> + seg_len = seg_end - seg_start; >> break; >> } >> if (msl_idx == RTE_MAX_MEMSEG_LISTS) { >> @@ -787,7 +797,7 @@ remap_segment(struct hugepage_file *hugepages, int >> seg_start, int seg_end) >> } >> RTE_LOG(DEBUG, EAL, "Allocated %" PRIu64 "M on socket %i\n", >> (seg_len * page_sz) >> 20, socket_id); >> - return 0; >> + return seg_len; >> } >> >> static uint64_t >> @@ -1022,10 +1032,17 @@ remap_needed_hugepages(struct hugepage_file >> *hugepages, int n_pages) >> if (new_memseg) { >> /* if this isn't the first time, remap segment */ >> if (cur_page != 0) { >> - ret = remap_segment(hugepages, seg_start_page, >> - cur_page); >> - if (ret != 0) >> - return -1; >> + int n_remapped = 0; >> + int n_needed = cur_page - seg_start_page; >> + >> + while (n_remapped < n_needed) { >> + ret = remap_segment(hugepages, >> + seg_start_page, >> + cur_page); >> + if (ret < 0) >> + return -1; >> + n_remapped += ret; >> + } >> } >> /* remember where we started */ >> seg_start_page = cur_page; >> @@ -1034,10 +1051,16 @@ remap_needed_hugepages(struct hugepage_file >> *hugepages, int n_pages) >> } >> /* we were stopped, but we didn't remap the last segment, do it now */ >> if (cur_page != 0) { >> - ret = remap_segment(hugepages, seg_start_page, >> - cur_page); >> - if (ret != 0) >> - return -1; >> + int n_remapped = 0; >> + int n_needed = cur_page - seg_start_page; >> + >> + while (n_remapped < n_needed) { >> + ret = remap_segment( >> + hugepages, seg_start_page, cur_page); >> + if (ret < 0) >> + return -1; >> + n_remapped += ret; >> + } >> } >> return 0; >> } >> >> --- >> >> This should do the trick? Also, this probably needs to be duplicated for >> Windows and FreeBSD init as well, because AFAIK they follow the legacy >> mem init logic. > I take a simple look at FreeBSD and Windows code, FreeBSD don't have this > problem, FreeBSD map hugepage is one by one, not mapp multiple at once > and windows call eal_dynmem_hugepage_init when have hugepage. > So It seems just modify linux is ok. Right, OK then. -- Thanks, Anatoly ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-25 14:26 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-05-22 12:41 [PATCH v2] eal: fix eal init may failed when too much continuous memsegs under legacy mode Fengnan Chang 2023-05-22 13:28 ` Burakov, Anatoly 2023-05-25 12:49 ` [External] " Fengnan Chang 2023-05-25 14:26 ` Burakov, Anatoly
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).