Received-SPF: pass (google.com: domain of linux-kernel+bounces-52761-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: Mon, 5 Feb 2024 14:56:22 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
To: Ilpo =?utf-8?B?SsOkcnZpbmVu?= <ilpo.jarvinen@linux.intel.com>
CC: <fenghua.yu@intel.com>, <reinette.chatre@intel.com>, <shuah@kernel.org>,
	<linux-kselftest@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 2/5] selftests/resctrl: Add helpers for the
 non-contiguous test
Message-ID: <aavahod3ehdj62egrxzqqz2qsa7nwxat72kj3gufeqckckn7b4@amwmn7q52z2z>
References: <cover.1707130307.git.maciej.wieczor-retman@intel.com>
 <89b8965d563e4e61b95b20be55c26475d830b245.1707130307.git.maciej.wieczor-retman@intel.com>
 <7cfff3da-b476-4f24-aa16-428c79bd81e2@linux.intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <7cfff3da-b476-4f24-aa16-428c79bd81e2@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?yUc+cVQrifvxZGGrRmA+ZrujrI97z2mBPY8E57nI+WEPgVRDM8qvzYMpjF?=
 =?iso-8859-1?Q?9quWlQQp/4lAL+RMwMkWngs+lwa45x/jf7L+6YV1O6CoWPBoA0+cFCXKKN?=
 =?iso-8859-1?Q?fl9iTen2nQPGSvTxa87uSVJwDREQ8l8DiQ48pTHUL/nv3OdPHS4Vya+xjl?=
 =?iso-8859-1?Q?YEPC76NHolmLgHCXbupaHoW9sPOnj+MGIMxO8eUt7PYa49lZT1IYQI8Ezk?=
 =?iso-8859-1?Q?dtoNXlLs228AFXh37CX+rqSZasvTyyPyocgwt0vf1te3bnzNvWnFucqmdW?=
 =?iso-8859-1?Q?6MT08CuN/hwy6xaNIcsftCVpWrPzcMTptz6YclD7jkDKlpLilVkZvXReKO?=
 =?iso-8859-1?Q?LaCZBSPZXbST21uPguIfrOxDpGjAr/HxgzcZz1TrD/MSe2sY2eunw/Hwk6?=
 =?iso-8859-1?Q?n04yP2w6nT30uI8YM0vlflm3oafsi5cUACfFpur7LZHDUEK8wXrjSGCc75?=
 =?iso-8859-1?Q?oNGZPdWfRx7u7Y6EYtHaTjjK4RSF2ny7mg/8NWYL8at/rACpi1nNZQsTxm?=
 =?iso-8859-1?Q?UEZDrHT2lVQZxTM6fMnZ3w6j4oEMhxDKmdT85OQhH+doQyJlGAeTFlvrnJ?=
 =?iso-8859-1?Q?1gtKL49cWwVMlBSkMb+LV6rohLIxEzfjs6AHBH2JCLbsjleG4KB0k/UAFf?=
 =?iso-8859-1?Q?rNNbmYJflB+oMpTLSN0KRudxk1c02m3OSIMm6WaNw/s0OHcxsDYWrLifHQ?=
 =?iso-8859-1?Q?AULTnf96x3qXVEbs9pu9eJ3cGZcMWeRLdYCMHKcl6kzpY/gphlUty5iQ3a?=
 =?iso-8859-1?Q?FL9Ki3caEeW3f9k411Ur/dINbH1ZeSQ47Kjgn5HQuNsjXbzZ0RQW4WqLMw?=
 =?iso-8859-1?Q?PuM8fciX+SWjMwdx9Z0cLzFpiKWsFkqhgOBzztBuOeQi3Liop9NGPd6soj?=
 =?iso-8859-1?Q?g2aVfvLH0LYPc6nMvwyADoAmGS9jbpOVZMXBK3Z9LFbCwftE4cU18P+++h?=
 =?iso-8859-1?Q?0R2GLtJ8F9Krcwp577rXAHmTlx05GyC/ofKBPTEK1B+ZcALEOmFtRUdLAb?=
 =?iso-8859-1?Q?ShoYX/rlu1tpqnWRMJhhRx3ID+VIfhfD/KGqr1aKYgmOGkEFsnuuvHt1mx?=
 =?iso-8859-1?Q?WLlyDzK9CJ7s4mKehjV+hXgYTNmN4LY9ubqrfgZtJUkQI3rdK2YG1fZw56?=
 =?iso-8859-1?Q?vXNcHmSkuTDs0vZPbvzsDKRtEm9QKMKvxT5avP0Jgk9Al/didLAS/Mb0s+?=
 =?iso-8859-1?Q?VlbKI8I056cdbVvJgUm8h0L8v15mPyLyAVSPfdZqOWP7Dy+4uHLAW2SVmV?=
 =?iso-8859-1?Q?fuTChTymfBbvSxwdjZ23GuTPVUyL1jSdWBMhQH1e1qO/JRbMm3fABUyB6t?=
 =?iso-8859-1?Q?B5ayJQZ3t/WbyUL9N2PiSBYlMdWYsgRwTA98OG5pDDGWRM5blyAGtRNBsa?=
 =?iso-8859-1?Q?ZftV67p1oofKciIpSSW84SqK2yo9TxXqzC/Ycv/Ac3EDBcgv+h1URpiD5m?=
 =?iso-8859-1?Q?mVkIpYy3YMPKs29MtQMY3ff+AJJQjT7wc53F9JXnWuUdg9rdJkWRkm61ww?=
 =?iso-8859-1?Q?ZHYjQgDlvA9ZGEeb+YSNy3QgwCSdHzwGr0WIJ/Vh50n3uZVkFq/XelLO1l?=
 =?iso-8859-1?Q?2SXHTcE2x4SqHyuKcZNhVlM16rQiJhvQI47FtHgIDPaEss8WnQkY/afv+c?=
 =?iso-8859-1?Q?RDlMJ0VYnRFoWWrOAYtl9UPnYLHbjq5RYKb1nGlfpt3dpVK+U6s8Y9nCz9?=
 =?iso-8859-1?Q?m9yhUbqj5ICROPuTGkY=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 380a84b3-7822-4782-6cca-08dc2652418e
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Feb 2024 13:56:31.0386
 (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: 8KxACUwpFKGrtrgTfvy4mKR8LTx3EYf9UyEb/w2xRkt5zIlBIB/LP8Q0xBmGgZ+/IUjGuuzCI7tgt4Spz5IDeoiBcrqcwXyIuRt5Aho+fG0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB6975
X-OriginatorOrg: intel.com

On 2024-02-05 at 15:16:27 +0200, Ilpo J?rvinen wrote:
>On Mon, 5 Feb 2024, Maciej Wieczor-Retman wrote:
>
>> The CAT non-contiguous selftests have to read the file responsible for
>> reporting support of non-contiguous CBMs in kernel (resctrl). Then the
>> test compares if that information matches what is reported by CPUID
>> output.
>> 
>> Add a generic helper function to read an unsigned number from a file in
>> /sys/fs/resctrl/info/<RESOURCE>/<FILE>.
>> 
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v4:
>> - Rewrite function comment.
>> - Redo ksft_perror() as ksft_print_msg(). (Reinette)
>> 
>> Changelog v3:
>> - Rewrite patch message.
>> - Add documentation and rewrote the function. (Reinette)
>> 
>> Changelog v2:
>> - Add this patch.
>> 
>>  tools/testing/selftests/resctrl/resctrl.h   |  1 +
>>  tools/testing/selftests/resctrl/resctrlfs.c | 36 +++++++++++++++++++++
>>  2 files changed, 37 insertions(+)
>> 
>> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
>> index a1462029998e..5116ea082d03 100644
>> --- a/tools/testing/selftests/resctrl/resctrl.h
>> +++ b/tools/testing/selftests/resctrl/resctrl.h
>> @@ -162,6 +162,7 @@ unsigned int count_contiguous_bits(unsigned long val, unsigned int *start);
>>  int get_full_cbm(const char *cache_type, unsigned long *mask);
>>  int get_mask_no_shareable(const char *cache_type, unsigned long *mask);
>>  int get_cache_size(int cpu_no, const char *cache_type, unsigned long *cache_size);
>> +int resource_info_unsigned_get(const char *resource, const char *filename, unsigned int *val);
>>  void ctrlc_handler(int signum, siginfo_t *info, void *ptr);
>>  int signal_handler_register(void);
>>  void signal_handler_unregister(void);
>> diff --git a/tools/testing/selftests/resctrl/resctrlfs.c b/tools/testing/selftests/resctrl/resctrlfs.c
>> index 5750662cce57..e0fbc46a917a 100644
>> --- a/tools/testing/selftests/resctrl/resctrlfs.c
>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
>> @@ -249,6 +249,42 @@ static int get_bit_mask(const char *filename, unsigned long *mask)
>>  	return 0;
>>  }
>>  
>> +/**
>> + * resource_info_unsigned_get - Read an unsigned value from
>> + * /sys/fs/resctrl/info/RESOURCE/FILENAME
>> + * @resource:	Resource name that matches directory name in
>> + *		/sys/fs/resctrl/info
>> + * @filename:	File in /sys/fs/resctrl/info/@resource
>> + * @val:	Contains read value on success.
>> + *
>> + * Return: = 0 on success, < 0 on failure. On success the read
>> + * value is saved into the @val.
>> + */
>> +int resource_info_unsigned_get(const char *resource, const char *filename,
>> +			       unsigned int *val)
>> +{
>> +	char file_path[PATH_MAX];
>> +	FILE *fp;
>> +
>> +	snprintf(file_path, sizeof(file_path), "%s/%s/%s", INFO_PATH, resource,
>> +		 filename);
>> +
>> +	fp = fopen(file_path, "r");
>> +	if (!fp) {
>> +		ksft_print_msg("Error in opening %s\n: %m\n", file_path);
>
>Adding the extra \n in between there will likely mess up the test output 
>formatting (the expected prefixes will not appear). Therefore, manually 
>adding newlines should be avoided.
>
>> +		return -1;
>> +	}
>> +
>> +	if (fscanf(fp, "%u", val) <= 0) {
>> +		ksft_print_msg("Could not get contents of %s\n: %m\n", file_path);
>
>Ditto.

I thought I've seen some other error checking paths where '%m' was on the next
line but I couldn't find it again right now so I'll remove the newline.

>
>With those two fixed,
>
>Reviewed-by: Ilpo J?rvinen <ilpo.jarvinen@linux.intel.com>
>
>
>-- 
> i.


-- 
Kind regards
Maciej Wiecz?r-Retman