[v5,1/2] mem: telemetry support for memseg and element information
Checks
Commit Message
Changes adds telemetry support to display memory occupancy
in memseg and the information of the elements allocated from
a memseg based on arguments provided by user. This patch
adds following endpoints:
1. /eal/memseg_list_array
The command displays the memseg list from which the memory
has been allocated.
Example:
--> /eal/memseg_list_array
{"/eal/memseg_list_array": [0, 1]}
2. /eal/memseg_list_info,<memseg-list-id>
The command outputs the memsegs, from which the memory is
allocated, for the memseg_list given as input.
Example:
--> /eal/memseg_list_info,1
{"/eal/memseg_list_info": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, \
12, 13, 14, 15]}
3. /eal/memseg_info,<memseg-list-id>,<memseg-id>
The command outputs the memseg information based on the
memseg-list and the memseg-id given as input.
Example:
--> /eal/memseg_info,0,10
{"/eal/memseg_info": {"Memseg_list_index": 0, \
"Memseg_index": 10, "Memseg_list_len": 64, \
"Start_addr": "0x260000000", "End_addr": "0x280000000", \
"Size": 536870912}}
--> /eal/memseg_info,1,15
{"/eal/memseg_info": {"Memseg_list_index": 1, \
"Memseg_index": 15, "Memseg_list_len": 64, \
"Start_addr": "0xb20000000", "End_addr": "0xb40000000", \
"Size": 536870912}}
4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
The command outputs number of elements in a memseg based
on the heap-id, memseg-list-id and memseg-id given as input.
Example:
--> /eal/element_list,0,0,63
{"/eal/element_list": {"Element_count": 52}}
--> /eal/element_list,0,1,15
{"/eal/element_list": {"Element_count": 52}}
5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
<elem-start-id>,<elem-end-id>
The command outputs element information like element start
address, end address, to which memseg it belongs, element
state, element size. User can give a range of elements to be
printed.
Example:
--> /eal/element_info,0,1,15,1,2
{"/eal/element_info": {"element.1": {"msl_id": 1, \
"ms_id": 15, "memseg_start_addr": "0xb20000000", \
"memseg_end_addr": "0xb40000000", \
"element_start_addr": "0xb201fe680", \
"element_end_addr": "0xb20bfe700", \
"element_size": 10485888, "element_state": "Busy"}, \
"element.2": {"msl_id": 1, "ms_id": 15, \
"memseg_start_addr": "0xb20000000", \
"memseg_end_addr": "0xb40000000", \
"element_start_addr": "0xb20bfe700", \
"element_end_addr": "0xb215fe780", "element_size": 10485888, \
"element_state": "Busy"}, "Element_count": 2}}
Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
---
v2:
- Fixed compilation error related int-to-pointer-cast
- Changes for code review suggestions
v3:
- Commit message changes
- Renaming end-points
- Changing input parameters to comma-seperated
- Reverting telemetry output buffer size
v4:
- Patch-2 adds telemetry support to display system memory
v5:
- Removed command help related changes
lib/eal/common/eal_common_memory.c | 447 ++++++++++++++++++++++++++++-
1 file changed, 442 insertions(+), 5 deletions(-)
Comments
Hi David,
If no other review comments, could you please pick this series for 22.11 rc1 ?
Thanks,
Amit Shukla
> -----Original Message-----
> From: Amit Prakash Shukla <amitprakashs@marvell.com>
> Sent: Thursday, September 29, 2022 5:13 PM
> To: Anatoly Burakov <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> david.marchand@redhat.com; bruce.richardson@intel.com;
> ciara.power@intel.com; dmitry.kozliuk@gmail.com; Amit Prakash Shukla
> <amitprakashs@marvell.com>
> Subject: [PATCH v5 1/2] mem: telemetry support for memseg and element
> information
>
> Changes adds telemetry support to display memory occupancy in memseg
> and the information of the elements allocated from a memseg based on
> arguments provided by user. This patch adds following endpoints:
>
> 1. /eal/memseg_list_array
> The command displays the memseg list from which the memory has been
> allocated.
> Example:
> --> /eal/memseg_list_array
> {"/eal/memseg_list_array": [0, 1]}
>
> 2. /eal/memseg_list_info,<memseg-list-id>
> The command outputs the memsegs, from which the memory is allocated,
> for the memseg_list given as input.
> Example:
> --> /eal/memseg_list_info,1
> {"/eal/memseg_list_info": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, \ 12, 13, 14, 15]}
>
> 3. /eal/memseg_info,<memseg-list-id>,<memseg-id>
> The command outputs the memseg information based on the memseg-list
> and the memseg-id given as input.
> Example:
> --> /eal/memseg_info,0,10
> {"/eal/memseg_info": {"Memseg_list_index": 0, \
> "Memseg_index": 10, "Memseg_list_len": 64, \
> "Start_addr": "0x260000000", "End_addr": "0x280000000", \
> "Size": 536870912}}
>
> --> /eal/memseg_info,1,15
> {"/eal/memseg_info": {"Memseg_list_index": 1, \
> "Memseg_index": 15, "Memseg_list_len": 64, \
> "Start_addr": "0xb20000000", "End_addr": "0xb40000000", \
> "Size": 536870912}}
>
> 4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
> The command outputs number of elements in a memseg based on the heap-
> id, memseg-list-id and memseg-id given as input.
> Example:
> --> /eal/element_list,0,0,63
> {"/eal/element_list": {"Element_count": 52}}
>
> --> /eal/element_list,0,1,15
> {"/eal/element_list": {"Element_count": 52}}
>
> 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> <elem-start-id>,<elem-end-id>
> The command outputs element information like element start address, end
> address, to which memseg it belongs, element state, element size. User can
> give a range of elements to be printed.
> Example:
> --> /eal/element_info,0,1,15,1,2
> {"/eal/element_info": {"element.1": {"msl_id": 1, \
> "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb201fe680", \
> "element_end_addr": "0xb20bfe700", \
> "element_size": 10485888, "element_state": "Busy"}, \
> "element.2": {"msl_id": 1, "ms_id": 15, \
> "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb20bfe700", \
> "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> "element_state": "Busy"}, "Element_count": 2}}
>
> Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> ---
> v2:
> - Fixed compilation error related int-to-pointer-cast
> - Changes for code review suggestions
>
> v3:
> - Commit message changes
> - Renaming end-points
> - Changing input parameters to comma-seperated
> - Reverting telemetry output buffer size
>
> v4:
> - Patch-2 adds telemetry support to display system memory
>
> v5:
> - Removed command help related changes
>
> lib/eal/common/eal_common_memory.c | 447
> ++++++++++++++++++++++++++++-
> 1 file changed, 442 insertions(+), 5 deletions(-)
>
> diff --git a/lib/eal/common/eal_common_memory.c
> b/lib/eal/common/eal_common_memory.c
> index 688dc615d7..6b863979e9 100644
> --- a/lib/eal/common/eal_common_memory.c
> +++ b/lib/eal/common/eal_common_memory.c
> @@ -3,6 +3,7 @@
> */
>
> #include <errno.h>
> +#include <ctype.h>
> #include <stdio.h>
> #include <stdint.h>
> #include <stdlib.h>
> @@ -26,6 +27,7 @@
> #include "eal_memcfg.h"
> #include "eal_options.h"
> #include "malloc_heap.h"
> +#include "malloc_elem.h"
>
> /*
> * Try to mmap *size bytes in /dev/zero. If it is successful, return the @@ -
> 1113,11 +1115,17 @@ rte_eal_memory_init(void) }
>
> #ifndef RTE_EXEC_ENV_WINDOWS
> -#define EAL_MEMZONE_LIST_REQ "/eal/memzone_list"
> -#define EAL_MEMZONE_INFO_REQ "/eal/memzone_info"
> -#define EAL_HEAP_LIST_REQ "/eal/heap_list"
> -#define EAL_HEAP_INFO_REQ "/eal/heap_info"
> -#define ADDR_STR 15
> +#define EAL_MEMZONE_LIST_REQ "/eal/memzone_list"
> +#define EAL_MEMZONE_INFO_REQ "/eal/memzone_info"
> +#define EAL_HEAP_LIST_REQ "/eal/heap_list"
> +#define EAL_HEAP_INFO_REQ "/eal/heap_info"
> +#define EAL_MEMSEG_LIST_ARR_REQ
> "/eal/memseg_list_array"
> +#define EAL_MEMSEG_LIST_INFO_REQ "/eal/memseg_list_info"
> +#define EAL_MEMSEG_INFO_REQ "/eal/memseg_info"
> +#define EAL_ELEMENT_LIST_REQ "/eal/element_list"
> +#define EAL_ELEMENT_INFO_REQ "/eal/element_info"
> +#define ADDR_STR 15
> +
>
> /* Telemetry callback handler to return heap stats for requested heap id. */
> static int @@ -1265,6 +1273,418 @@
> handle_eal_memzone_list_request(const char *cmd __rte_unused,
> return 0;
> }
>
> +static int
> +handle_eal_memseg_list_array_request(const char *cmd __rte_unused,
> + const char *params __rte_unused,
> + struct rte_tel_data *d)
> +{
> + struct rte_mem_config *mcfg;
> + int i;
> +
> + rte_tel_data_start_array(d, RTE_TEL_INT_VAL);
> +
> + rte_mcfg_mem_read_lock();
> + mcfg = rte_eal_get_configuration()->mem_config;
> +
> + for (i = 0; i < RTE_MAX_MEMSEG_LISTS; i++) {
> + struct rte_memseg_list *msl = &mcfg->memsegs[i];
> + if (msl->memseg_arr.count == 0)
> + continue;
> +
> + rte_tel_data_add_array_int(d, i);
> + }
> + rte_mcfg_mem_read_unlock();
> +
> + return 0;
> +}
> +
> +static int
> +handle_eal_memseg_list_info_request(const char *cmd __rte_unused,
> + const char *params, struct rte_tel_data *d)
> {
> + struct rte_mem_config *mcfg;
> + struct rte_memseg_list *msl;
> + struct rte_fbarray *arr;
> + uint32_t ms_list_idx;
> + int ms_idx;
> +
> + if (params == NULL || strlen(params) == 0)
> + return -1;
> +
> + if (!isdigit(*params))
> + return -1;
> +
> + ms_list_idx = strtoul(params, NULL, 10);
> + if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS)
> + return -1;
> +
> + rte_tel_data_start_array(d, RTE_TEL_INT_VAL);
> +
> + rte_mcfg_mem_read_lock();
> + mcfg = rte_eal_get_configuration()->mem_config;
> + msl = &mcfg->memsegs[ms_list_idx];
> + if (msl->memseg_arr.count == 0)
> + goto done;
> +
> + arr = &msl->memseg_arr;
> +
> + ms_idx = rte_fbarray_find_next_used(arr, 0);
> + while (ms_idx >= 0) {
> + rte_tel_data_add_array_int(d, ms_idx);
> + ms_idx = rte_fbarray_find_next_used(arr, ms_idx + 1);
> + }
> +
> +done:
> + rte_mcfg_mem_read_unlock();
> +
> + return 0;
> +}
> +
> +static int
> +handle_eal_memseg_info_request(const char *cmd __rte_unused,
> + const char *params, struct rte_tel_data *d) {
> + struct rte_mem_config *mcfg;
> + uint64_t ms_start_addr, ms_end_addr, ms_size;
> + struct rte_memseg_list *msl;
> + const struct rte_memseg *ms;
> + struct rte_fbarray *arr;
> + char addr[ADDR_STR];
> + uint32_t ms_list_idx = 0;
> + uint32_t ms_idx = 0;
> + uint32_t msl_len;
> + char dlim[2] = ",";
> + char *token;
> + char *params_args;
> +
> + if (params == NULL || strlen(params) == 0)
> + return -1;
> +
> + /* strtok expects char * and param is const char *. Hence on using
> + * params as "const char *" compiler throws warning.
> + */
> + params_args = strdup(params);
> + token = strtok(params_args, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + ms_list_idx = strtoul(token, NULL, 10);
> + if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
> + free(params_args);
> + return -1;
> + }
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> + ms_idx = strtoul(token, NULL, 10);
> +
> + free(params_args);
> +
> + rte_mcfg_mem_read_lock();
> +
> + mcfg = rte_eal_get_configuration()->mem_config;
> + msl = &mcfg->memsegs[ms_list_idx];
> + if (msl->memseg_arr.count == 0) {
> + rte_mcfg_mem_read_unlock();
> + return -1;
> + }
> +
> + arr = &msl->memseg_arr;
> + msl_len = arr->len;
> +
> + ms = rte_fbarray_get(arr, ms_idx);
> + if (ms == NULL) {
> + rte_mcfg_mem_read_unlock();
> + RTE_LOG(DEBUG, EAL, "Error fetching requested
> memseg.\n");
> + return -1;
> + }
> +
> + ms_start_addr = ms->addr_64;
> + ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
> + ms_size = ms->hugepage_sz;
> +
> + rte_mcfg_mem_read_unlock();
> +
> + rte_tel_data_start_dict(d);
> + rte_tel_data_add_dict_int(d, "Memseg_list_index", ms_list_idx);
> + rte_tel_data_add_dict_int(d, "Memseg_index", ms_idx);
> + rte_tel_data_add_dict_int(d, "Memseg_list_len", msl_len);
> + snprintf(addr, ADDR_STR, "0x%"PRIx64, ms_start_addr);
> + rte_tel_data_add_dict_string(d, "Start_addr", addr);
> + snprintf(addr, ADDR_STR, "0x%"PRIx64, ms_end_addr);
> + rte_tel_data_add_dict_string(d, "End_addr", addr);
> + rte_tel_data_add_dict_int(d, "Size", ms_size);
> +
> + return 0;
> +}
> +
> +static int
> +handle_eal_element_list_request(const char *cmd __rte_unused,
> + const char *params, struct rte_tel_data *d) {
> + struct rte_mem_config *mcfg;
> + struct rte_memseg_list *msl;
> + const struct rte_memseg *ms;
> + struct malloc_elem *elem;
> + struct malloc_heap *heap;
> + uint64_t ms_start_addr, ms_end_addr;
> + uint64_t elem_start_addr, elem_end_addr;
> + uint32_t ms_list_idx = 0;
> + uint32_t heap_id = 0;
> + uint32_t ms_idx = 0;
> + char dlim[2] = ",";
> + int elem_count = 0;
> + char *token;
> + char *params_args;
> +
> + if (params == NULL || strlen(params) == 0)
> + return -1;
> +
> + /* strtok expects char * and param is const char *. Hence on using
> + * params as "const char *" compiler throws warning.
> + */
> + params_args = strdup(params);
> + token = strtok(params_args, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + heap_id = strtoul(token, NULL, 10);
> + if (heap_id >= RTE_MAX_HEAPS) {
> + free(params_args);
> + return -1;
> + }
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + ms_list_idx = strtoul(token, NULL, 10);
> + if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
> + free(params_args);
> + return -1;
> + }
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + ms_idx = strtoul(token, NULL, 10);
> +
> + free(params_args);
> +
> + rte_mcfg_mem_read_lock();
> +
> + mcfg = rte_eal_get_configuration()->mem_config;
> + msl = &mcfg->memsegs[ms_list_idx];
> + ms = rte_fbarray_get(&msl->memseg_arr, ms_idx);
> + if (ms == NULL) {
> + rte_mcfg_mem_read_unlock();
> + RTE_LOG(DEBUG, EAL, "Error fetching requested
> memseg.\n");
> + return -1;
> + }
> +
> + ms_start_addr = ms->addr_64;
> + ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
> + rte_mcfg_mem_read_unlock();
> +
> + rte_tel_data_start_dict(d);
> +
> + heap = &mcfg->malloc_heaps[heap_id];
> + rte_spinlock_lock(&heap->lock);
> +
> + elem = heap->first;
> + while (elem) {
> + elem_start_addr = (uint64_t)elem;
> + elem_end_addr =
> + (uint64_t)RTE_PTR_ADD(elem_start_addr, elem-
> >size);
> +
> + if ((uint64_t)elem_start_addr >= ms_start_addr &&
> + (uint64_t)elem_end_addr <= ms_end_addr)
> + elem_count++;
> + elem = elem->next;
> + }
> +
> + rte_spinlock_unlock(&heap->lock);
> +
> + rte_tel_data_add_dict_int(d, "Element_count", elem_count);
> +
> + return 0;
> +}
> +
> +static int
> +handle_eal_element_info_request(const char *cmd __rte_unused,
> + const char *params, struct rte_tel_data *d) {
> + struct rte_mem_config *mcfg;
> + struct rte_memseg_list *msl;
> + const struct rte_memseg *ms;
> + struct malloc_elem *elem;
> + struct malloc_heap *heap;
> + struct rte_tel_data *c;
> + uint64_t ms_start_addr, ms_end_addr;
> + uint64_t elem_start_addr, elem_end_addr;
> + uint32_t ms_list_idx = 0;
> + uint32_t heap_id = 0;
> + uint32_t ms_idx = 0;
> + uint32_t start_elem = 0, end_elem = 0;
> + uint32_t count = 0, elem_count = 0;
> + char dlim[2] = ",";
> + char str[ADDR_STR];
> + char *params_args;
> + char *token;
> +
> + if (params == NULL || strlen(params) == 0)
> + return -1;
> +
> + /* strtok expects char * and param is const char *. Hence on using
> + * params as "const char *" compiler throws warning.
> + */
> + params_args = strdup(params);
> + token = strtok(params_args, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + heap_id = strtoul(token, NULL, 10);
> + if (heap_id >= RTE_MAX_HEAPS) {
> + free(params_args);
> + return -1;
> + }
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + ms_list_idx = strtoul(token, NULL, 10);
> + if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
> + free(params_args);
> + return -1;
> + }
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + ms_idx = strtoul(token, NULL, 10);
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + start_elem = strtoul(token, NULL, 10);
> +
> + token = strtok(NULL, dlim);
> + if (token == NULL || !isdigit(*token)) {
> + free(params_args);
> + return -1;
> + }
> +
> + end_elem = strtoul(token, NULL, 10);
> +
> + free(params_args);
> +
> + if (end_elem < start_elem)
> + return -1;
> +
> + rte_mcfg_mem_read_lock();
> +
> + mcfg = rte_eal_get_configuration()->mem_config;
> + msl = &mcfg->memsegs[ms_list_idx];
> + ms = rte_fbarray_get(&msl->memseg_arr, ms_idx);
> + if (ms == NULL) {
> + rte_mcfg_mem_read_unlock();
> + RTE_LOG(DEBUG, EAL, "Error fetching requested
> memseg.\n");
> + return -1;
> + }
> +
> + ms_start_addr = ms->addr_64;
> + ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
> +
> + rte_mcfg_mem_read_unlock();
> +
> + rte_tel_data_start_dict(d);
> +
> + heap = &mcfg->malloc_heaps[heap_id];
> + rte_spinlock_lock(&heap->lock);
> +
> + elem = heap->first;
> + while (elem) {
> + elem_start_addr = (uint64_t)elem;
> + elem_end_addr =
> + (uint64_t)RTE_PTR_ADD(elem_start_addr, elem-
> >size);
> +
> + if (elem_start_addr < ms_start_addr ||
> + elem_end_addr > ms_end_addr) {
> + elem = elem->next;
> + continue;
> + }
> +
> + if (count < start_elem) {
> + elem = elem->next;
> + count++;
> + continue;
> + }
> +
> + c = rte_tel_data_alloc();
> + if (c == NULL)
> + break;
> +
> + rte_tel_data_start_dict(c);
> + rte_tel_data_add_dict_int(c, "msl_id", ms_list_idx);
> + rte_tel_data_add_dict_int(c, "ms_id", ms_idx);
> + snprintf(str, ADDR_STR, "0x%"PRIx64, ms_start_addr);
> + rte_tel_data_add_dict_string(c, "memseg_start_addr", str);
> + snprintf(str, ADDR_STR, "0x%"PRIx64, ms_end_addr);
> + rte_tel_data_add_dict_string(c, "memseg_end_addr", str);
> + snprintf(str, ADDR_STR, "0x%"PRIx64, elem_start_addr);
> + rte_tel_data_add_dict_string(c, "element_start_addr", str);
> + snprintf(str, ADDR_STR, "0x%"PRIx64, elem_end_addr);
> + rte_tel_data_add_dict_string(c, "element_end_addr", str);
> + rte_tel_data_add_dict_int(c, "element_size", elem->size);
> + snprintf(str, ADDR_STR, "%s", elem->state == 0 ? "Free" :
> + elem->state == 1 ? "Busy" : elem->state == 2 ?
> + "Pad" : "Error");
> + rte_tel_data_add_dict_string(c, "element_state", str);
> +
> + snprintf(str, ADDR_STR, "%s.%u", "element", count);
> + if (rte_tel_data_add_dict_container(d, str, c, 0) != 0) {
> + rte_tel_data_free(c);
> + break;
> + }
> +
> + elem_count++;
> + count++;
> + if (count > end_elem)
> + break;
> +
> + elem = elem->next;
> + }
> +
> + rte_spinlock_unlock(&heap->lock);
> +
> + rte_tel_data_add_dict_int(d, "Element_count", elem_count);
> +
> + return 0;
> +}
> +
> RTE_INIT(memory_telemetry)
> {
> rte_telemetry_register_cmd(
> @@ -1279,5 +1699,22 @@ RTE_INIT(memory_telemetry)
> rte_telemetry_register_cmd(
> EAL_HEAP_INFO_REQ,
> handle_eal_heap_info_request,
> "Returns malloc heap stats. Parameters: int
> heap_id");
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_LIST_ARR_REQ,
> + handle_eal_memseg_list_array_request,
> + "Returns hugepage list. Takes no parameters");
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_LIST_INFO_REQ,
> + handle_eal_memseg_list_info_request,
> + "Returns memseg list. Parameters: int
> memseg_list_id");
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_INFO_REQ,
> handle_eal_memseg_info_request,
> + "Returns memseg info. Parameter: int
> memseg_list_id,int memseg_id");
> + rte_telemetry_register_cmd(EAL_ELEMENT_LIST_REQ,
> + handle_eal_element_list_request,
> + "Returns element info. Parameters: int heap_id, int
> memseg_list_id, int memseg_id");
> + rte_telemetry_register_cmd(EAL_ELEMENT_INFO_REQ,
> + handle_eal_element_info_request,
> + "Returns element info. Parameters: int heap_id,
> memseg_list_id,
> +memseg_id, start_elem_id, end_elem_id");
> }
> #endif
> --
> 2.25.1
On Thu, Sep 29, 2022 at 1:43 PM Amit Prakash Shukla
<amitprakashs@marvell.com> wrote:
> 4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
> The command outputs number of elements in a memseg based
> on the heap-id, memseg-list-id and memseg-id given as input.
> Example:
> --> /eal/element_list,0,0,63
> {"/eal/element_list": {"Element_count": 52}}
>
> --> /eal/element_list,0,1,15
> {"/eal/element_list": {"Element_count": 52}}
>
> 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> <elem-start-id>,<elem-end-id>
> The command outputs element information like element start
> address, end address, to which memseg it belongs, element
> state, element size. User can give a range of elements to be
> printed.
> Example:
> --> /eal/element_info,0,1,15,1,2
> {"/eal/element_info": {"element.1": {"msl_id": 1, \
> "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb201fe680", \
> "element_end_addr": "0xb20bfe700", \
> "element_size": 10485888, "element_state": "Busy"}, \
> "element.2": {"msl_id": 1, "ms_id": 15, \
> "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb20bfe700", \
> "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> "element_state": "Busy"}, "Element_count": 2}}
>
element is too generic.
Just look at the command name: /eal/element_XXX.
What is an EAL element?
On Thu, Oct 6, 2022 at 9:07 AM Amit Prakash Shukla
<amitprakashs@marvell.com> wrote:
>
> Hi David,
>
> If no other review comments, could you please pick this series for 22.11 rc1 ?
I had a better look and sent some comments.
We can still consider this series for rc2 in any case.
Thanks David for the feedback.
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Saturday, October 8, 2022 1:19 AM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Anatoly Burakov
> <anatoly.burakov@intel.com>; dmitry.kozliuk@gmail.com
> Cc: dev@dpdk.org; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> bruce.richardson@intel.com; ciara.power@intel.com; Thomas Monjalon
> <thomas@monjalon.net>
> Subject: [EXT] Re: [PATCH v5 1/2] mem: telemetry support for memseg and
> element information
>
> External Email
>
> ----------------------------------------------------------------------
> On Thu, Sep 29, 2022 at 1:43 PM Amit Prakash Shukla
> <amitprakashs@marvell.com> wrote:
> > 4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
> > The command outputs number of elements in a memseg based on the
> > heap-id, memseg-list-id and memseg-id given as input.
> > Example:
> > --> /eal/element_list,0,0,63
> > {"/eal/element_list": {"Element_count": 52}}
> >
> > --> /eal/element_list,0,1,15
> > {"/eal/element_list": {"Element_count": 52}}
> >
> > 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> > <elem-start-id>,<elem-end-id>
> > The command outputs element information like element start address,
> > end address, to which memseg it belongs, element state, element size.
> > User can give a range of elements to be printed.
> > Example:
> > --> /eal/element_info,0,1,15,1,2
> > {"/eal/element_info": {"element.1": {"msl_id": 1, \
> > "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> > "memseg_end_addr": "0xb40000000", \
> > "element_start_addr": "0xb201fe680", \
> > "element_end_addr": "0xb20bfe700", \
> > "element_size": 10485888, "element_state": "Busy"}, \
> > "element.2": {"msl_id": 1, "ms_id": 15, \
> > "memseg_start_addr": "0xb20000000", \
> > "memseg_end_addr": "0xb40000000", \
> > "element_start_addr": "0xb20bfe700", \
> > "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> > "element_state": "Busy"}, "Element_count": 2}}
> >
>
> element is too generic.
> Just look at the command name: /eal/element_XXX.
> What is an EAL element?
Sure, we can rename the commands to /eal/mem_element_XXX in v6 ?
>
>
> --
> David Marchand
- Amit Shukla
2022-09-29 17:13 (UTC+0530), Amit Prakash Shukla:
> Changes adds telemetry support to display memory occupancy
> in memseg and the information of the elements allocated from
> a memseg based on arguments provided by user. This patch
> adds following endpoints:
>
> 1. /eal/memseg_list_array
> The command displays the memseg list from which the memory
> has been allocated.
> Example:
> --> /eal/memseg_list_array
> {"/eal/memseg_list_array": [0, 1]}
>
> 2. /eal/memseg_list_info,<memseg-list-id>
> The command outputs the memsegs, from which the memory is
> allocated, for the memseg_list given as input.
> Example:
> --> /eal/memseg_list_info,1
> {"/eal/memseg_list_info": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, \
> 12, 13, 14, 15]}
MSL has more properties worth reporting.
Also note that by default
#define RTE_MAX_MEMSEG_PER_LIST 8192
which means that the array will not fit into the output buffer (16KB).
Large number of memsegs is quite possible with 2MB hugepages.
I suggest to have a request for MSL properties, including length,
and this request be a separate one.
If this one fails due to insufficient buffer,
the user will at least know the range of possible indices.
> 3. /eal/memseg_info,<memseg-list-id>,<memseg-id>
> The command outputs the memseg information based on the
> memseg-list and the memseg-id given as input.
> Example:
> --> /eal/memseg_info,0,10
> {"/eal/memseg_info": {"Memseg_list_index": 0, \
> "Memseg_index": 10, "Memseg_list_len": 64, \
> "Start_addr": "0x260000000", "End_addr": "0x280000000", \
> "Size": 536870912}}
"Memseg_list_len" is neither a property or an identifier of a memseg.
Important memseg fields are missing, like socket, hugepage_sz, and flags.
Note that "Size" displays hugepage_sz, but this is not correct:
for external memory memseg is not necessarily a single page.
Size and hugepage size fields must be distinct.
>
> --> /eal/memseg_info,1,15
> {"/eal/memseg_info": {"Memseg_list_index": 1, \
> "Memseg_index": 15, "Memseg_list_len": 64, \
> "Start_addr": "0xb20000000", "End_addr": "0xb40000000", \
> "Size": 536870912}}
>
> 4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
> The command outputs number of elements in a memseg based
> on the heap-id, memseg-list-id and memseg-id given as input.
> Example:
> --> /eal/element_list,0,0,63
> {"/eal/element_list": {"Element_count": 52}}
How does the user learn heap_id?
There probably should be /eal/heap_id returning a list of heap IDs.
Please use a consistent naming scheme for requests returning ID lists.
Currently MSL have "_array" suffix but memsegs and elements don't.
> --> /eal/element_list,0,1,15
> {"/eal/element_list": {"Element_count": 52}}
>
> 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> <elem-start-id>,<elem-end-id>
> The command outputs element information like element start
> address, end address, to which memseg it belongs, element
> state, element size. User can give a range of elements to be
> printed.
> Example:
> --> /eal/element_info,0,1,15,1,2
> {"/eal/element_info": {"element.1": {"msl_id": 1, \
> "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb201fe680", \
> "element_end_addr": "0xb20bfe700", \
> "element_size": 10485888, "element_state": "Busy"}, \
> "element.2": {"msl_id": 1, "ms_id": 15, \
> "memseg_start_addr": "0xb20000000", \
> "memseg_end_addr": "0xb40000000", \
> "element_start_addr": "0xb20bfe700", \
> "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> "element_state": "Busy"}, "Element_count": 2}}
How this request is going to be used?
Elements don't have permanent IDs like MSL/memseg index or heap ID.
Heap layout may change between /eal/element_list and this request.
Maybe instead there should be a filter by address
with maybe a context parameter (like "grep -C")?
The proposed API is not bad at all by itself,
I'm asking to make sure it solves the task in the best way.
[...]
> +static int
> +handle_eal_memseg_info_request(const char *cmd __rte_unused,
> + const char *params, struct rte_tel_data *d)
> +{
> + struct rte_mem_config *mcfg;
> + uint64_t ms_start_addr, ms_end_addr, ms_size;
> + struct rte_memseg_list *msl;
> + const struct rte_memseg *ms;
> + struct rte_fbarray *arr;
> + char addr[ADDR_STR];
> + uint32_t ms_list_idx = 0;
> + uint32_t ms_idx = 0;
> + uint32_t msl_len;
> + char dlim[2] = ",";
> + char *token;
> + char *params_args;
> +
> + if (params == NULL || strlen(params) == 0)
> + return -1;
> +
> + /* strtok expects char * and param is const char *. Hence on using
> + * params as "const char *" compiler throws warning.
> + */
> + params_args = strdup(params);
Please check the allocation result hear and in the rest of the handlers.
It would be nice to have a local helper to parse N integer params,
this would reduce and simplify the code:
static int
parse_params(const char *params, int *vals, size_t vals_n);
[...]
> RTE_INIT(memory_telemetry)
> {
> rte_telemetry_register_cmd(
> @@ -1279,5 +1699,22 @@ RTE_INIT(memory_telemetry)
> rte_telemetry_register_cmd(
> EAL_HEAP_INFO_REQ, handle_eal_heap_info_request,
> "Returns malloc heap stats. Parameters: int heap_id");
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_LIST_ARR_REQ,
> + handle_eal_memseg_list_array_request,
> + "Returns hugepage list. Takes no parameters");
"hugepage list" -> "array of memseg list IDs"
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_LIST_INFO_REQ,
> + handle_eal_memseg_list_info_request,
> + "Returns memseg list. Parameters: int memseg_list_id");
"memseg list" -> "memseg list info"
> + rte_telemetry_register_cmd(
> + EAL_MEMSEG_INFO_REQ, handle_eal_memseg_info_request,
> + "Returns memseg info. Parameter: int memseg_list_id,int memseg_id");
> + rte_telemetry_register_cmd(EAL_ELEMENT_LIST_REQ,
> + handle_eal_element_list_request,
> + "Returns element info. Parameters: int heap_id, int memseg_list_id, int memseg_id");
"element info" -> "array of heap element IDs".
> + rte_telemetry_register_cmd(EAL_ELEMENT_INFO_REQ,
> + handle_eal_element_info_request,
> + "Returns element info. Parameters: int heap_id, memseg_list_id, memseg_id, start_elem_id, end_elem_id");
> }
> #endif
Please make parameter descriptions consistent ("int x, int y" vs "int x, y").
Thanks Dmitry for the feedback. Please find my reply in-line.
> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Sent: Thursday, October 20, 2022 5:11 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>
> Cc: Anatoly Burakov <anatoly.burakov@intel.com>; dev@dpdk.org; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; david.marchand@redhat.com;
> bruce.richardson@intel.com; ciara.power@intel.com
> Subject: [EXT] Re: [PATCH v5 1/2] mem: telemetry support for memseg and
> element information
>
> External Email
>
> ----------------------------------------------------------------------
> 2022-09-29 17:13 (UTC+0530), Amit Prakash Shukla:
> > Changes adds telemetry support to display memory occupancy in memseg
> > and the information of the elements allocated from a memseg based on
> > arguments provided by user. This patch adds following endpoints:
> >
> > 1. /eal/memseg_list_array
> > The command displays the memseg list from which the memory has been
> > allocated.
> > Example:
> > --> /eal/memseg_list_array
> > {"/eal/memseg_list_array": [0, 1]}
> >
> > 2. /eal/memseg_list_info,<memseg-list-id>
> > The command outputs the memsegs, from which the memory is allocated,
> > for the memseg_list given as input.
> > Example:
> > --> /eal/memseg_list_info,1
> > {"/eal/memseg_list_info": [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, \
> > 12, 13, 14, 15]}
>
> MSL has more properties worth reporting.
> Also note that by default
>
> #define RTE_MAX_MEMSEG_PER_LIST 8192
>
> which means that the array will not fit into the output buffer (16KB).
> Large number of memsegs is quite possible with 2MB hugepages.
> I suggest to have a request for MSL properties, including length, and this
> request be a separate one.
> If this one fails due to insufficient buffer, the user will at least know the
> range of possible indices.
Agreed. Will implement new request for MSL properties in v6.
>
> > 3. /eal/memseg_info,<memseg-list-id>,<memseg-id>
> > The command outputs the memseg information based on the memseg-list
> > and the memseg-id given as input.
> > Example:
> > --> /eal/memseg_info,0,10
> > {"/eal/memseg_info": {"Memseg_list_index": 0, \
> > "Memseg_index": 10, "Memseg_list_len": 64, \
> > "Start_addr": "0x260000000", "End_addr": "0x280000000", \
> > "Size": 536870912}}
>
> "Memseg_list_len" is neither a property or an identifier of a memseg.
Idea to print Memseg_list_len was to display total number of memsegs in the MSL,
But I agree with you that it makes more sense to be printed in new request for
MSL properties.
> Important memseg fields are missing, like socket, hugepage_sz, and flags.
> Note that "Size" displays hugepage_sz, but this is not correct:
> for external memory memseg is not necessarily a single page.
> Size and hugepage size fields must be distinct.
Sure, will add it in v6.
>
> >
> > --> /eal/memseg_info,1,15
> > {"/eal/memseg_info": {"Memseg_list_index": 1, \
> > "Memseg_index": 15, "Memseg_list_len": 64, \
> > "Start_addr": "0xb20000000", "End_addr": "0xb40000000", \
> > "Size": 536870912}}
> >
> > 4. /eal/element_list,<heap-id>,<memseg-list-id>,<memseg-id>
> > The command outputs number of elements in a memseg based on the
> > heap-id, memseg-list-id and memseg-id given as input.
> > Example:
> > --> /eal/element_list,0,0,63
> > {"/eal/element_list": {"Element_count": 52}}
>
> How does the user learn heap_id?
> There probably should be /eal/heap_id returning a list of heap IDs.
Request for list of active heap Id's is already present.
" /eal/heap_list"
>
> Please use a consistent naming scheme for requests returning ID lists.
> Currently MSL have "_array" suffix but memsegs and elements don't.
Sure
>
> > --> /eal/element_list,0,1,15
> > {"/eal/element_list": {"Element_count": 52}}
> >
> > 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> > <elem-start-id>,<elem-end-id>
> > The command outputs element information like element start address,
> > end address, to which memseg it belongs, element state, element size.
> > User can give a range of elements to be printed.
> > Example:
> > --> /eal/element_info,0,1,15,1,2
> > {"/eal/element_info": {"element.1": {"msl_id": 1, \
> > "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> > "memseg_end_addr": "0xb40000000", \
> > "element_start_addr": "0xb201fe680", \
> > "element_end_addr": "0xb20bfe700", \
> > "element_size": 10485888, "element_state": "Busy"}, \
> > "element.2": {"msl_id": 1, "ms_id": 15, \
> > "memseg_start_addr": "0xb20000000", \
> > "memseg_end_addr": "0xb40000000", \
> > "element_start_addr": "0xb20bfe700", \
> > "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> > "element_state": "Busy"}, "Element_count": 2}}
>
> How this request is going to be used?
> Elements don't have permanent IDs like MSL/memseg index or heap ID.
> Heap layout may change between /eal/element_list and this request.
Idea here was to print information related to memory element. This information
Can be printed for a single element or for a range of elements.
> Maybe instead there should be a filter by address with maybe a context
> parameter (like "grep -C")?
You mean that the user shall be able to grep for a memory address to check
which element it belongs to ? If my understanding is correct, can I implement
it as new request post rc2 and keep this request as-is?
> The proposed API is not bad at all by itself, I'm asking to make sure it solves
> the task in the best way.
>
> [...]
>
> > +static int
> > +handle_eal_memseg_info_request(const char *cmd __rte_unused,
> > + const char *params, struct rte_tel_data *d) {
> > + struct rte_mem_config *mcfg;
> > + uint64_t ms_start_addr, ms_end_addr, ms_size;
> > + struct rte_memseg_list *msl;
> > + const struct rte_memseg *ms;
> > + struct rte_fbarray *arr;
> > + char addr[ADDR_STR];
> > + uint32_t ms_list_idx = 0;
> > + uint32_t ms_idx = 0;
> > + uint32_t msl_len;
> > + char dlim[2] = ",";
> > + char *token;
> > + char *params_args;
> > +
> > + if (params == NULL || strlen(params) == 0)
> > + return -1;
> > +
> > + /* strtok expects char * and param is const char *. Hence on using
> > + * params as "const char *" compiler throws warning.
> > + */
> > + params_args = strdup(params);
>
> Please check the allocation result hear and in the rest of the handlers.
Sure, will do it in v6.
>
> It would be nice to have a local helper to parse N integer params, this would
> reduce and simplify the code:
>
> static int
> parse_params(const char *params, int *vals, size_t vals_n);
>
Sure.
> [...]
> > RTE_INIT(memory_telemetry)
> > {
> > rte_telemetry_register_cmd(
> > @@ -1279,5 +1699,22 @@ RTE_INIT(memory_telemetry)
> > rte_telemetry_register_cmd(
> > EAL_HEAP_INFO_REQ,
> handle_eal_heap_info_request,
> > "Returns malloc heap stats. Parameters: int
> heap_id");
> > + rte_telemetry_register_cmd(
> > + EAL_MEMSEG_LIST_ARR_REQ,
> > + handle_eal_memseg_list_array_request,
> > + "Returns hugepage list. Takes no parameters");
>
> "hugepage list" -> "array of memseg list IDs"
>
> > + rte_telemetry_register_cmd(
> > + EAL_MEMSEG_LIST_INFO_REQ,
> > + handle_eal_memseg_list_info_request,
> > + "Returns memseg list. Parameters: int
> memseg_list_id");
>
> "memseg list" -> "memseg list info"
>
> > + rte_telemetry_register_cmd(
> > + EAL_MEMSEG_INFO_REQ,
> handle_eal_memseg_info_request,
> > + "Returns memseg info. Parameter: int
> memseg_list_id,int memseg_id");
> > + rte_telemetry_register_cmd(EAL_ELEMENT_LIST_REQ,
> > + handle_eal_element_list_request,
> > + "Returns element info. Parameters: int heap_id, int
> > +memseg_list_id, int memseg_id");
>
> "element info" -> "array of heap element IDs".
>
> > + rte_telemetry_register_cmd(EAL_ELEMENT_INFO_REQ,
> > + handle_eal_element_info_request,
> > + "Returns element info. Parameters: int heap_id,
> memseg_list_id,
> > +memseg_id, start_elem_id, end_elem_id");
> > }
> > #endif
>
> Please make parameter descriptions consistent ("int x, int y" vs "int x, y").
Sure.
Hi Amit,
2022-10-21 19:26 (UTC+0000), Amit Prakash Shukla:
[...]
> > How does the user learn heap_id?
> > There probably should be /eal/heap_id returning a list of heap IDs.
>
> Request for list of active heap Id's is already present.
> " /eal/heap_list"
My bad!
> > > --> /eal/element_list,0,1,15
> > > {"/eal/element_list": {"Element_count": 52}}
> > >
> > > 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> > > <elem-start-id>,<elem-end-id>
> > > The command outputs element information like element start address,
> > > end address, to which memseg it belongs, element state, element size.
> > > User can give a range of elements to be printed.
> > > Example:
> > > --> /eal/element_info,0,1,15,1,2
> > > {"/eal/element_info": {"element.1": {"msl_id": 1, \
> > > "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> > > "memseg_end_addr": "0xb40000000", \
> > > "element_start_addr": "0xb201fe680", \
> > > "element_end_addr": "0xb20bfe700", \
> > > "element_size": 10485888, "element_state": "Busy"}, \
> > > "element.2": {"msl_id": 1, "ms_id": 15, \
> > > "memseg_start_addr": "0xb20000000", \
> > > "memseg_end_addr": "0xb40000000", \
> > > "element_start_addr": "0xb20bfe700", \
> > > "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> > > "element_state": "Busy"}, "Element_count": 2}}
> >
> > How this request is going to be used?
> > Elements don't have permanent IDs like MSL/memseg index or heap ID.
> > Heap layout may change between /eal/element_list and this request.
>
> Idea here was to print information related to memory element. This information
> Can be printed for a single element or for a range of elements.
I understand what this request does, the question is: what info the user has
initially, what they want to learn, and whether the request helps them.
For example, if the user wants to understand
which elements hold the known hugepage from being freed,
then your request is good as it is.
On the other hand, if the user initially has an address from some debug print
and wants to know if it's free, and if not, what's there,
then my suggestion about query by address is more suitable.
Or maybe both are good to have eventually.
> > Maybe instead there should be a filter by address with maybe a context
> > parameter (like "grep -C")?
>
> You mean that the user shall be able to grep for a memory address to check
> which element it belongs to ? If my understanding is correct, can I implement
> it as new request post rc2 and keep this request as-is?
Yes, this is what I mean.
AFAIU, rc1 is API freeze, and telemetry is API, so the new request must wait.
If the current request helps solving actual issues, of course let's have it.
Hi Dmitry,
Sure, will implement 2 new request (1. Msl properties, 2. Address lookup) in a separate patch.
I will work on other review suggestions as part of v6.
Thanks,
Amit Shukla
> -----Original Message-----
> From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Sent: Saturday, October 22, 2022 1:38 AM
> To: Amit Prakash Shukla <amitprakashs@marvell.com>
> Cc: Anatoly Burakov <anatoly.burakov@intel.com>; dev@dpdk.org; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; david.marchand@redhat.com;
> bruce.richardson@intel.com; ciara.power@intel.com
> Subject: Re: [EXT] Re: [PATCH v5 1/2] mem: telemetry support for memseg
> and element information
>
> Hi Amit,
>
> 2022-10-21 19:26 (UTC+0000), Amit Prakash Shukla:
> [...]
> > > How does the user learn heap_id?
> > > There probably should be /eal/heap_id returning a list of heap IDs.
> >
> > Request for list of active heap Id's is already present.
> > " /eal/heap_list"
>
> My bad!
>
> > > > --> /eal/element_list,0,1,15
> > > > {"/eal/element_list": {"Element_count": 52}}
> > > >
> > > > 5. /eal/element_info,<heap-id>,<memseg-list-id>,<memseg-id>, \
> > > > <elem-start-id>,<elem-end-id>
> > > > The command outputs element information like element start
> > > > address, end address, to which memseg it belongs, element state,
> element size.
> > > > User can give a range of elements to be printed.
> > > > Example:
> > > > --> /eal/element_info,0,1,15,1,2
> > > > {"/eal/element_info": {"element.1": {"msl_id": 1, \
> > > > "ms_id": 15, "memseg_start_addr": "0xb20000000", \
> > > > "memseg_end_addr": "0xb40000000", \
> > > > "element_start_addr": "0xb201fe680", \
> > > > "element_end_addr": "0xb20bfe700", \
> > > > "element_size": 10485888, "element_state": "Busy"}, \
> > > > "element.2": {"msl_id": 1, "ms_id": 15, \
> > > > "memseg_start_addr": "0xb20000000", \
> > > > "memseg_end_addr": "0xb40000000", \
> > > > "element_start_addr": "0xb20bfe700", \
> > > > "element_end_addr": "0xb215fe780", "element_size": 10485888, \
> > > > "element_state": "Busy"}, "Element_count": 2}}
> > >
> > > How this request is going to be used?
> > > Elements don't have permanent IDs like MSL/memseg index or heap ID.
> > > Heap layout may change between /eal/element_list and this request.
> >
> > Idea here was to print information related to memory element. This
> > information Can be printed for a single element or for a range of elements.
>
> I understand what this request does, the question is: what info the user has
> initially, what they want to learn, and whether the request helps them.
> For example, if the user wants to understand which elements hold the
> known hugepage from being freed, then your request is good as it is.
> On the other hand, if the user initially has an address from some debug print
> and wants to know if it's free, and if not, what's there, then my suggestion
> about query by address is more suitable.
> Or maybe both are good to have eventually.
>
> > > Maybe instead there should be a filter by address with maybe a
> > > context parameter (like "grep -C")?
> >
> > You mean that the user shall be able to grep for a memory address to
> > check which element it belongs to ? If my understanding is correct,
> > can I implement it as new request post rc2 and keep this request as-is?
>
> Yes, this is what I mean.
> AFAIU, rc1 is API freeze, and telemetry is API, so the new request must wait.
> If the current request helps solving actual issues, of course let's have it.
@@ -3,6 +3,7 @@
*/
#include <errno.h>
+#include <ctype.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
@@ -26,6 +27,7 @@
#include "eal_memcfg.h"
#include "eal_options.h"
#include "malloc_heap.h"
+#include "malloc_elem.h"
/*
* Try to mmap *size bytes in /dev/zero. If it is successful, return the
@@ -1113,11 +1115,17 @@ rte_eal_memory_init(void)
}
#ifndef RTE_EXEC_ENV_WINDOWS
-#define EAL_MEMZONE_LIST_REQ "/eal/memzone_list"
-#define EAL_MEMZONE_INFO_REQ "/eal/memzone_info"
-#define EAL_HEAP_LIST_REQ "/eal/heap_list"
-#define EAL_HEAP_INFO_REQ "/eal/heap_info"
-#define ADDR_STR 15
+#define EAL_MEMZONE_LIST_REQ "/eal/memzone_list"
+#define EAL_MEMZONE_INFO_REQ "/eal/memzone_info"
+#define EAL_HEAP_LIST_REQ "/eal/heap_list"
+#define EAL_HEAP_INFO_REQ "/eal/heap_info"
+#define EAL_MEMSEG_LIST_ARR_REQ "/eal/memseg_list_array"
+#define EAL_MEMSEG_LIST_INFO_REQ "/eal/memseg_list_info"
+#define EAL_MEMSEG_INFO_REQ "/eal/memseg_info"
+#define EAL_ELEMENT_LIST_REQ "/eal/element_list"
+#define EAL_ELEMENT_INFO_REQ "/eal/element_info"
+#define ADDR_STR 15
+
/* Telemetry callback handler to return heap stats for requested heap id. */
static int
@@ -1265,6 +1273,418 @@ handle_eal_memzone_list_request(const char *cmd __rte_unused,
return 0;
}
+static int
+handle_eal_memseg_list_array_request(const char *cmd __rte_unused,
+ const char *params __rte_unused,
+ struct rte_tel_data *d)
+{
+ struct rte_mem_config *mcfg;
+ int i;
+
+ rte_tel_data_start_array(d, RTE_TEL_INT_VAL);
+
+ rte_mcfg_mem_read_lock();
+ mcfg = rte_eal_get_configuration()->mem_config;
+
+ for (i = 0; i < RTE_MAX_MEMSEG_LISTS; i++) {
+ struct rte_memseg_list *msl = &mcfg->memsegs[i];
+ if (msl->memseg_arr.count == 0)
+ continue;
+
+ rte_tel_data_add_array_int(d, i);
+ }
+ rte_mcfg_mem_read_unlock();
+
+ return 0;
+}
+
+static int
+handle_eal_memseg_list_info_request(const char *cmd __rte_unused,
+ const char *params, struct rte_tel_data *d)
+{
+ struct rte_mem_config *mcfg;
+ struct rte_memseg_list *msl;
+ struct rte_fbarray *arr;
+ uint32_t ms_list_idx;
+ int ms_idx;
+
+ if (params == NULL || strlen(params) == 0)
+ return -1;
+
+ if (!isdigit(*params))
+ return -1;
+
+ ms_list_idx = strtoul(params, NULL, 10);
+ if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS)
+ return -1;
+
+ rte_tel_data_start_array(d, RTE_TEL_INT_VAL);
+
+ rte_mcfg_mem_read_lock();
+ mcfg = rte_eal_get_configuration()->mem_config;
+ msl = &mcfg->memsegs[ms_list_idx];
+ if (msl->memseg_arr.count == 0)
+ goto done;
+
+ arr = &msl->memseg_arr;
+
+ ms_idx = rte_fbarray_find_next_used(arr, 0);
+ while (ms_idx >= 0) {
+ rte_tel_data_add_array_int(d, ms_idx);
+ ms_idx = rte_fbarray_find_next_used(arr, ms_idx + 1);
+ }
+
+done:
+ rte_mcfg_mem_read_unlock();
+
+ return 0;
+}
+
+static int
+handle_eal_memseg_info_request(const char *cmd __rte_unused,
+ const char *params, struct rte_tel_data *d)
+{
+ struct rte_mem_config *mcfg;
+ uint64_t ms_start_addr, ms_end_addr, ms_size;
+ struct rte_memseg_list *msl;
+ const struct rte_memseg *ms;
+ struct rte_fbarray *arr;
+ char addr[ADDR_STR];
+ uint32_t ms_list_idx = 0;
+ uint32_t ms_idx = 0;
+ uint32_t msl_len;
+ char dlim[2] = ",";
+ char *token;
+ char *params_args;
+
+ if (params == NULL || strlen(params) == 0)
+ return -1;
+
+ /* strtok expects char * and param is const char *. Hence on using
+ * params as "const char *" compiler throws warning.
+ */
+ params_args = strdup(params);
+ token = strtok(params_args, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ ms_list_idx = strtoul(token, NULL, 10);
+ if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
+ free(params_args);
+ return -1;
+ }
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+ ms_idx = strtoul(token, NULL, 10);
+
+ free(params_args);
+
+ rte_mcfg_mem_read_lock();
+
+ mcfg = rte_eal_get_configuration()->mem_config;
+ msl = &mcfg->memsegs[ms_list_idx];
+ if (msl->memseg_arr.count == 0) {
+ rte_mcfg_mem_read_unlock();
+ return -1;
+ }
+
+ arr = &msl->memseg_arr;
+ msl_len = arr->len;
+
+ ms = rte_fbarray_get(arr, ms_idx);
+ if (ms == NULL) {
+ rte_mcfg_mem_read_unlock();
+ RTE_LOG(DEBUG, EAL, "Error fetching requested memseg.\n");
+ return -1;
+ }
+
+ ms_start_addr = ms->addr_64;
+ ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
+ ms_size = ms->hugepage_sz;
+
+ rte_mcfg_mem_read_unlock();
+
+ rte_tel_data_start_dict(d);
+ rte_tel_data_add_dict_int(d, "Memseg_list_index", ms_list_idx);
+ rte_tel_data_add_dict_int(d, "Memseg_index", ms_idx);
+ rte_tel_data_add_dict_int(d, "Memseg_list_len", msl_len);
+ snprintf(addr, ADDR_STR, "0x%"PRIx64, ms_start_addr);
+ rte_tel_data_add_dict_string(d, "Start_addr", addr);
+ snprintf(addr, ADDR_STR, "0x%"PRIx64, ms_end_addr);
+ rte_tel_data_add_dict_string(d, "End_addr", addr);
+ rte_tel_data_add_dict_int(d, "Size", ms_size);
+
+ return 0;
+}
+
+static int
+handle_eal_element_list_request(const char *cmd __rte_unused,
+ const char *params, struct rte_tel_data *d)
+{
+ struct rte_mem_config *mcfg;
+ struct rte_memseg_list *msl;
+ const struct rte_memseg *ms;
+ struct malloc_elem *elem;
+ struct malloc_heap *heap;
+ uint64_t ms_start_addr, ms_end_addr;
+ uint64_t elem_start_addr, elem_end_addr;
+ uint32_t ms_list_idx = 0;
+ uint32_t heap_id = 0;
+ uint32_t ms_idx = 0;
+ char dlim[2] = ",";
+ int elem_count = 0;
+ char *token;
+ char *params_args;
+
+ if (params == NULL || strlen(params) == 0)
+ return -1;
+
+ /* strtok expects char * and param is const char *. Hence on using
+ * params as "const char *" compiler throws warning.
+ */
+ params_args = strdup(params);
+ token = strtok(params_args, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ heap_id = strtoul(token, NULL, 10);
+ if (heap_id >= RTE_MAX_HEAPS) {
+ free(params_args);
+ return -1;
+ }
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ ms_list_idx = strtoul(token, NULL, 10);
+ if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
+ free(params_args);
+ return -1;
+ }
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ ms_idx = strtoul(token, NULL, 10);
+
+ free(params_args);
+
+ rte_mcfg_mem_read_lock();
+
+ mcfg = rte_eal_get_configuration()->mem_config;
+ msl = &mcfg->memsegs[ms_list_idx];
+ ms = rte_fbarray_get(&msl->memseg_arr, ms_idx);
+ if (ms == NULL) {
+ rte_mcfg_mem_read_unlock();
+ RTE_LOG(DEBUG, EAL, "Error fetching requested memseg.\n");
+ return -1;
+ }
+
+ ms_start_addr = ms->addr_64;
+ ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
+ rte_mcfg_mem_read_unlock();
+
+ rte_tel_data_start_dict(d);
+
+ heap = &mcfg->malloc_heaps[heap_id];
+ rte_spinlock_lock(&heap->lock);
+
+ elem = heap->first;
+ while (elem) {
+ elem_start_addr = (uint64_t)elem;
+ elem_end_addr =
+ (uint64_t)RTE_PTR_ADD(elem_start_addr, elem->size);
+
+ if ((uint64_t)elem_start_addr >= ms_start_addr &&
+ (uint64_t)elem_end_addr <= ms_end_addr)
+ elem_count++;
+ elem = elem->next;
+ }
+
+ rte_spinlock_unlock(&heap->lock);
+
+ rte_tel_data_add_dict_int(d, "Element_count", elem_count);
+
+ return 0;
+}
+
+static int
+handle_eal_element_info_request(const char *cmd __rte_unused,
+ const char *params, struct rte_tel_data *d)
+{
+ struct rte_mem_config *mcfg;
+ struct rte_memseg_list *msl;
+ const struct rte_memseg *ms;
+ struct malloc_elem *elem;
+ struct malloc_heap *heap;
+ struct rte_tel_data *c;
+ uint64_t ms_start_addr, ms_end_addr;
+ uint64_t elem_start_addr, elem_end_addr;
+ uint32_t ms_list_idx = 0;
+ uint32_t heap_id = 0;
+ uint32_t ms_idx = 0;
+ uint32_t start_elem = 0, end_elem = 0;
+ uint32_t count = 0, elem_count = 0;
+ char dlim[2] = ",";
+ char str[ADDR_STR];
+ char *params_args;
+ char *token;
+
+ if (params == NULL || strlen(params) == 0)
+ return -1;
+
+ /* strtok expects char * and param is const char *. Hence on using
+ * params as "const char *" compiler throws warning.
+ */
+ params_args = strdup(params);
+ token = strtok(params_args, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ heap_id = strtoul(token, NULL, 10);
+ if (heap_id >= RTE_MAX_HEAPS) {
+ free(params_args);
+ return -1;
+ }
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ ms_list_idx = strtoul(token, NULL, 10);
+ if (ms_list_idx >= RTE_MAX_MEMSEG_LISTS) {
+ free(params_args);
+ return -1;
+ }
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ ms_idx = strtoul(token, NULL, 10);
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ start_elem = strtoul(token, NULL, 10);
+
+ token = strtok(NULL, dlim);
+ if (token == NULL || !isdigit(*token)) {
+ free(params_args);
+ return -1;
+ }
+
+ end_elem = strtoul(token, NULL, 10);
+
+ free(params_args);
+
+ if (end_elem < start_elem)
+ return -1;
+
+ rte_mcfg_mem_read_lock();
+
+ mcfg = rte_eal_get_configuration()->mem_config;
+ msl = &mcfg->memsegs[ms_list_idx];
+ ms = rte_fbarray_get(&msl->memseg_arr, ms_idx);
+ if (ms == NULL) {
+ rte_mcfg_mem_read_unlock();
+ RTE_LOG(DEBUG, EAL, "Error fetching requested memseg.\n");
+ return -1;
+ }
+
+ ms_start_addr = ms->addr_64;
+ ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
+
+ rte_mcfg_mem_read_unlock();
+
+ rte_tel_data_start_dict(d);
+
+ heap = &mcfg->malloc_heaps[heap_id];
+ rte_spinlock_lock(&heap->lock);
+
+ elem = heap->first;
+ while (elem) {
+ elem_start_addr = (uint64_t)elem;
+ elem_end_addr =
+ (uint64_t)RTE_PTR_ADD(elem_start_addr, elem->size);
+
+ if (elem_start_addr < ms_start_addr ||
+ elem_end_addr > ms_end_addr) {
+ elem = elem->next;
+ continue;
+ }
+
+ if (count < start_elem) {
+ elem = elem->next;
+ count++;
+ continue;
+ }
+
+ c = rte_tel_data_alloc();
+ if (c == NULL)
+ break;
+
+ rte_tel_data_start_dict(c);
+ rte_tel_data_add_dict_int(c, "msl_id", ms_list_idx);
+ rte_tel_data_add_dict_int(c, "ms_id", ms_idx);
+ snprintf(str, ADDR_STR, "0x%"PRIx64, ms_start_addr);
+ rte_tel_data_add_dict_string(c, "memseg_start_addr", str);
+ snprintf(str, ADDR_STR, "0x%"PRIx64, ms_end_addr);
+ rte_tel_data_add_dict_string(c, "memseg_end_addr", str);
+ snprintf(str, ADDR_STR, "0x%"PRIx64, elem_start_addr);
+ rte_tel_data_add_dict_string(c, "element_start_addr", str);
+ snprintf(str, ADDR_STR, "0x%"PRIx64, elem_end_addr);
+ rte_tel_data_add_dict_string(c, "element_end_addr", str);
+ rte_tel_data_add_dict_int(c, "element_size", elem->size);
+ snprintf(str, ADDR_STR, "%s", elem->state == 0 ? "Free" :
+ elem->state == 1 ? "Busy" : elem->state == 2 ?
+ "Pad" : "Error");
+ rte_tel_data_add_dict_string(c, "element_state", str);
+
+ snprintf(str, ADDR_STR, "%s.%u", "element", count);
+ if (rte_tel_data_add_dict_container(d, str, c, 0) != 0) {
+ rte_tel_data_free(c);
+ break;
+ }
+
+ elem_count++;
+ count++;
+ if (count > end_elem)
+ break;
+
+ elem = elem->next;
+ }
+
+ rte_spinlock_unlock(&heap->lock);
+
+ rte_tel_data_add_dict_int(d, "Element_count", elem_count);
+
+ return 0;
+}
+
RTE_INIT(memory_telemetry)
{
rte_telemetry_register_cmd(
@@ -1279,5 +1699,22 @@ RTE_INIT(memory_telemetry)
rte_telemetry_register_cmd(
EAL_HEAP_INFO_REQ, handle_eal_heap_info_request,
"Returns malloc heap stats. Parameters: int heap_id");
+ rte_telemetry_register_cmd(
+ EAL_MEMSEG_LIST_ARR_REQ,
+ handle_eal_memseg_list_array_request,
+ "Returns hugepage list. Takes no parameters");
+ rte_telemetry_register_cmd(
+ EAL_MEMSEG_LIST_INFO_REQ,
+ handle_eal_memseg_list_info_request,
+ "Returns memseg list. Parameters: int memseg_list_id");
+ rte_telemetry_register_cmd(
+ EAL_MEMSEG_INFO_REQ, handle_eal_memseg_info_request,
+ "Returns memseg info. Parameter: int memseg_list_id,int memseg_id");
+ rte_telemetry_register_cmd(EAL_ELEMENT_LIST_REQ,
+ handle_eal_element_list_request,
+ "Returns element info. Parameters: int heap_id, int memseg_list_id, int memseg_id");
+ rte_telemetry_register_cmd(EAL_ELEMENT_INFO_REQ,
+ handle_eal_element_info_request,
+ "Returns element info. Parameters: int heap_id, memseg_list_id, memseg_id, start_elem_id, end_elem_id");
}
#endif