Received-SPF: pass (google.com: domain of linux-kernel+bounces-103547-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 14 Mar 2024 17:07:24 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
To: Ilpo =?utf-8?B?SsOkcnZpbmVu?= <ilpo.jarvinen@linux.intel.com>
CC: <linux-kselftest@vger.kernel.org>, Reinette Chatre
	<reinette.chatre@intel.com>, Shuah Khan <shuah@kernel.org>, Babu Moger
	<babu.moger@amd.com>, Fenghua Yu <fenghua.yu@intel.com>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 08/13] selftests/resctrl: Add ->init() callback into
 resctrl_val_param
Message-ID: <jzyc3lehsspybfa7fjzpwwcw2lgyfftsyqysqmpbrpq37aggln@cbjul5wgozu7>
References: <20240311135230.7007-1-ilpo.jarvinen@linux.intel.com>
 <20240311135230.7007-9-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20240311135230.7007-9-ilpo.jarvinen@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?9cLJ+S6nqgZdKip+nZXGkkyvC1zpyTH91uRkODLyh2hjEFz6zx7KlMVwk6?=
 =?iso-8859-1?Q?JYOokydRtNOsGz3KAhqMqoek6Lvf7SnJZjoetePKF7urZwK5wJuke+w/f/?=
 =?iso-8859-1?Q?LpaCUNksXl6AZwAPg1uEn+om7ABpPWb5wTmFQ9q5QAR84x5PZAUDa4tPtN?=
 =?iso-8859-1?Q?pt6Hc0RhcPHQQgqkVt43cPX9VAfHxmS9idonC7Fqlb1kQ7yUOd4urYaJv8?=
 =?iso-8859-1?Q?6aY+u0lI/kb7YOJ/oBfhxsdGP0xZVPO/MqFG0T+9RcI6mp0tD8xVSQ/n7k?=
 =?iso-8859-1?Q?VH3bwTb3J8a/4TF/KIU0ragzFuYyxzxFrd2Qq0ZpFqytcPg5XQbHIlrWoe?=
 =?iso-8859-1?Q?9Ap0WKjx1+nsWtOgF0+pm0p0ihpEQcgBT9yTG0jKcVYgsQ8iIl+GogxK49?=
 =?iso-8859-1?Q?hebL1qPXwNDg0NmfLl7m8novAzsukEvECfhA+D8a0fhWt/dMVEcE3WNber?=
 =?iso-8859-1?Q?6ZWRA8t9ztQO2NNfp4honcz6LUUSGHAznvUT3rxcvyreq6QiSxZ5TWCy6W?=
 =?iso-8859-1?Q?+QS3A2JgIBW0HMK59+R2VXPAib3VpIpKy7l5QMh1vQH4dV3U4dN86S8JwF?=
 =?iso-8859-1?Q?cj1T97QUoGFQub1k9FY5yf9q+wsHA5X5gL82htz+/9bVfCIWjefpQAMURO?=
 =?iso-8859-1?Q?jA5I5DroGGUQ3YLGtdTyaNhDc4rl3TfoWY90OzXUxcMIj0k+Mxie7VaXtx?=
 =?iso-8859-1?Q?bvo3hOVy9Ca4nFUUW+bD2org1zt/XnlLtl5zWCjPUU7Wkea4MvDg8bAS6H?=
 =?iso-8859-1?Q?+r9qdLj7cvLP5023vu0ZgW/fWTqXH6qgvBRX4uKBxjJ3iH7Rvd8thsdpQ4?=
 =?iso-8859-1?Q?dFxINFkFdFEtP+s7TpQIMPKdN07T/ipzsGSueNsX4mYkxXoMqlHvBQHIMC?=
 =?iso-8859-1?Q?dLH28PV2pGZ9eiOtZXWC6N/O6HTxDAouSYPZpoFS4OR4oPXSBoRfVxAMMk?=
 =?iso-8859-1?Q?aAGMMFFuUxFWkVl+1r7RJ6TCk57L9rwKaggoIndauV32H3v6vWn9IxMGZI?=
 =?iso-8859-1?Q?dRJEwd9BopmxdtLvYZhnMEWiNsHEyAHkjHoEt8OXdPGW+CExKzqarMcqmX?=
 =?iso-8859-1?Q?c4s0zDS/vQMrQZcivTnJG6aNVSQ7sGZyLsbNPtGv2TDzqd3TZMT0UpZTWy?=
 =?iso-8859-1?Q?OOFpjHDd4TjxXixUBrR7ixciNqfYUGcr8ZJXxw9G9Hhp/3h7ml0t4tsfqd?=
 =?iso-8859-1?Q?ftDdUacZVXApdjwQxSRjd1kx7Pjb9VzbK2qetciR5YBXBdpfIMogWr3h58?=
 =?iso-8859-1?Q?EeLp8djH7IQpCZ7ZPsmpggAGFCgtXUHK2mjIr7Uk/k6O0utLFzFdhKSzkd?=
 =?iso-8859-1?Q?/1zNyW8G4CKhg7dyDEbXMxaq3ObAfJG4VO7okIQsmVigN3LAu1pTg0mtHy?=
 =?iso-8859-1?Q?763Pw2NQPITBRmoS7PDNPrI34WcRqQ4/shCV/CPbIZFWTxcckMXURitEcq?=
 =?iso-8859-1?Q?MN/BOR2qsNT6YNekJ3iDd1BWevy3Bh/8w34aDf97Yz05qJ2vsYH36iryL/?=
 =?iso-8859-1?Q?tYELnsMgaP8jfg7Hoh+LPj3TCjvolSXsqBccHjgyKAYJrh4eIjjhITXytX?=
 =?iso-8859-1?Q?/gceeaDogOv6KaFER0WP3jk4O9VEhSBSluFLfI+Lkdxbwg6y9T5KM/04Tv?=
 =?iso-8859-1?Q?/2Nq7FU0mzBNFZWov6xCKXmbW8PJ7fen6WY+YWJvbdU98qJE+Ix3MFLozI?=
 =?iso-8859-1?Q?sC2RQqyr4RFcwnEeUug=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 20750c78-9397-4a97-0611-08dc4440dc7c
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Mar 2024 16:07:35.0634
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: n6EajvMM3zgBYkjIwP/pEQMwugw2KW/9+0khAND/gQyKovwLx9R3ZH2q/36d3NAaEGAeN6XVb72V4IkjB8sKzTfOMAsz1jZ2VeRetNw45d0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW6PR11MB8389
X-OriginatorOrg: intel.com

