On 09/04/2018 05:15 PM, Anatoly Burakov wrote:
> Enable setting and retrieving segment fd's internally.
>
> For now, retrieving fd's will not be used anywhere until we
> get an external API, but it will be useful for things like
> virtio, where we wish to share segment fd's.
>
> Setting segment fd's will not be available as a public API
> at this time, but internally it is needed for legacy mode,
> because we're not allocating our hugepages in memalloc in
> legacy mode case, and we still need to store the fd.
>
> Another user of get segment fd API is memseg info dump, to
> show which pages use which fd's.
>
> Not supported on FreeBSD.
>
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
> lib/librte_eal/bsdapp/eal/eal_memalloc.c | 12 +++++
> lib/librte_eal/common/eal_common_memory.c | 8 +--
> lib/librte_eal/common/eal_memalloc.h | 6 +++
> lib/librte_eal/linuxapp/eal/eal_memalloc.c | 60 +++++++++++++++++-----
> lib/librte_eal/linuxapp/eal/eal_memory.c | 44 +++++++++++++---
> 5 files changed, 109 insertions(+), 21 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_memalloc.c b/lib/librte_eal/bsdapp/eal/eal_memalloc.c
> index f7f07abd6..a5fb09f71 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_memalloc.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_memalloc.c
> @@ -47,6 +47,18 @@ eal_memalloc_sync_with_primary(void)
> return -1;
> }
>
> +int
> +eal_memalloc_get_seg_fd(int list_idx, int seg_idx)
> +{
> + return -1;
Why not returning -ENOTSUPP directly here? (see patch 7).
> +}
> +
> +int
> +eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd)
> +{
> + return -1;
Ditto.
> +}
> +
Other than that, the patch looks good to me.
Maxime
On 14-Sep-18 8:54 AM, Maxime Coquelin wrote:
>
>
> On 09/04/2018 05:15 PM, Anatoly Burakov wrote:
>> Enable setting and retrieving segment fd's internally.
>>
>> For now, retrieving fd's will not be used anywhere until we
>> get an external API, but it will be useful for things like
>> virtio, where we wish to share segment fd's.
>>
>> Setting segment fd's will not be available as a public API
>> at this time, but internally it is needed for legacy mode,
>> because we're not allocating our hugepages in memalloc in
>> legacy mode case, and we still need to store the fd.
>>
>> Another user of get segment fd API is memseg info dump, to
>> show which pages use which fd's.
>>
>> Not supported on FreeBSD.
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
>> ---
>> lib/librte_eal/bsdapp/eal/eal_memalloc.c | 12 +++++
>> lib/librte_eal/common/eal_common_memory.c | 8 +--
>> lib/librte_eal/common/eal_memalloc.h | 6 +++
>> lib/librte_eal/linuxapp/eal/eal_memalloc.c | 60 +++++++++++++++++-----
>> lib/librte_eal/linuxapp/eal/eal_memory.c | 44 +++++++++++++---
>> 5 files changed, 109 insertions(+), 21 deletions(-)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_memalloc.c
>> b/lib/librte_eal/bsdapp/eal/eal_memalloc.c
>> index f7f07abd6..a5fb09f71 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_memalloc.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_memalloc.c
>> @@ -47,6 +47,18 @@ eal_memalloc_sync_with_primary(void)
>> return -1;
>> }
>> +int
>> +eal_memalloc_get_seg_fd(int list_idx, int seg_idx)
>> +{
>> + return -1;
>
> Why not returning -ENOTSUPP directly here? (see patch 7).
>
>> +}
>> +
>> +int
>> +eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd)
>> +{
>> + return -1;
>
> Ditto.
>
>> +}
>> +
>
> Other than that, the patch looks good to me.
>
> Maxime
>
Mainly for consistency reasons. Returning errno values but not using
them looks weird to me, but i can change this to return those at the outset.
@@ -47,6 +47,18 @@ eal_memalloc_sync_with_primary(void)
return -1;
}
+int
+eal_memalloc_get_seg_fd(int list_idx, int seg_idx)
+{
+ return -1;
+}
+
+int
+eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd)
+{
+ return -1;
+}
+
int
eal_memalloc_init(void)
{
@@ -294,7 +294,7 @@ dump_memseg(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
void *arg)
{
struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
- int msl_idx, ms_idx;
+ int msl_idx, ms_idx, fd;
FILE *f = arg;
msl_idx = msl - mcfg->memsegs;
@@ -305,10 +305,11 @@ dump_memseg(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
if (ms_idx < 0)
return -1;
+ fd = eal_memalloc_get_seg_fd(msl_idx, ms_idx);
fprintf(f, "Segment %i-%i: IOVA:0x%"PRIx64", len:%zu, "
"virt:%p, socket_id:%"PRId32", "
"hugepage_sz:%"PRIu64", nchannel:%"PRIx32", "
- "nrank:%"PRIx32"\n",
+ "nrank:%"PRIx32" fd:%i\n",
msl_idx, ms_idx,
ms->iova,
ms->len,
@@ -316,7 +317,8 @@ dump_memseg(const struct rte_memseg_list *msl, const struct rte_memseg *ms,
ms->socket_id,
ms->hugepage_sz,
ms->nchannel,
- ms->nrank);
+ ms->nrank,
+ fd);
return 0;
}
@@ -76,6 +76,12 @@ eal_memalloc_mem_alloc_validator_unregister(const char *name, int socket_id);
int
eal_memalloc_mem_alloc_validate(int socket_id, size_t new_len);
+int
+eal_memalloc_get_seg_fd(int list_idx, int seg_idx);
+
+int
+eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd);
+
int
eal_memalloc_init(void);
@@ -1334,16 +1334,10 @@ secondary_msl_create_walk(const struct rte_memseg_list *msl,
}
static int
-fd_list_create_walk(const struct rte_memseg_list *msl,
- void *arg __rte_unused)
+alloc_list(int list_idx, int len)
{
- struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
- unsigned int i, len;
- int msl_idx;
int *data;
-
- msl_idx = msl - mcfg->memsegs;
- len = msl->memseg_arr.len;
+ int i;
/* ensure we have space to store fd per each possible segment */
data = malloc(sizeof(int) * len);
@@ -1355,14 +1349,56 @@ fd_list_create_walk(const struct rte_memseg_list *msl,
for (i = 0; i < len; i++)
data[i] = -1;
- fd_list[msl_idx].fds = data;
- fd_list[msl_idx].len = len;
- fd_list[msl_idx].count = 0;
- fd_list[msl_idx].memseg_list_fd = -1;
+ fd_list[list_idx].fds = data;
+ fd_list[list_idx].len = len;
+ fd_list[list_idx].count = 0;
+ fd_list[list_idx].memseg_list_fd = -1;
return 0;
}
+static int
+fd_list_create_walk(const struct rte_memseg_list *msl,
+ void *arg __rte_unused)
+{
+ struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
+ unsigned int len;
+ int msl_idx;
+
+ msl_idx = msl - mcfg->memsegs;
+ len = msl->memseg_arr.len;
+
+ return alloc_list(msl_idx, len);
+}
+
+int
+eal_memalloc_set_seg_fd(int list_idx, int seg_idx, int fd)
+{
+ struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
+
+ /* if list is not allocated, allocate it */
+ if (fd_list[list_idx].len == 0) {
+ int len = mcfg->memsegs[list_idx].memseg_arr.len;
+
+ if (alloc_list(list_idx, len) < 0)
+ return -1;
+ }
+ fd_list[list_idx].fds[seg_idx] = fd;
+
+ return 0;
+}
+
+int
+eal_memalloc_get_seg_fd(int list_idx, int seg_idx)
+{
+ if (internal_config.single_file_segments)
+ return fd_list[list_idx].memseg_list_fd;
+ /* list not initialized */
+ if (fd_list[list_idx].len == 0)
+ return -1;
+ return fd_list[list_idx].fds[seg_idx];
+}
+
int
eal_memalloc_init(void)
{
@@ -772,7 +772,10 @@ remap_segment(struct hugepage_file *hugepages, int seg_start, int seg_end)
rte_fbarray_set_used(arr, ms_idx);
- close(fd);
+ /* store segment fd internally */
+ if (eal_memalloc_set_seg_fd(msl_idx, ms_idx, fd) < 0)
+ RTE_LOG(ERR, EAL, "Could not store segment fd: %s\n",
+ rte_strerror(rte_errno));
}
RTE_LOG(DEBUG, EAL, "Allocated %" PRIu64 "M on socket %i\n",
(seg_len * page_sz) >> 20, socket_id);
@@ -1771,6 +1774,7 @@ getFileSize(int fd)
static int
eal_legacy_hugepage_attach(void)
{
+ struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
struct hugepage_file *hp = NULL;
unsigned int num_hp = 0;
unsigned int i = 0;
@@ -1814,6 +1818,9 @@ eal_legacy_hugepage_attach(void)
struct hugepage_file *hf = &hp[i];
size_t map_sz = hf->size;
void *map_addr = hf->final_va;
+ int msl_idx, ms_idx;
+ struct rte_memseg_list *msl;
+ struct rte_memseg *ms;
/* if size is zero, no more pages left */
if (map_sz == 0)
@@ -1831,25 +1838,50 @@ eal_legacy_hugepage_attach(void)
if (map_addr == MAP_FAILED) {
RTE_LOG(ERR, EAL, "Could not map %s: %s\n",
hf->filepath, strerror(errno));
- close(fd);
- goto error;
+ goto fd_error;
}
/* set shared lock on the file. */
if (flock(fd, LOCK_SH) < 0) {
RTE_LOG(DEBUG, EAL, "%s(): Locking file failed: %s\n",
__func__, strerror(errno));
- close(fd);
- goto error;
+ goto fd_error;
}
- close(fd);
+ /* find segment data */
+ msl = rte_mem_virt2memseg_list(map_addr);
+ if (msl == NULL) {
+ RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg list\n",
+ __func__);
+ goto fd_error;
+ }
+ ms = rte_mem_virt2memseg(map_addr, msl);
+ if (ms == NULL) {
+ RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg\n",
+ __func__);
+ goto fd_error;
+ }
+
+ msl_idx = msl - mcfg->memsegs;
+ ms_idx = rte_fbarray_find_idx(&msl->memseg_arr, ms);
+ if (ms_idx < 0) {
+ RTE_LOG(DEBUG, EAL, "%s(): Cannot find memseg idx\n",
+ __func__);
+ goto fd_error;
+ }
+
+ /* store segment fd internally */
+ if (eal_memalloc_set_seg_fd(msl_idx, ms_idx, fd) < 0)
+ RTE_LOG(ERR, EAL, "Could not store segment fd: %s\n",
+ rte_strerror(rte_errno));
}
/* unmap the hugepage config file, since we are done using it */
munmap(hp, size);
close(fd_hugepage);
return 0;
+fd_error:
+ close(fd);
error:
/* map all segments into memory to make sure we get the addrs */
cur_seg = 0;