Received-SPF: pass (google.com: domain of linux-kernel+bounces-59723-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <f65f7c8a-e2d8-444b-abbc-d28eba0f86f5@intel.com>
Date: Fri, 9 Feb 2024 09:20:27 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v5 3/5] selftests/resctrl: Split
 validate_resctrl_feature_request()
Content-Language: en-US
To: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>,
	<shuah@kernel.org>, <fenghua.yu@intel.com>
CC: <linux-kselftest@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<ilpo.jarvinen@linux.intel.com>
References: <cover.1707487039.git.maciej.wieczor-retman@intel.com>
 <f914c55b9b9f635cff082b0b4a8b636598580821.1707487039.git.maciej.wieczor-retman@intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <f914c55b9b9f635cff082b0b4a8b636598580821.1707487039.git.maciej.wieczor-retman@intel.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?QmFmVzdtVXBzQjFrSXBFc0tkL0ZhRTFQWlZWV0JHZWVsdVowaWROakFqUk5N?=
 =?utf-8?B?eExVTTh5TFRadUxMbjRHeU5PTGtNc2ZEQUwyS0hVb1VNM2UrcDUrV0h4YlNa?=
 =?utf-8?B?VVpQRlY2MVpwZGVRNzRaVS9uSFJkbm9wZXNic1hTemFqNDNWM2xXcmxpYVBJ?=
 =?utf-8?B?a0F1QittV3dhVWpjQ0tWZU1zakk4NHlMOHF5MGdDN2pRUkg3cXdKK1FWRktL?=
 =?utf-8?B?NGs5R1NQNjE4bmY2TlJNQTRLLzBjTm0zSlE4T1ZKdzVNWnpiRGlXVUdtQUdt?=
 =?utf-8?B?ZFdyVi9XQkpiMXRHSm5wYXhSZC8yLzlmUXJ0elY3Q1ZheDNCUjBCeStlQTJD?=
 =?utf-8?B?NnV3cExVY2Urd2JnVGZGSWFWaExja2lFUnNPV20yQkMxcDAra0NVaGNjV215?=
 =?utf-8?B?QzhiTzBmSTVHMFgxR0E1MVNmTTNNbFNXRVhvQjk4akpRdnFvbmdnVzg0ZmE1?=
 =?utf-8?B?U1VSTmN2ZEF0c2wwaklnVXJDSUJFWUhlbEdVQ0t3YXNaZ3h2ZnZGQ290Q1VO?=
 =?utf-8?B?V2FlTHJkc09pMXdtU2h6V0hSQnJqbCtrVDVsaEJGZXBPNGVxRE04dXNpd2w5?=
 =?utf-8?B?anY2OSszQVpQdWFFYzdkQko3ODBFSjZvS2YzSHlRVUt3ZGtQUklFZEVycjNh?=
 =?utf-8?B?bGRRaTFNWC9zNnF0WUlLSVZDYlQ4TXU3RElnQzNhVysrUzhGWXFzdUpGRlM2?=
 =?utf-8?B?dTNXSGc5MUhhODcvb0hDZW9aV3ZVZGlzVVh5VUZYYUo4QVRNcUlBVDFySnlz?=
 =?utf-8?B?QlRaU0xGRWZSQjNPTVlWWjJncUdKYmVXcW9qbXo4dTVYd1VjSHlFRDIxbEFQ?=
 =?utf-8?B?c2RsZWVhYU1NYm1KOWk4OWhkNTkvT0xYTFJlQmJjaFVOUm9QSVNYYUkyeUhx?=
 =?utf-8?B?aXlQMFBOVTQyVW01OUNldkNmZS84UFJnMU1pUC9GQ0tYTys5TTZKRGtBV2Zs?=
 =?utf-8?B?SWMwUTZqaEliYWxoUkxCVFRkb1NRSkFQa2QxNVlyZExsR1EyTEtzSlk1VW9F?=
 =?utf-8?B?blgwRzVaVysxVTZhdFJJb05Ja0diZXhYbFJ2Ry9zMlYwZHlrRGRNQjFBQk9I?=
 =?utf-8?B?UHE1R2hEdythbFNTTVMwWm9pa2VhVmRDaHlkOENVQzJZUHBXV09hOGJBTWNL?=
 =?utf-8?B?Q3hqTy9EUVR3MjJGcGxqdjNBRnpZWjd5Z0xLS29GOFFOeFJCSjlnZVFUWHQw?=
 =?utf-8?B?SlhRWmZOZkJJSXZtOTFOd0dBUHpublRwQmNpU1VJcm84anJhdGZNWjZKR2kz?=
 =?utf-8?B?Q0luQm5oOXc1YmpYUys2MUN4VCs3VktoZ3BGKytlWlBzeVJvUy9lT1YzQVBF?=
 =?utf-8?B?YVRYTkhRV2MwMzZkbENLdVYxc255YjZzM3FId1p1M0Yrd0MzbkkrVW5na0xT?=
 =?utf-8?B?emZaS0s4NTRmM1JCSWhWdDBiSFlMM0UwRUZtbzNkdEpKckRJUkIrRC9ickpk?=
 =?utf-8?B?YlhJM3RaUDBCdTVaZU9MN2p1d0dIeWtLRmZhMjBxL2xjdzZVelFZTTJNQWtm?=
 =?utf-8?B?NGxXUC9JUk9YZzJzVjBBOHp0R21VKzYzcFJ3YnVpQ0lDYm1vSGI3c0tnZTZm?=
 =?utf-8?B?STVMQytoS0tNaFM3WDBDblR4ZE9sR3FXempGT3RtcVA1cnE1QjNtS25QQUlO?=
 =?utf-8?B?Y3EzMm9iMEFNVTR6NFdjbXlLekNBT3ZwL1FzUTZ0NktJZ01BTjlOaGFneXIv?=
 =?utf-8?B?dGhoR05wVXhGSHhiMTR1L1lYbHo1d1JKZDFpbEZGNGdPRmZTSHpWSUJDNlQ3?=
 =?utf-8?B?YS9OQWtqYXlYTXlHdFdITGpGaXlYR3FvOGpPM2JZREpMcDJYQUpYaXI3ZUdx?=
 =?utf-8?B?cGxVZkpibVlpcDd0L1VUWFUxMURaSGxRci9weCt3NkRuSlpvRzEvMEpIclhL?=
 =?utf-8?B?U3ZQeG1uMmRxRE9KZGp4L09mMTM2Slk0dGxIUTdUNVVPWmh1UUJnam5uWVYy?=
 =?utf-8?B?UnpFZEREZEFZZ2MyVEdqUmZnWkpaN20xSm1KUzdxWWhjdEYxblpVL0hJTmZi?=
 =?utf-8?B?U2dFZFlISGFHZlZVeGpWMWRBWkNlMHpVc29GdDBSajB3eFlHdWg4b3MwOWk5?=
 =?utf-8?B?SjB3QlhxdC85US9CRDFxalJLendGWThrSDlGeTRLT05iMStlZzI3YXZGeXVl?=
 =?utf-8?B?MGV1L0liZWpqQUNGOWE5LzBMdlh4RDFPTDVTaHhoTGluR1VIaUluMXlyNW9T?=
 =?utf-8?B?NlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cfb8987d-a230-4a50-1fb6-08dc299368e0
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 17:20:27.7272
 (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: pm1JmSAAdq7CYVbpXHllZoNth1cDcSUG7p6LzuWKT9Qyl3veCWlhUiM09uI1anSZ4CTzjVeCAL2THGBX+qTG9mORUUFxnE72JS632EqGcDY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB8454
X-OriginatorOrg: intel.com

Hi Maciej,

On 2/9/2024 6:01 AM, Maciej Wieczor-Retman wrote:

..

> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index 5116ea082d03..f434a6543b4f 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -136,7 +136,9 @@ int get_domain_id(const char *resource, int cpu_no, int *domain_id);
>  int mount_resctrlfs(void);
>  int umount_resctrlfs(void);
>  int validate_bw_report_request(char *bw_report);
> -bool validate_resctrl_feature_request(const char *resource, const char *feature);
> +bool resctrl_resource_exists(const char *resource);
> +bool resctrl_mon_feature_exists(const char *resource, const char *feature);
> +bool resource_info_file_exists(const char *resource, const char *file);

Portion of next patch sneaked in here.

>  bool test_resource_feature_check(const struct resctrl_test *test);
>  char *fgrep(FILE *inf, const char *str);
>  int taskset_benchmark(pid_t bm_pid, int cpu_no, cpu_set_t *old_affinity);
> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
> index 8a183c73bc23..af4f800ab23d 100644
> --- a/tools/testing/selftests/resctrl/resctrlfs.c
> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
> @@ -708,20 +708,16 @@ char *fgrep(FILE *inf, const char *str)
>  }
>  
>  /*
> - * validate_resctrl_feature_request - Check if requested feature is valid.
> - * @resource:	Required resource (e.g., MB, L3, L2, L3_MON, etc.)
> - * @feature:	Required monitor feature (in mon_features file). Can only be
> - *		set for L3_MON. Must be NULL for all other resources.
> + * resctrl_resource_exists - Check if a resource is supported.
> + * @resource:	Resctrl resource (e.g., MB, L3, L2, L3_MON, etc.)
>   *
> - * Return: True if the resource/feature is supported, else false. False is
> + * Return: True if the resource is supported, else false. False is
>   *         also returned if resctrl FS is not mounted.
>   */
> -bool validate_resctrl_feature_request(const char *resource, const char *feature)
> +bool resctrl_resource_exists(const char *resource)
>  {
>  	char res_path[PATH_MAX];
>  	struct stat statbuf;
> -	char *res;
> -	FILE *inf;
>  	int ret;
>  
>  	if (!resource)
> @@ -736,8 +732,25 @@ bool validate_resctrl_feature_request(const char *resource, const char *feature)
>  	if (stat(res_path, &statbuf))
>  		return false;
>  
> +	return true;
> +}
> +
> +/*
> + * resctrl_mon_feature_exists - Check if requested monitoring feature is valid.
> + * @resource:	Resource that uses the mon_features file. Currently only L3_MON
> + *		is valid.
> + * @feature:	Required monitor feature (in mon_features file).
> + *
> + * Return: True if the feature is supported, else false.
> + */
> +bool resctrl_mon_feature_exists(const char *resource, const char *feature)
> +{
> +	char res_path[PATH_MAX];
> +	char *res;
> +	FILE *inf;
> +
>  	if (!feature)

Now that resource is a new parameter should it be checked also? It is not obvious
why only one parameter is checked.

Reinette