Hi again :)

On 2024-03-11 at 15:52:25 +0200, Ilpo J?rvinen wrote:
>diff --git a/tools/testing/selftests/resctrl/mbm_test.c b/tools/testing/selftests/resctrl/mbm_test.c
>index 17398cd3aace..ffbfcecf9bd6 100644
>--- a/tools/testing/selftests/resctrl/mbm_test.c
>+++ b/tools/testing/selftests/resctrl/mbm_test.c
>@@ -8,12 +8,19 @@
>  *    Sai Praneeth Prakhya <sai.praneeth.prakhya@intel.com>,
>  *    Fenghua Yu <fenghua.yu@intel.com>
>  */
>+#include <limits.h>
>+
> #include "resctrl.h"
> 
> #define RESULT_FILE_NAME	"result_mbm"
> #define MAX_DIFF_PERCENT	8
> #define NUM_OF_RUNS		5
> 
>+#define CON_MON_MBM_LOCAL_BYTES_PATH \
>+	"%s/%s/mon_groups/%s/mon_data/mon_L3_%02d/mbm_local_bytes"
>+
>+static char mbm_total_path[PATH_MAX];
>+
> static int
> show_bw_info(unsigned long *bw_imc, unsigned long *bw_resc, size_t span)
> {
>@@ -86,6 +93,20 @@ static int check_results(size_t span)
> 	return ret;
> }
> 
>+static int set_mbm_path(const struct resctrl_val_param *param, int domain_id)
>+{
>+	int ret;
>+
>+	ret = initialize_mem_bw_imc();

I just noticed this. Since there is not only path stuff here but also some imc
logic maybe the function names could be changed? Something like

	set_mbm_path -> init_mbm

The same could apply for all these init functions or at least the mba one.

>+	if (ret)
>+		return ret;
>+
>+	sprintf(mbm_total_path, CON_MON_MBM_LOCAL_BYTES_PATH,
>+		RESCTRL_PATH, param->ctrlgrp, param->mongrp, domain_id);
>+
>+	return 0;
>+}
>+
>

-- 
Kind regards
Maciej Wiecz?r-Retman