[PATCH v5 1/2] mem: telemetry support for memseg and element information
Dmitry Kozlyuk
dmitry.kozliuk at gmail.com
Thu Oct 20 13:40:46 CEST 2022
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").
More information about the dev
mailing list