Received-SPF: pass (google.com: domain of linux-kernel+bounces-194528-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <446beadc-d334-4f80-b8d7-53b51b85816b@intel.com>
Date: Wed, 29 May 2024 10:46:56 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v4 11/16] selftests/resctrl: Simplify bandwidth report
 type handling
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-12-ilpo.jarvinen@linux.intel.com>
From: Reinette Chatre <reinette.chatre@intel.com>
Content-Language: en-US
In-Reply-To: <20240520123020.18938-12-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?a0dPOWtqK2UrU1BTRVUxZ2czdEsvT1JRZmFQSFRCNVJCelNxdTMvQjdqRyty?=
 =?utf-8?B?R2FLTFpNRi9GRXdVNEhDTzNBQy9BQ1Zqb3ZIb2hPc1cvcFozVlpSbkYvTkVz?=
 =?utf-8?B?cG5CU2FGNy91aDFNeXpOZDBMS0pxNy8yNVppRzl6TDJYZEVOOFZvMGlWOGlP?=
 =?utf-8?B?ZWx3OW5YdDR6TXZOcXFhUmEvVTBTNTRQR2U0RzM1aFBBU213bGRwRllKZVJO?=
 =?utf-8?B?UmlhOHlucTlldGNhS0Z1WnE5UjVCOERrMlJHWDhnMDRuaWtobzZ2eVRmL2xv?=
 =?utf-8?B?MjU2ejRhSjdFMUU3N0VGNmpxRG1YNW1ZMWxuQUtuU2ZqWFptVTdReEl3K2E2?=
 =?utf-8?B?cGZCU050QnVNQ01vZ2xDVktyL1AyTUNqVnpmWmt2Tnp1MnFoTnNCSktGU0w3?=
 =?utf-8?B?dTA2Q2xLa2JhQzkzZHhYMnkrOVJVbzhPd2FVZXJ3ejRkb0s4dVZQUXgzbzl4?=
 =?utf-8?B?VDY2cE5xbkNBaU5wSHJTVWRBdFpDWmZmWS9EQTdncDJCdmZyaTgvejlWLzhE?=
 =?utf-8?B?Rml5Vjc5NE5wTC9ldEh2WmdPdm8ycy95OTI5cGFNRFloaFNkQXUyYTFGMktB?=
 =?utf-8?B?QXV0d09zckdsUDROTWhnVXh5MFNIdUgxM0xNdGgwQm9sYStCcEZsYnluM0lw?=
 =?utf-8?B?blNmdjFkWC9Cc2p6WUxLck5oOW1tL0tTMTJzWW50dVVvYlpEcVpJSjFjdFBP?=
 =?utf-8?B?bVVCYzliWkNEWDdJUFMwME9NS0tUd1dUczNwUGxxMVNiTkZOMU43NFFKV3dN?=
 =?utf-8?B?bEZsS0FVREFFV1dJT3FEQ3lFT3hSTFg5SkNkVzJGS29qTGxBdFBSVHBqRDZC?=
 =?utf-8?B?NE85RU5ZUE12WWwyZWsvaHhhL2RER3doZmVHZDFkRjQ1a0dqWCsyL2xuN2xN?=
 =?utf-8?B?MlRLSzU3NTcvUS9kVEdiUnA4ZkdRc0pUM1VPWVo0NzM3bkFvVUZkN2lLeFdL?=
 =?utf-8?B?a1Bvd3Rha1pVcVQzenZvTGNwdUNOUVg5MC9XNmR0WGtsM2JWVzhIT1RXeDZs?=
 =?utf-8?B?Z2pXbUxNbHFlSkVOKzZkU3FoeUlydXMzbjZNZTRYK2cvRm5nZHJZME00UGlo?=
 =?utf-8?B?ajJpZ2JYLzdkdXgxME5vc0w5Tk5IOS9jTkVCczVCMm12bWkrRXVrS29PMGFV?=
 =?utf-8?B?V3kxRzRiZ2E0U3RNRFppT213MHhKVFJrZGZQbWRLMkFoZkljc0hIZnpRRURT?=
 =?utf-8?B?QzB6WHpaNHZoakIzenBmdXh5NzNzZGgrR1BRNiswVUxTMHFLYnROYjFIY0pR?=
 =?utf-8?B?SEFZMG84YnloZisraVh4VVhXZEJ5czMxck8zTzQrNlNTQTBLL055dm9nZnpj?=
 =?utf-8?B?bHk1aUt3dUFUZFRkR1N4eDg2TExUSGFtbnBLeCttMXBNeFhoSkJNbm9CNnJw?=
 =?utf-8?B?WXVnS1FtUHh1cTlKVytrSk5JU0hjMjU5UlVMU1FuRkh0ZVdZVW5nbCtLZ1Vz?=
 =?utf-8?B?dU11eTdtdWZmdkZ5d0lVVWFnWUNwMDYxVU44M3pmdlV6dGJJS3hoZlAzMVJt?=
 =?utf-8?B?RS9wYWllYWJTMW1MZDZHS3ZuVUpCZ3hCenlHQml6V3BYS0V4MVAyenR4cGM2?=
 =?utf-8?B?TCtsUStYak8rY3puUFNUSmdQYmd5a055ZXJSS01TMTRhL2loSW1oVlNEczQ0?=
 =?utf-8?B?QXNNOVNucW9LTTRuT2tBUTRWa3AvWVZTU3M3Smc0MVFnQW1RZW1Db0xTVk1m?=
 =?utf-8?B?YlJuOThJVVVjNUVRd0NUeVZhUmYwRGdhT1RVTGZmRFRQOG5YT0JNdnljOFR2?=
 =?utf-8?B?TDVlcGtrWDc4TjNCWHQxTWtrQXFlYWRGSEJoUnk4Um9FWGxjVjFTaTB2anpy?=
 =?utf-8?B?Qno2SktLbCttQm5CNG9QQzRSdFNIMnB0ZmhSaGw4MldUV2c4Q09haUExMUFV?=
 =?utf-8?B?dHVCQ0lHVzlGYlJGaFFFS3BYM25lWEtFQzFIRGFsUm1nNEdhcmJEb2JPaHVv?=
 =?utf-8?B?a2t5WStrZXQ5ZWxlVjdZNTIzbnVkZm1YczA5SXlsTmMxd1NZSnNiekl5b2Mz?=
 =?utf-8?B?MlFwY1lhNzhRSVoveWVlM1JvK2V0RC82TGdkTWpDcE4yMjlUL2RoNFRqYTdl?=
 =?utf-8?B?UzU0c2o2M00wRVhyNk1hSGpUUmVEOTZYS29tTFU0UFM2ZC8rYXNmc1NTcGp1?=
 =?utf-8?B?bkF0dTdoSHU5K0w0TzlNSzY0NnJiclNadWs1Ny9yNjdUbm45ZDNWeEVkK2lR?=
 =?utf-8?B?ZWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8a1c1167-cc3c-4a2a-5381-08dc80075697
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:46:58.6803
 (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: l2Q2Kag8BORQhC3E3hFuaxFpRYtOL4sDDwvwB5TYyX7Eh+Edin9uA0Plgw6YuWBKMn5akC6dccq7cIPifRahv8WFIgmwUaHvQnYWq988pgo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4587
X-OriginatorOrg: intel.com

Hi Ilpo,

On 5/20/24 5:30 AM, Ilpo Järvinen wrote:
> bw_report is only needed for selecting the correct value from the
> values IMC measured. It is a member in the resctrl_val_param struct and
> is always set to "reads". The value is then checked in resctrl_val()
> using validate_bw_report_request() that besides validating the input,
> assumes it can mutate the string which is questionable programming
> practice.
> 
> Simplify handling bw_report:
> 
> - Convert validate_bw_report_request() into get_bw_report_type() that
>    inputs and returns const char *. Use NULL to indicate error.
> 
> - Validate the report types inside measure_mem_bw(), not in
>    resctrl_val().
> 
> - As resctrl_val() no longer needs bw_report for anything, it can just
>    be passed to measure_mem_bw() by the ->measure() hooks.

Needs imperative tone

> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

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

Reinette