RE: [EXT] Re: [PATCH v7] mem: telemetry support for memseg and element information

[Amit Prakash Shukla <amitprakashs@marvell.com>]

Thanks Anatoly for review and feedback. I will make the suggested changes in next version of the patch.

Thanks,
Amit Shukla

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Tuesday, May 16, 2023 4:17 PM
> To: Amit Prakash Shukla <amitprakashs@marvell.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
> Subject: [EXT] Re: [PATCH v7] mem: telemetry support for memseg and
> element information
> 
> External Email
> 
> ----------------------------------------------------------------------
> On 10/25/2022 2:02 PM, Amit Prakash Shukla wrote:
> > 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_lists
> > The command displays the memseg list from which the memory has been
> > allocated.
> > Example:
> > --> /eal/memseg_lists
> > {"/eal/memseg_lists": [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,0
> > {"/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, "Start_addr": "0x101600000", \
> > "End_addr": "0x101800000", "Size": 2097152, "Hugepage_size": 2097152,
> > \
> > "Socket_id": 0, "flags": 0}}
> >
> > --> /eal/memseg_info,0,15
> > {"/eal/memseg_info": {"Memseg_list_index": 0, "Memseg_index": 15, \
> > "Start_addr": "0x102000000", "End_addr": "0x102200000", \
> > "Size": 2097152, "Hugepage_size": 2097152, "Socket_id": 0, "flags":
> > 0}}
> >
> > 4. /eal/mem_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/mem_element_list,0,0,10
> > {"/eal/mem_element_list": {"Element_count": 52}}
> >
> > --> /eal/mem_element_list,0,1,15
> > {"/eal/mem_element_list": {"Element_count": 52}}
> >
> > 5. /eal/mem_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/mem_element_info,0,0,15,1,2
> > {"/eal/mem_element_info": {"element_1": {"msl_id": 0, "ms_id": 15, \
> > "memseg_start_addr": "0x102000000", "memseg_end_addr":
> "0x102200000",
> > \
> > "element_start_addr": "0x102000b00", "element_end_addr":
> > "0x102003380", \
> > "element_size": 10368, "element_state": "Busy"}, "element_2": \
> > {"msl_id": 0, "ms_id": 15, "memseg_start_addr": "0x102000000", \
> > "memseg_end_addr": "0x102200000", "element_start_addr":
> "0x102003380",
> > \
> > "element_end_addr": "0x102005c00", "element_size": 10368, \
> > "element_state": "Busy"}, "Element_count": 2}}
> >
> > Signed-off-by: Amit Prakash Shukla <amitprakashs@marvell.com>
> > ---
> 
> 
> 
> > +	ms_start_addr = ms->addr_64;
> > +	ms_end_addr = (uint64_t)RTE_PTR_ADD(ms_start_addr, ms->len);
> > +	ms_size = ms->len;
> > +	hugepage_size = ms->hugepage_sz;
> > +	ms_socket_id = ms->socket_id;
> > +	ms_flags = ms->flags;
> > +
> > +	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);
> > +	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_u64(d, "Size", ms_size);
> > +	rte_tel_data_add_dict_u64(d, "Hugepage_size", hugepage_size);
> 
> Seems like rte_tel_data_add_dict_u64 is deprecated now.
> 
> I would also suggest outputting IOVA address for memsegs as well (unless it's
> set to RTE_BAD_IOVA, in which case it should say something meaningful
> too).
> 
> Otherwise (and with above changes), LGTM
> 
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> --
> Thanks,
> Anatoly