Received-SPF: pass (google.com: domain of linux-kernel+bounces-194525-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <358c9ac7-4a55-4aa7-b51d-be15e377d154@intel.com>
Date: Wed, 29 May 2024 10:44:59 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 07/16] selftests/resctrl: Rename measure_vals() to
 measure_mem_bw_vals() & document
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
	<linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>, Babu Moger
	<babu.moger@amd.com>, =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?=
	<maciej.wieczor-retman@intel.com>
CC: <linux-kernel@vger.kernel.org>, Fenghua Yu <fenghua.yu@intel.com>, "Shuah
 Khan" <skhan@linuxfoundation.org>
References: <20240520123020.18938-1-ilpo.jarvinen@linux.intel.com>
 <20240520123020.18938-8-ilpo.jarvinen@linux.intel.com>
Content-Language: en-US
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20240520123020.18938-8-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VzllQUFMbVI4OW5oazBOeUM0Ry9CUGJld2xhZ0pjNmFxZWpOT2RNL0JoQ2M4?=
 =?utf-8?B?blp6SWI5blo4dnBoYUhVOGU4djNYb0hPdm1aTmxaV2wxVGlXRFZtVEFNazJs?=
 =?utf-8?B?bG1kbnNkc21QaTQrMjR1UGRINllzZmo0K3dSQWEyVEhXSWl2dGZqSDdxdnpp?=
 =?utf-8?B?c0FOQkxQNlZhc1pBVTJuNi84WFpvZmVWSGwwMzdIenBiZkU5eEIyWjkyQzR4?=
 =?utf-8?B?cTZMZDRydzlBQktrVXlrVUpiTzZYelVoM25Bd0FncmtTL3JJUUU0YzMwcVc3?=
 =?utf-8?B?MUxaK3RzUG11Y0x3UGJPc1gxbThCR0pXbm52dlRKK3Z6K0xZM1g5MUNPZVNG?=
 =?utf-8?B?OUNsaUpta0xQd0xkWVRJWW5EUis3bHB1TnNNY0lZMWNuL2gvbUIxakgvcXRi?=
 =?utf-8?B?OGRUejNKbDYrSWNjbGlveTJHVXR4cE9pelpmQ1BxdlMzMnFOdnFxdzVuZHRa?=
 =?utf-8?B?V1dhS2lpWTVqc3VaQlpEa0MwMldqSWoxcnZkTjhORUhGeFZ4YzhVZzAyaHN5?=
 =?utf-8?B?V3FFZitocWJHM2N4TGRwM045Lzh2T1lGWWxtbXdRRGY4KytLVzZGVjBLWDE1?=
 =?utf-8?B?WTB3cUVoNWNtL0hwc3lsNktZT2tKOWw1b2hrSTNKTHJ1YjRPYmJ2d0pUZSsv?=
 =?utf-8?B?OHJEOHNkZUJGYTFERVE1dVlXV0FQVDVVRmhlRGpadjF5dHJRTi9qNlpXempK?=
 =?utf-8?B?aXhHaHRtZmw0Q1k3K0pnd2xDUDVVL1F1eFprOC9oaEhSYVhNSkFPc1RBZ1FR?=
 =?utf-8?B?ZFYzQmVRMTVhZ09zdEtxRjBrcWZ0N2p5RGovNVRvd2laNUFkWVFBajFNT0ho?=
 =?utf-8?B?WHhIRS9qQjFQYVdTdEZyeWVFSW5Xem5KcG1saUtOKzJENVFXU0xYOGJRWUo3?=
 =?utf-8?B?SFNncVNFN0I0TmNNaWpBVkJjV2c3cHh4QmUzQ2Npb0s4c2EvU1NwUWEwYTNI?=
 =?utf-8?B?RlJEcWVSdFJGZ0NYZFMzT2gyNVd2ZFdkQUxxREwzSVhuMnBqbzdxWHN3K05B?=
 =?utf-8?B?ZElFK29DSjFBUXVpZ3czenJDSVBXMGJJdUE2dW1taFVmSjVoL2MxY3g4bFVs?=
 =?utf-8?B?YXFhY1g5UGRienRybmRhVlJhSGkvWDFnQmVEaUIrSWlwNHpIOEE4a09BTDgx?=
 =?utf-8?B?OVJUdURDRktlZ081NGdpVFUyNVpkbkxhZ1RYaTVoMXF6d1B5N0F3SFFNUUJv?=
 =?utf-8?B?MU1sb09RWjNEZnI5V0NRdXpEaEJVcStqV2duT1Zjc2JORllpOENxV2xnOTFw?=
 =?utf-8?B?L0w3eXFHWHJNaG9wWVpZVnhLOWVQWHJSSlR5enVSdVk3c1VCNGVGRXdCTy9K?=
 =?utf-8?B?eFZncEpQNlRFR3Z4WlZabG5QeFNnaTJDSjJQUUZZakVxMmdFSjc2NEJjY3gw?=
 =?utf-8?B?cHZ1Smtta3UyaWNJbHVUeEFYS0FpMXRZMERtMllMdmNmaVFTYllmUEJ1T2NM?=
 =?utf-8?B?Nkg3MEpxaW1ldEJsY2NEY09QZGYwSGhsaU5jamF5WmFXMi82Vk1kZWJCQlhN?=
 =?utf-8?B?Ky9Ub1V1NklabjBRNWNtdEg2UFc1UTJhYVE4U21TMVZJbEJ2UGpCVlpXMHYx?=
 =?utf-8?B?T1FQSWtjNDlFbjhOZ1lyMFFJK1ZVUFYvRUlGSXdKTzcwMlNYVlFxaWNsQTJa?=
 =?utf-8?B?VjByRzVwNm5xVzRESFVVU09aUDdvSDVOUVdDQTRBeEIydTgzYngvZkd3cTFY?=
 =?utf-8?B?UnQ1SFVrTVRQc2tHYk1OR1AxWlVlRnBWT2VIVE1tRGo3d2FzVlg2cWMrN1NS?=
 =?utf-8?B?MUFYcUVpWVBndG9YSENpY3d4SVpFaEp0MXk2bkZRaXhTaDVRdmxvOS9UWWtH?=
 =?utf-8?B?S2t2cUtMVXd5d2ZKRGtIcEpJaFlHditnVkV0U2R3WVRSNnZSMjBXWmRPeGtt?=
 =?utf-8?B?dHJ2bW9TWGZJWXNIMXJYZjRxWGs5R1NUZ3JUcnVGenEwZ2xLTytES1JJZE9X?=
 =?utf-8?B?Y2tUeTNOTnUxWHM5L1p6THVVM01nc3NGZkhlZ1Y2SE4wQkZLWU5IelBxMzRt?=
 =?utf-8?B?UFdkZmtDajV5Rmx3OUhBQjV1cHlhV2o0MmJSRlI5YkRYMTlOUW1JUjNOUjNV?=
 =?utf-8?B?elJNZlVPM002VmRNT3ZBc0FobUZJb2dHZ1ovQXQzN1hhQnduZXBLK2tST0Zx?=
 =?utf-8?B?ZTF1bkNBb1U5Wm5Tbk9EV2UrL2dueWIrYTc0bEN0ZFdkdHllcWlwMnZ2QnFE?=
 =?utf-8?B?ZUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 05de695f-5a27-4f2e-8bcb-08dc800710e5
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 May 2024 17:45:01.7396
 (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: 3KQZ/23UFvH1vXI09M8QR/+qSJifLPWr8Ciw2SbYXev/VLhsRGIyUhpOkfbJOUtxvNAjFq2wx/V5i7Y2yI268yPMjMDKM66aZzS711/ZfCI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6446
X-OriginatorOrg: intel.com

Hi Ilpo,

Function name in shortlog does not match the patch.

On 5/20/24 5:30 AM, Ilpo Järvinen wrote:
> measure_val() is awfully generic name so rename it to measure_mem_bw()

measure_val() -> measure_vals()

> to describe better what it does and document the function parameters.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>   tools/testing/selftests/resctrl/resctrl_val.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index 198b5133432a..6c53eb9171ea 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -637,8 +637,14 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
>   		set_cmt_path(ctrlgrp, mongrp, domain_id);
>   }
>   
> -static int measure_vals(const struct user_params *uparams,
> -			struct resctrl_val_param *param, pid_t bm_pid)
> +/*
> + * measure_mem_bw - Measures memory bandwidth numbers while benchmark runs
> + * @uparams:		User supplied parameters
> + * @param:		parameters passed to resctrl_val()

Please consistently start descriptions with upper case.

> + * @bm_pid:		PID that runs the benchmark
> + */
> +static int measure_mem_bw(const struct user_params *uparams,
> +			  struct resctrl_val_param *param, pid_t bm_pid)
>   {
>   	unsigned long bw_resc, bw_resc_start, bw_resc_end;
>   	FILE *mem_bw_fp, *mem_bw_fp2;
> @@ -912,7 +918,7 @@ int resctrl_val(const struct resctrl_test *test,
>   
>   		if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
>   		    !strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
> -			ret = measure_vals(uparams, param, bm_pid);
> +			ret = measure_mem_bw(uparams, param, bm_pid);
>   			if (ret)
>   				break;
>   		} else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {

With typos fixed:

| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette