Received-SPF: pass (google.com: domain of linux-kernel+bounces-174970-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <eb80df2b-d6ba-4171-ad05-c76c218b0e3a@intel.com>
Date: Thu, 9 May 2024 14:11:10 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/4] selftests/resctrl: Pass sysfs controller name of
 the vendor
To: Babu Moger <babu.moger@amd.com>, <fenghua.yu@intel.com>,
	<shuah@kernel.org>
CC: <linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
	<ilpo.jarvinen@linux.intel.com>, <maciej.wieczor-retman@intel.com>,
	<peternewman@google.com>, <eranian@google.com>
References: <cover.1708637563.git.babu.moger@amd.com>
 <cover.1714073751.git.babu.moger@amd.com>
 <406d1fbe6b480fbe13fd0b8c432cdc517a7bd8c1.1714073751.git.babu.moger@amd.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <406d1fbe6b480fbe13fd0b8c432cdc517a7bd8c1.1714073751.git.babu.moger@amd.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?K3E4QVRIRlEzdDJodVdNWVBML0JROHFyQ3M1ak5ycEVkY2lvUVdYN3FxTXdL?=
 =?utf-8?B?RVdHK3pYZUZlSU4wakFqdUQxaFNuU3Y3a3lPZzF4UkZHWjgrckJ1YVl0R3Yz?=
 =?utf-8?B?QjBja2ZmdkE2MmJncE1LVEc2NHlnQnIyYk54SWZjN2gvWng3S0p5NDVxQ2VM?=
 =?utf-8?B?blpCdmNHR2p2VGZNdTlFWFdJUkVTWU1Qc3FoamVCTGU2aTFaMng1RWRzL3RP?=
 =?utf-8?B?REQwN1dpLzlZTXB6NlY5WHhHMmFlamlJTDE4NFRWdGVnazNxUlN3NVVqUHo1?=
 =?utf-8?B?MDIrZjZEajVUTy90YVFFcnJSeGJFOG5iOGxWQVJ2NnZDdWVRRUg3YllobDlK?=
 =?utf-8?B?Y2prVVk4cmZiQ0o0dW1abGI3ZmpkazVQbm5hZVFtdHhsWlhUblM0TUxNeEJF?=
 =?utf-8?B?dUNVbzltMDk1T0V1SVBqSXNUL0krdU5heEZ6R3paTWRSUDhmWGdhSXMyTEVV?=
 =?utf-8?B?ZDZZK3ZzV0hSWkxabkZPaEJPbWoyNTI2VTlNVk9uWmtka2huYWpLRytUcStQ?=
 =?utf-8?B?TzhXZkgvcEo5amUyc09OaURhWndPNkRDRldyMUJNRU9TT0VsRGhnTS94WHZ1?=
 =?utf-8?B?bEtCNlRQWmNtcjNzaXZEMnlOV1JPdGZvcFVpMFlGckZjNXNXVmg3Q2hZOFJW?=
 =?utf-8?B?WUhhaUlkM0x1Qjhhd1JZWGVqRmVOQVF1c3IxMFhEOXFwMDJINmZUaUpZVXZm?=
 =?utf-8?B?OE9QVis0a0ZVclJOdVBhdzN0SHRkV2JqNUdYak9naUU0NlVkNlBoZkppUTl5?=
 =?utf-8?B?T240RXBnZzc5dTNuYnZSeFJNQkJUbXFKUlh3RnFSNzI2ZU5sVjUwaVZWV2dt?=
 =?utf-8?B?Tm9DY1RBQXpIMSs2Tkt1cGlwb0FVenJXK1A1ZnZMZHFCcjBFZ2VGMVptVk5N?=
 =?utf-8?B?RHYwREVGNUw5SGxxdkNpZ0ozZkhlK1NaaG91Y1o1MkJ3RlU2bFFuT1N1dEtQ?=
 =?utf-8?B?eUtEc3o4MG1HQWROalpzWC9XblU1Um9FMTlSek1uTFl4S1ZMNEZOSGo1WHBt?=
 =?utf-8?B?bFVpWFUzVVZJbTlselZQMGpSNTE2ZXBNYTVRdzk0VFlqOVpCRmZIV2FDallh?=
 =?utf-8?B?eEpnM0dTKzE5RXhuQ2xRTUVrR0E0d1U1NkZrVlFsZnlBRENYc1R1ZEVROHE2?=
 =?utf-8?B?UkJubnlob1dzTVh3d2Y1b2szbVhJM3BncDdxdWJMeE9ZYU12K0xSSkxOZlR0?=
 =?utf-8?B?ZVJxL0lzV0ttNTYrcFJ0Vk5UNjRVdUhSWU04MWZkMEJaeWNrWGNFVGx4a0M4?=
 =?utf-8?B?STVZYjJmTkMyN2IyR2FwYWJMa0k5dnhLWGVtL2paeG15OVBPSzZGZE5oK0FB?=
 =?utf-8?B?VWlDaTF3bkM5d1dQQnVYWEFjcXBOQ0xyWmlqUG5SN1N2SHRmb3BZTlhrK0Mz?=
 =?utf-8?B?ckgyVUpnMUdzUmR0ajIzZytRbENpelpNZzJvV3lrenFkQVRLZjJMbWdIWGJy?=
 =?utf-8?B?Wk1QWDNuWGJxOVFsSHRtclNsUmdDL28yUzMxWUpla29yZVJSQUN6UUxvMzB4?=
 =?utf-8?B?RWtoY01ZWmFiemRaUHhsZHFaM3ROQWs0dVI2REkvRjBIbTQzdzN5Ukh4MDNv?=
 =?utf-8?B?eitURUQrOUJEc3BGRUgwTG1oUGJNNXJYRkZPbmwzR0gvakcvNFZlZEpaS1A5?=
 =?utf-8?B?Z0ZScFZ1YVhtZFhaVFdhNmYvTXhFN1BjMG1Ic3liLzVVZUx5c0M4cHBUd2Z0?=
 =?utf-8?B?V3FDck5pWlF2cFdqZzRLY2FvQ1NCNkk2SjAwSzVSTFBneHVhU2kxMXhZT0NV?=
 =?utf-8?B?Uk5GMmdzSGI5NERUM2RsZzAvRWxRS0ovb1dqdUlhNUJhM0l0L0ptcnNDcUwy?=
 =?utf-8?B?R3JUclVxTW81MzcrdDdwaWJkd1d4L05SZFVxbmxPK1VlRlhMMXM5SkYyc0hJ?=
 =?utf-8?B?MVZ2R3dJWUhtWjNxUXQwRy8vZCtSM3JMd2U0MkY3REo4Q00zWmhoN0lMdFFD?=
 =?utf-8?B?RHRpeWFIalJkTnFOMTF1aGRkeFBJc1BnMnk5VE5HNmxCMjNmT2pqRHdaWElm?=
 =?utf-8?B?eDVSQzhBMTNpMHA5ZExKdEVObWFpUzU3d1RhOXBUTSt5SWsvQXRvRk8wZVc3?=
 =?utf-8?B?ZW9xdTJXMVpTWUxxUHZVNCttV2JiUkdqQk5yUGsra1FPVE9RTXg1TnJnYjBW?=
 =?utf-8?B?V09ZcU1wSnZNeFQwUGQ4RVIza1duOGY1L1FWdjlJVmJ2b2ZuSUtBMWVyWGNY?=
 =?utf-8?B?TUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 4fa337ca-5418-491d-2a91-08dc706c8e05
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 May 2024 21:11:12.2514
 (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: +U+zznIjeei+bIKhc8ynnbDJg4g01RyyFc1Y0w+AejQxRivsx6fEps2SMwvqGxgYu/aYcVKWqeliA4qqx3Tn4UP1+h8RyInNzM+CoPJEEZ4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR11MB8008
X-OriginatorOrg: intel.com

Hi Babu,

On 4/25/2024 1:17 PM, Babu Moger wrote:
> Detect the vendor and pass the sysfs name for the vendor for searching
> the controller information.

Could you please write a proper changelog?

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
>  tools/testing/selftests/resctrl/resctrl_val.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index a30cfcff605f..e3b09128ec3d 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -224,14 +224,24 @@ static int num_of_mem_controllers(void)
>  {
>  	char imc_dir[512], *temp;
>  	unsigned int count = 0;
> +	int ret, vendor, size;
>  	struct dirent *ep;
> -	int ret;
> +	char *sysfs_name;
>  	DIR *dp;
>  
> +	vendor = get_vendor();
> +	if (vendor == ARCH_INTEL) {
> +		sysfs_name = UNCORE_IMC;
> +		size = sizeof(UNCORE_IMC);

Why is separate size needed? Can strlen() just be used when needed?

> +	} else {
> +		perror("Unsupported Vendor!\n");

ksft_perror()?

In the message, "Vendor" need not start with capital. It may also
help to print the vendor value in this unlikely case.

> +		return -1;
> +	}
> +
>  	dp = opendir(DYN_PMU_PATH);
>  	if (dp) {
>  		while ((ep = readdir(dp))) {
> -			temp = strstr(ep->d_name, UNCORE_IMC);
> +			temp = strstr(ep->d_name, sysfs_name);
>  			if (!temp)
>  				continue;
>  
> @@ -242,7 +252,7 @@ static int num_of_mem_controllers(void)
>  			 * well and hence the last underscore character in
>  			 * uncore_imc'_' need not be counted.
>  			 */
> -			temp = temp + sizeof(UNCORE_IMC);
> +			temp = temp + size;

strlen()? (Keeping in mind the adjustment for the "_" character).

>  
>  			/*
>  			 * Some directories under "DYN_PMU_PATH" could have

Reinette