Received-SPF: pass (google.com: domain of linux-kernel+bounces-46400-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: Wed, 31 Jan 2024 13:07:10 +0100
From: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
To: Reinette Chatre <reinette.chatre@intel.com>
CC: <shuah@kernel.org>, <fenghua.yu@intel.com>,
	<linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
	<ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH v3 4/5] selftests/resctrl: Add resource_info_file_exists()
Message-ID: <fog3ndnmeka3isn3recquazpd5wtrzty6v75iikd542rfwk4kd@emlyliiu7jop>
References: <cover.1706180726.git.maciej.wieczor-retman@intel.com>
 <e73a79210076d6b1b7b584b12d0499e2f2e5f4fe.1706180726.git.maciej.wieczor-retman@intel.com>
 <9f8f1f00-f8a8-4c40-90ab-fd3357ab3b6e@intel.com>
Content-Type: text/plain; charset="iso-8859-1"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <9f8f1f00-f8a8-4c40-90ab-fd3357ab3b6e@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?kC79Q+8lB3aSpyLof+JS+IuQ3rgQLrdvXfSlRyFmpPYEIOnKjnrDr7N5VI?=
 =?iso-8859-1?Q?cZDLWSkjMbHIPQxvHRfftTd5eJ3VsMJxdB192LAtakgkHxjU+ZXsToU0fO?=
 =?iso-8859-1?Q?KGBoIoLadmbMvUw4eAI4y6WirMhShe9raqNB/Lt63SnA4cuwPPIm+dRDe/?=
 =?iso-8859-1?Q?x1XVd1wmavztqFxkC1T0IF+qG1L72W6VbDxDp6oAM5vWgOtOtB+DuKSy0M?=
 =?iso-8859-1?Q?i8ppP6VZRkF5zZXFpMGiMCWQlxfxmQPasEoMIjRkg+HBL9oIoCvSNEFde2?=
 =?iso-8859-1?Q?ot+rt/U1b+K80NyD//WC3qCRBoeRz5lP+wgaRWDDKXp6KmC3JUG6isTzLZ?=
 =?iso-8859-1?Q?k1R6RQzwPi/V3uosZDDLm+w8PSqrjTyOevniGYVWL8Fo9dcVtVXPCZYg7r?=
 =?iso-8859-1?Q?7j9rHXvpA970ulpfwST1S5l/hEUXd/hY2vq0xdWAGF7MEbU6DBM/P6U3MJ?=
 =?iso-8859-1?Q?yv+ulZo7dzoPr7/xoGMX4aB+Lz7rUIveq0zhoxn4hrlxy495Ys/+vvcOFn?=
 =?iso-8859-1?Q?DKvjQcq6TDV2otKRdZS7t/TehBk4KVEupzPH0PVVP6MrcPscMzG+pi2SIi?=
 =?iso-8859-1?Q?dL5FwqFUMJCtNzLPcuqfQSS/4krRFwAh2l78uU87IV7m23iQWAyFXXlEGK?=
 =?iso-8859-1?Q?pL50Tg1EAx7xDOgd/IvjC31TwssYpq+Ip/WH9Yunrw5fZUm3WnZ/j+FVDv?=
 =?iso-8859-1?Q?dazBHBxyD7j5k6SBET7dByr1w9Mf7gNt7ujVstagFg6p3BJ/f7yYHKbXM0?=
 =?iso-8859-1?Q?dj+pIuliImEfLCGy+sKphtl5NHygN02BWlZj4KT+ty/xSslHWd9VRzihDP?=
 =?iso-8859-1?Q?JYYkvYRSdmZRE1qGba7/VSdZN1dDzb1ojxEQ+bmAxP8R9mTed8bUt3+4wN?=
 =?iso-8859-1?Q?t7lBL5WoN8WtkrC2+fmZuJxxGOV2fy9kJSi9NE/9NnyAEn8sYPp8Wd3nzO?=
 =?iso-8859-1?Q?QhdLUraG0W+/M4xINZoFjnhKjwIhOX8k6k4eombtKHRyyFjGEW4TfDvGwS?=
 =?iso-8859-1?Q?LNIb3AvEEMGjgBA9FSM+wTsIMB5so3ZO7Ag9ko6jEFi2P6z2vRAnUbyQJn?=
 =?iso-8859-1?Q?JdK0spQJV3sM5EKXVXGfecVCOAo+PoW8vb6qPsfoI2nX5BJfaVZiXAk+3M?=
 =?iso-8859-1?Q?us0ujFeoH76Lqs0eCHA7AdTG32dLPFVCu3ac18TGilhjVgA34ccdN75+7W?=
 =?iso-8859-1?Q?GiOrC1U77HCrHymUww02eOXGVMcrwBX7hoQuyT3iNUEYdnAk3rYhT8ikgE?=
 =?iso-8859-1?Q?rK4916mEEc+6whRAwqNKwp06gkgHoy2506yhcSj8UsPpLlyhPhNUpRL29n?=
 =?iso-8859-1?Q?DaRZ4AJARTOMuHcc/Swa6CA7le5C4kxyhgn2v8/B4nLaJYW/yeq7y7TCpE?=
 =?iso-8859-1?Q?PorbFX71pvkE2U0MAGiEMyWVOZmEbyljK5nKeMlOXgBHoBrtj35uLUyVJR?=
 =?iso-8859-1?Q?bggikogU1qMGOY2A9JZGFs26h4O50rVEws6q1C7PSdUJJJ201sRuUfDfOr?=
 =?iso-8859-1?Q?z9t+Tl09tEMofu+RL/JHwWDR0JD8vAjgzt4e1chq6EoLtVwh36JO1cNajj?=
 =?iso-8859-1?Q?YZMr+sYCM+3LhlFOLw8xrm6ppnPuFX015oUGq5YNxprIA4bbMSLMj3ArVw?=
 =?iso-8859-1?Q?eeu2s/799XtjMwiM2aKbNRBMx6av0QFI5bgK2WvDvNsBrmoxVy4PG56bce?=
 =?iso-8859-1?Q?wsx9SNzK4/DzIkKSA9I=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c8dea873-03a3-47e0-76c5-08dc22552cb5
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6231.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2024 12:07:19.9676
 (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: gpMg+AcmkoekKDwgD+F6stJR8GNOLUp9WaNnulYjBe4VA3FdPtgZr0v7HjQfSXYzIXb/wlbnCjwnB8DkQMQLc3x4ymyWstuD/tLJziIIVn0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR11MB8304
X-OriginatorOrg: intel.com

Hello Reinette,

On 2024-01-26 at 13:08:52 -0800, Reinette Chatre wrote:
>Hi Maciej,
>
>On 1/25/2024 3:12 AM, Maciej Wieczor-Retman wrote:
>> Feature checking done by resctrl_mon_feature_exists() covers features
>> represented by the feature name presence inside the 'mon_features' file
>> in /sys/fs/resctrl/info/L3_MON directory. There exists a different way
>> to represent feature support and that is by the presence of 0 or 1 in a
>> single file in the info/resource directory. In this case the filename
>> represents what feature support is being indicated.
>> 
>> Add a generic function to check file presence in the
>> /sys/fs/resctrl/info/<RESOURCE> directory.
>> 
>> Signed-off-by: Maciej Wieczor-Retman <maciej.wieczor-retman@intel.com>
>> ---
>> Changelog v3:
>> - Split off the new function into this patch. (Reinette)
>> 
>> Changelog v2:
>> - Add this patch.
>> 
>>  tools/testing/selftests/resctrl/resctrl.h   |  2 ++
>>  tools/testing/selftests/resctrl/resctrlfs.c | 26 +++++++++++++++++++++
>>  2 files changed, 28 insertions(+)
>> 
>> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
>> index 4603b215b97e..c39105f46da9 100644
>> --- a/tools/testing/selftests/resctrl/resctrl.h
>> +++ b/tools/testing/selftests/resctrl/resctrl.h
>> @@ -138,6 +138,8 @@ int umount_resctrlfs(void);
>>  int validate_bw_report_request(char *bw_report);
>>  bool resctrl_resource_exists(const char *resource);
>>  bool resctrl_mon_feature_exists(const char *feature);
>> +bool resource_info_file_exists(const char *resource,
>> +			       const char *feature);
>
>In addition what Ilpo commented about other line, this too can be
>on one line.

Thanks!

>
>>  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 e4ba8614fb7b..a6427732e0ad 100644
>> --- a/tools/testing/selftests/resctrl/resctrlfs.c
>> +++ b/tools/testing/selftests/resctrl/resctrlfs.c
>> @@ -763,6 +763,32 @@ bool resctrl_mon_feature_exists(const char *feature)
>>  	return !!res;
>>  }
>>  
>> +/*
>> + * resource_info_file_exists - Check if a file is present inside
>> + * /sys/fs/resctrl/info/RESOURCE.
>
>As confirmed in the changelog this is intended to be a generic function that
>tests if a file exists ...
>
>> + * @resource:	Required resource (Eg: MB, L3, L2, etc.)
>> + * @feature:	Required feature.
>
>... so assuming a use of this by referring to the file as "feature" is unexpected.
>
>This function jumps between "file" and "feature" in comments and code, please
>just stick to goal of making it a generic utility that checks for a file and
>refer to it consistently so.

Okay, I'll stick to 'file'.

>
>Reinette

-- 
Kind regards
Maciej Wiecz?r-Retman