From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 3BED4A0032; Fri, 21 Oct 2022 21:26:49 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15F27400D6; Fri, 21 Oct 2022 21:26:49 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id 7ADE040042 for ; Fri, 21 Oct 2022 21:26:47 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 29L8pfoA005519; Fri, 21 Oct 2022 12:26:46 -0700 Received: from nam04-mw2-obe.outbound.protection.outlook.com (mail-mw2nam04lp2168.outbound.protection.outlook.com [104.47.73.168]) by mx0b-0016f401.pphosted.com (PPS) with ESMTPS id 3kb125fj8k-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 21 Oct 2022 12:26:46 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=UfZ60/MUvfAiZxfGHZUXzbe2NiCihCjN11DEGKh5Mn/S/WqAjOY5GPGd3SLtG74hsIb5VoOgxPeK7dUes8mNVYAa6+fwjCcCXwtvRC3X9sa0coeuLd5/i/w4d/T/hV6tOA5J1OXJMnE5pnZKd+4CCyI/3jK2XmW3V+JSDyx2L3Vs8/GwB9z9eMj5E2YpokEakqGAieqFQRphlsco94x5c+PiDr3ffdvxUS4RBpE0uKXjKBjDnYUQy6owdHmsmJWy1bAAcvIIFVepj0j9uoEMIMiHouKsLPd5K/jjjX/1CV0pbifej5nsCMtvPCSRozMPD7O7PRvm66cR1nOl+oBayg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=OeAH3hZ7/54/sTO84YKD47iKmWEvTwGSCmY3AU9JEj8=; b=mOaORH9oW3eo0ngA4rtnIjveql0Urfn76WoM/Kj3CgqxDwdWJ0w8Yh9W6XUY0c832kEGDvvX01jZumFTt0jx6dv216m/XRK0zwp2DPI2hHbnjPTHGp0OuxPp+SxuIdHEKpAkRHdp9Ph3iSoFCrkCngArusUNfEKnvNcgX8YUl1GGF69oameJAWpBp3vHa6zzMYhVcfqH0+uyvl2fu62keNsQU/FhQ591mvQVJ2uLZfh+SDPhxeJbcwj3xmKPlK2ESIDa5n4KXmRfGPQOVdepd+/26oBwsPfzzG/1gDxudFRp2LV9gaG9/2PE6jCHdlPUEsExiPc5LxRfCqlj1lchPg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com; dkim=pass header.d=marvell.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OeAH3hZ7/54/sTO84YKD47iKmWEvTwGSCmY3AU9JEj8=; b=NGNZwI3Wiz8dYFIRQHxl76QV3gNjwhV0oTGHRKiAKUvc5z7Sa6+YGdnntKnBi4x764beV5YJ4/NX+hZIzRyVUxhfJqgjeOpeGxmcws5IQN2GuWGzMJnwdZ22XFmFwySi1OFWRxcNZk6eeoxJ/MnYipuZ00sLVsCYV/bMhgQ9EjA= Received: from PH0PR18MB5167.namprd18.prod.outlook.com (2603:10b6:510:168::7) by CO3PR18MB4960.namprd18.prod.outlook.com (2603:10b6:303:176::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5723.29; Fri, 21 Oct 2022 19:26:42 +0000 Received: from PH0PR18MB5167.namprd18.prod.outlook.com ([fe80::babe:8efb:2ef6:1acf]) by PH0PR18MB5167.namprd18.prod.outlook.com ([fe80::babe:8efb:2ef6:1acf%6]) with mapi id 15.20.5723.032; Fri, 21 Oct 2022 19:26:42 +0000 From: Amit Prakash Shukla To: Dmitry Kozlyuk CC: Anatoly Burakov , "dev@dpdk.org" , Jerin Jacob Kollanukkaran , "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 Thread-Topic: [EXT] Re: [PATCH v5 1/2] mem: telemetry support for memseg and element information Thread-Index: AQHY0/i09WhvvvieGEa9Y8zxwl1gYK4XSaQAgAHkHLA= Date: Fri, 21 Oct 2022 19:26:42 +0000 Message-ID: References: <20220525103352.1806937-1-amitprakashs@marvell.com> <20220929114313.1346972-1-amitprakashs@marvell.com> <20221020144046.13b98d76@sovereign> In-Reply-To: <20221020144046.13b98d76@sovereign> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-publictraffictype: Email x-ms-traffictypediagnostic: PH0PR18MB5167:EE_|CO3PR18MB4960:EE_ x-ms-office365-filtering-correlation-id: 66d2de68-30a4-4f77-7a7a-08dab39a2f26 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: htmrjiDBtwypf3mvYZti6WwsEDVkVND27gb7dCzOG6W2wY8VSXqLN2omoOC8XhECne30Hf3oUfu7PjvEaJIsoH1KF0P7bbmrDu5ZtJTvw9s8j+3b7HSG4kXs8n1ImMynZluNJs7jxkilaM4dgPvsBiv9KEWShN4qxRm+ZKVjpF9mv+byYO68pdb5Oc92gvbB+S3pe7ghnU3SoVk5oCCqBd2y9u54hjg3YBC/lNfmPz0PfC8aUAQjDzKmTUXIodHsAnO8lmrdodKWPnn88SKunLbedv+fD2rr4FWyCSAZX119iI8JQoLYqkZs8w9rvyHyXnurAgudeMaJU81SGh/gBLOZAlKzWANgpzQtRB9TZHMIQRlHelXjnQcJGP9ykt531SjrN2XA6CjX49UVflVjUaiW8B/YgAIbg0DfDDzjRCzGDSB01qcSUXVGxfruA45dp6uKVes3A9mxrUiE7coI1CsCfn+2faHDejpZ9+mPw6VLwyxoJHIpUGtTCAitKQQUgOVS6ek07jVe6aWu4/cyVT1WcykXc+S7M4BpN28IbWCdWbSIWR+A9LhnwicZ1/7TKzWiii2Kr5YcQL+rleJGaKm6pshjGfBjAwDaRzBp5vFXYNjo9wBZ/iEIRBZ7bnGDTF8Fd0CAzmp4oApNyLPyr8KtMu7tEjZwmCuDMH0Mh2oA7UqY8wu7P/29ehW6hxb6ShMeHLWBnk3Db9ANs1ZIKdkeKukbHKgYwBSm77QuhoQPbn8Q4JxBNZdwEvg6ly0Q5vqJlh/RHhr03kLUDJld+g== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:PH0PR18MB5167.namprd18.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230022)(4636009)(136003)(376002)(346002)(39860400002)(396003)(366004)(451199015)(316002)(186003)(86362001)(33656002)(6506007)(7696005)(5660300002)(26005)(52536014)(8936002)(2906002)(53546011)(6916009)(83380400001)(64756008)(66556008)(66446008)(66476007)(8676002)(55016003)(66946007)(54906003)(76116006)(71200400001)(9686003)(41300700001)(38100700002)(122000001)(4326008)(38070700005)(478600001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?rGFLM5SP5/iJfmwvbfAASs443PYyDVT8hxMTNPbje74ZOfQSsffIGG8R1Van?= =?us-ascii?Q?UgYVVB4lkCovoF9s0bxxiFIfj3YycxoSmPEVDDBaaamfqLmgQLYK2o4JNozq?= =?us-ascii?Q?0yYIiX0QV/q9gpdIRZfmlWg5r385OgtoNjpB4S/xXeTDCHRcFOyIZZ2RPl3w?= =?us-ascii?Q?2gCQbr6rlF4JxGjNSQe9oYouL5p1AljIfj2KItRjyeT3wa+5gKRB+SB7tiQV?= =?us-ascii?Q?E+6Qa/LXqA9p65PyZYqaxNDH4SCtKCtdtcMSLeQUIpJyLPAurjDxIEbYNAh+?= =?us-ascii?Q?maO7zh9KI92m5vKj/JvMmBJ35cnqcbwxrVj4icehR5f7x25hdyiHFITyEjP+?= =?us-ascii?Q?PBYg4qEVzIilr8eK+V/2jYvO3WhdAvkMbRDQuSZrAlD1U4df8D2xzl71TS3G?= =?us-ascii?Q?tH7XLr1APtK5MoEtzQul8vEWTApQdiawFXmAGxlb3YDTPHRGoNelVtfSttFv?= =?us-ascii?Q?a246yv2Bvz+6zlVId5pCJoSvAV2AACyMx7vpFlf+6FPhWBwNTzks2dqUM1Qa?= =?us-ascii?Q?9owTp78S19+hj5nMPsMhAyt36OkTvmkVq/akA2sAOlNSA4gJrUq4J1xmJqPj?= =?us-ascii?Q?sYj/w7SoHJD1u0o/jpL2IXItT0ldBRF20gSh+6J+Eezvd1WignKmT00Fix2V?= =?us-ascii?Q?zFqWhMo26sI6wkdZcaPr4eijWXpKH93nAzF/VdR3XUlmNEAS0Bt2qaj0v16S?= =?us-ascii?Q?Pi/EvBYXath+sfktnBzzuQS50oF9GTeD77vHU4gpm0GQNo9denBvCfknVAHC?= =?us-ascii?Q?LC0GBV3loZcYNpN7Z6EWGnbDB3Mp+LjPVLB86Nj4xiHJbNp1HZr1NkBnrFSS?= =?us-ascii?Q?rIX0Mn1G6Ez5IaoejqWlzxEuSnpbNFMMwmir66h+1Z+C+iWNRkgHiWHZY+VN?= =?us-ascii?Q?Es2Bp7I40FrnId4X5L7UMB1DfGgEuAMfmTADy55Wt/M5fWPGHgU/2DN87y76?= =?us-ascii?Q?JB48mlm8GKNrnEIHMtw5lg/02gbrzLZIOWhmXzqQUyxnvSyB+7Wj2D8c5UdB?= =?us-ascii?Q?Mt3IGGFaQdYsPYGj5v8sxufqAJeM61jfXlZ/3wmiY/tbDfq5xOaJ3n2OVI1m?= =?us-ascii?Q?YB8lZYJHFLNadh6wEOKJFKZLWN0fl/5M77y75n6w1/nu/pvHsnqSf6VEolwS?= =?us-ascii?Q?+4nG2kWHmGFBa7oK3UZAIf2VBHDPXg0xEwTyZxmTRFml4ngWqMTOdol6XsER?= =?us-ascii?Q?/nT8z1e7m6AlN7DO2hyV7ZkoDjiYSNKQfjJ04boLRV5dUaDMOqCJr0tvgnG7?= =?us-ascii?Q?2EZThMNOV2ZqEXS+a/XwYX2Pk0e/PaSYWWjBTk9WV7iaWn18o3VmRBwp6mxc?= =?us-ascii?Q?/7GqVg1/7Tks9C4V9KdttVFk/fbuL3l/wQNjJn+8EWcnHE2vRQWCAxM750MS?= =?us-ascii?Q?LaZg/PSmCAGvCDgkhaG4V1JZocIOKNHvft7uE26V4Lb5TNf7qvB69Kb82Y/b?= =?us-ascii?Q?QtSBcbHklRcPOn253XOt2616xEWTy6OgXM7Gtl5saj52p1hoRLFIrvd3ofQm?= =?us-ascii?Q?osy25j9Nk8om2BJIJYHwSaOX2avowp8pOARb0dp24yN4/5Y38FAqNTXQJvp1?= =?us-ascii?Q?aR2TH42nTyVmJcEe3WO3F/DzgoIrDtDBHaSOgEK8?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: marvell.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB5167.namprd18.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 66d2de68-30a4-4f77-7a7a-08dab39a2f26 X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Oct 2022 19:26:42.3420 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: yWy02sC2C8d0mmP/eJpImIMGURew6vDNO+M37LYOW1UnC5O2CBfLTelMyanr+YuDG8zZaNR1W6PIaamtqdpsWrY1uOWVSlN7v+C6VSwMQ3M= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO3PR18MB4960 X-Proofpoint-GUID: bgrsWFvNn_poIGFII5BUj6IE1kas4dPR X-Proofpoint-ORIG-GUID: bgrsWFvNn_poIGFII5BUj6IE1kas4dPR X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-10-21_04,2022-10-21_01,2022-06-22_01 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Thanks Dmitry for the feedback. Please find my reply in-line. > -----Original Message----- > From: Dmitry Kozlyuk > Sent: Thursday, October 20, 2022 5:11 PM > To: Amit Prakash Shukla > Cc: Anatoly Burakov ; dev@dpdk.org; Jerin > Jacob Kollanukkaran ; 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 >=20 > External Email >=20 > ---------------------------------------------------------------------- > 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, > > 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]} >=20 > MSL has more properties worth reporting. > Also note that by default >=20 > #define RTE_MAX_MEMSEG_PER_LIST 8192 >=20 > 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 thi= s > 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. >=20 > > 3. /eal/memseg_info,, > > 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}} >=20 > "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,=20 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. >=20 > > > > --> /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,,, > > 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}} >=20 > 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.=20 " /eal/heap_list" >=20 > Please use a consistent naming scheme for requests returning ID lists. > Currently MSL have "_array" suffix but memsegs and elements don't. Sure >=20 > > --> /eal/element_list,0,1,15 > > {"/eal/element_list": {"Element_count": 52}} > > > > 5. /eal/element_info,,,, \ > > , > > 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}} >=20 > 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 informat= ion 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 impleme= nt 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. >=20 > [...] >=20 > > +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 =3D 0; > > + uint32_t ms_idx =3D 0; > > + uint32_t msl_len; > > + char dlim[2] =3D ","; > > + char *token; > > + char *params_args; > > + > > + if (params =3D=3D NULL || strlen(params) =3D=3D 0) > > + return -1; > > + > > + /* strtok expects char * and param is const char *. Hence on using > > + * params as "const char *" compiler throws warning. > > + */ > > + params_args =3D strdup(params); >=20 > Please check the allocation result hear and in the rest of the handlers. Sure, will do it in v6. >=20 > It would be nice to have a local helper to parse N integer params, this w= ould > reduce and simplify the code: >=20 > static int > parse_params(const char *params, int *vals, size_t vals_n); >=20 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"); >=20 > "hugepage list" -> "array of memseg list IDs" >=20 > > + rte_telemetry_register_cmd( > > + EAL_MEMSEG_LIST_INFO_REQ, > > + handle_eal_memseg_list_info_request, > > + "Returns memseg list. Parameters: int > memseg_list_id"); >=20 > "memseg list" -> "memseg list info" >=20 > > + 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"); >=20 > "element info" -> "array of heap element IDs". >=20 > > + 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 >=20 > Please make parameter descriptions consistent ("int x, int y" vs "int x, = y"). Sure.