Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.31 as permitted sender) client-ip=23.128.96.31;
Message-ID: <90607b60-b5b0-4dba-b51b-0cd32367d8b2@intel.com>
Date:   Tue, 28 Nov 2023 14:11:05 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 04/26] selftests/resctrl: Refactor get_cbm_mask() and
 rename to get_full_cbm()
Content-Language: en-US
To:     =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>,
        <linux-kselftest@vger.kernel.org>, Shuah Khan <shuah@kernel.org>,
        "Shaopeng Tan" <tan.shaopeng@jp.fujitsu.com>,
        =?UTF-8?Q?Maciej_Wiecz=C3=B3r-Retman?= 
        <maciej.wieczor-retman@intel.com>,
        Fenghua Yu <fenghua.yu@intel.com>
CC:     <linux-kernel@vger.kernel.org>
References: <20231120111340.7805-1-ilpo.jarvinen@linux.intel.com>
 <20231120111340.7805-5-ilpo.jarvinen@linux.intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231120111340.7805-5-ilpo.jarvinen@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?TkQwNTNOVHd3Q3F3aVNTVUVjWTRIalFpcE5RT014L1AyRWtoblE5N0JlcUVJ?=
 =?utf-8?B?S1g0MnBaOXR2TU4rQURVdDZHMC9DZzlJS0pmTUVLTFpVYnFsM0hkWlMvYjly?=
 =?utf-8?B?QWJTV1pyVzdWVkoyWjRtK1FTWnVQdWRSR29MUDRxRTJNaGdYUm9STDVoZ3hv?=
 =?utf-8?B?RE4vSW9LUXU2RWpVbnA2WUdhQkZvODFrNjFDZGd3N2RLYkZqcm1IL1hvUE1S?=
 =?utf-8?B?UEtCdmpqcnQ2RkVtdWdLWnhzRFdLRk8zbWt5ZENrb2ZGYXJHenZBcHhVaUdS?=
 =?utf-8?B?L25rVGVxdmdwNUFHNXErQ01TcVppdVFMNkVhajFwcVd3YjJCOURuSldqSThq?=
 =?utf-8?B?aU9aL1hZanNCaHU5djdmaWdDTlQ4SUtYQW5IRnFla3lDSURmclR5TTRSaFBm?=
 =?utf-8?B?N0pBM2I1WXZMVjErbWl2SjQwejFteXkwUTduNGxRVXJYc2ExS0c4MXEvOTBN?=
 =?utf-8?B?anpEV3dOUXRTb0VhTTZRWnhSN3hUL3BiVEhpNWp4a3pKaitjVzl4UmdXRUFV?=
 =?utf-8?B?ZFBXM3crL0c1cGJDN3BVNUo5cjlHU01YVVB0SW5wQWJOSGo1ZlpXWFlZY0lm?=
 =?utf-8?B?a2I5NlJzQW1tQVYzQmZteXRzUCtXSUw4Qk9TOHJYbzZWQlNEUHlzOUovZE1J?=
 =?utf-8?B?bWF5MCtxdVFCbFRmd1d0MjV1SVVDYldCbEZuZ0N0dElkTEE3SlZITlB1UHNO?=
 =?utf-8?B?Tmx4NEZPeUU4Y0d3bk02NmVWbWxUVjNScE1aZnY3eDdkaWdLdm9BV3RDVTRp?=
 =?utf-8?B?ekQvOGZmaDZoTXJCYkozdlZtKzVxbGRoWGJHYVJOVEJzNnRObFVIc2JzNmRX?=
 =?utf-8?B?T2pya2thaVo0YWR6OTZkN003YngxRjhQdW0zclFrVWhsSGpMVFpZdHhNSFFn?=
 =?utf-8?B?MEt3Vm5VQit4d3R0TzM3V3o2MWRqRDVqZHl3Q1VlbGxkKy9YcTh6M2crWmtz?=
 =?utf-8?B?QkxLdXJvTGRwa09VMmY0czNKSkJIeVQrY2IreVBEUDFLUVJscWNGWlk2WlMv?=
 =?utf-8?B?eE1vQ2FMdEk5UFMvQ0o4OWVpb3lLY2FpdUpYbER2WVVNcXAyODZGTnZGc24y?=
 =?utf-8?B?aDJkWGhXWHJGVngrWmpzcUk1N1A4Qm51ZGUzam1vY3B4Zzg3K2xqdnJESDA0?=
 =?utf-8?B?S3VhU3p6ZXBueTIrempGNGRmNFpqdjkvL1d6eWhiZ1Rrdis3L1BFVVg5K1dZ?=
 =?utf-8?B?WTdjaGVhQTUyclRjVkZIbWVVQVkzM1FSUTEzSEhKcnJVczFNNDhzZkJHVlQ3?=
 =?utf-8?B?N3E5aWt6WFRHNmxjeWtOcjJ2bjFGcnBBWVc0a3ZQNndFQ3ZQL0pKL0hNMENF?=
 =?utf-8?B?YXlGcEkyc3hscjE3UDQ0dnBYMjRhY1FLaFhuRGtjRGtRWjJwQUJtYkY4RVlS?=
 =?utf-8?B?c05ranRaR29LUmVNa0hPb2xWZ29VdTBYU1FJcDRhVEVpTDR3VFlENXVUd0g0?=
 =?utf-8?B?OHZhblJ0YjRUQ29lYmpPbzZSZ240RTFuNVB6a1pXQ3J4YUEzVmUxZjUrd3Bv?=
 =?utf-8?B?Ukw2V0tRanhBdDJWWWE4d21YclhVVEQ1b2tNTS9idXJLWUw5QnhORkZhOThz?=
 =?utf-8?B?MVRLSmxyNzU5QjFvR3d6ZWhETG5yeFBlaCtzZzBabElGRmJsdjU3bUQ2TjJC?=
 =?utf-8?B?cnlyc291WWVXMVJ6cTFSQWVYZ1BLUkRNdU9YdkFkZzh4bGxYZUk5MTVla1ZC?=
 =?utf-8?B?RDA0aWlRRjNHNlZzTnNLNktvdGhIWDNBc1lrVXMwb2o1SlJmSGZzb0tTVVZi?=
 =?utf-8?B?TzNadWxPYkJNWGJOK0hSanlySW83bGNTRHZ6UEJxRElJTkNLUUhtMmtFMUdo?=
 =?utf-8?B?VXNjU3lGS2d2ajQ5K1k5RE42NnFzSEpUaTQrdkdMdXBXK3BseFo4YlVJZ3VV?=
 =?utf-8?B?cjlLL0s3WjkwOUg0eTl5YzliRVM0cXEvY3EyMkFvMG5xWkJwejVvSUNMQTNa?=
 =?utf-8?B?M042RzBJOU1qaW1xbUVFY1BuRnEyVmtGVUxybHZKQnlDUlpTR3Q4QnpNVkhn?=
 =?utf-8?B?T3RLSDhwVVJVcmMxTllxbndBc3FPa3pETHF0YkEwOEMzVlV1bnMrc3Z4ejh2?=
 =?utf-8?B?OHJXQ1FHM3lkVU5ueG12eGtlK0E4aXp4NllkYzNQVHhDQS9VTkRpVGRVVDJE?=
 =?utf-8?B?UVR3eHVYSUVyVEViV2kzcTVRS25pM1pYREtObzVlaVpMZUxPc29Qd1B3UDZK?=
 =?utf-8?B?NGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 021147d0-20ce-4b22-17d6-08dbf05eeae2
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Nov 2023 22:11:06.2458
 (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: Z1hi8bscVKcuu+XMr1tLYECivzCzX9inX8ZgBSkx8/SA899mjbkmKrSeDIvaYtWSYvyYtLyBEAyGnY5XB/TewOmf6HNnUD4hfm5rToTVPjI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB6328
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Tue, 28 Nov 2023 14:11:17 -0800 (PST)

Hi Ilpo,

On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
> Callers of get_cbm_mask() are required to pass a string into which the
> capacity bitmask (CBM) is read. Neither CAT nor CMT tests need the
> bitmask as string but just convert it into an unsigned long value.
> 
> Another limitation is that the bit mask reader can only read
> .../cbm_mask files.
> 
> Generalize the bit mask reading function into get_bit_mask() such that
> it can be used to handle other files besides the .../cbm_mask and
> handles the unsigned long conversion within get_bit_mask() using
> fscanf(). Change get_cbm_mask() to use get_bit_mask() and rename it to
> get_full_cbm() to better indicates what the function does.

"to better indicates" -> "to better indicate"

> 
> Also mark cache_type const while at it and remove useless comments that
> are related to processing of CBM bits.
> 
> Co-developed-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

...

> @@ -229,6 +228,32 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)
>  	return 0;
>  }
>  
> +/*
> + * get_full_cbm - Get full Cache Bit Mask (CBM)
> + * @cache_type:	Cache type as "L2" or "L3"
> + * @mask:	Full cache bit mask representing the maximal portion of cache
> + *		available for allocation, returned as unsigned long.
> + *
> + * Return: = 0 on success, < 0 on failure.
> + */
> +int get_full_cbm(const char *cache_type, unsigned long *mask)
> +{
> +	char cbm_path[PATH_MAX];
> +	int ret;
> +
> +	if (!cache_type)
> +		return -1;
> +
> +	snprintf(cbm_path, sizeof(cbm_path), "%s/%s/cbm_mask",
> +		 INFO_PATH, cache_type);
> +
> +	ret = get_bit_mask(cbm_path, mask);
> +	if (ret)
> +		return -1;
> +
> +	return 0;

Can this just be "return get_bit_mask()" ? But actually, I would
like to propose that this also returns a failure if the returned
mask is zero. This would make the code more robust, especially looking
ahead at utility like cache_portion_size() that divides by this full
mask.

Reinette