Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <575b4c53-34c4-48ed-906d-b9dedb80e0ef@intel.com>
Date:   Thu, 2 Nov 2023 10:47:10 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 03/24] selftests/resctrl: Refactor get_cbm_mask()
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: <20231024092634.7122-1-ilpo.jarvinen@linux.intel.com>
 <20231024092634.7122-4-ilpo.jarvinen@linux.intel.com>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231024092634.7122-4-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?QnFHVkQzam1LYlhYajRBV3VmZ2V1Y3JsTFRtWnErWVBHRGw4ZjZ4dGVsYUJs?=
 =?utf-8?B?YmtER1VXZ3pmUE5wWG83VFpsbWpEOUsyQ0Fqc1AxRFBhQWRyMVdsVERzcmlM?=
 =?utf-8?B?VHJCNXUyejZEVlVsQTZSSDJYYWxOaXQvNlFQL2xBaEptNzZzNExVM0JEbGFt?=
 =?utf-8?B?ZFlXSWY0aGpwVVVvTytIekNsS0lja1FRYjkvVVYxa1RyaW1oWGQzUjZocTdC?=
 =?utf-8?B?TkNlVmhMUC9ydjlSdTlxT3d1dW1tUmdubDhLZGh1K3VOcXpqZ0NEK2p1S0kv?=
 =?utf-8?B?M2oybkhGYmhBOXpDNHN2Z1FQa3FrOFlrekRnSUlvLzJvckFvOXFZcFl4UEJL?=
 =?utf-8?B?NVJqWkczeWoxYkFJb01RaW9nOG5rdUlYVmFFbm9DR2dTVkJsQS8zUzdoTzdC?=
 =?utf-8?B?SGdiL1RXUXNMcmNCYkVqQ3hUWU5BMWIzbFZqS1JqdUMvUE5vdUcyQ1JmRm1n?=
 =?utf-8?B?R2FnQVNYOXNlUHBtSEg0YnZVTTN1QXYvMWE4Q1BtNGlRZ2YrZGx6WnN3YVNa?=
 =?utf-8?B?dmdIRnhEZnJUVW9ETFZ5UDErTlZLMkZzdDk2NTFxUUhoS0F4Z3U1T0hJeiti?=
 =?utf-8?B?TTJTNVRxckE5MU5NaFBpQ2J6cnJIOExnMjhoeVZFVjNZVlVmZFBzOVVQbnJ2?=
 =?utf-8?B?WlFsb0IySTlqRXgzYXFxeEFUTFZPMlhxZndzMHh1K1dyTVNiOXBpaUdLYTVr?=
 =?utf-8?B?WWx0T1g0bkZnK21BbnlsN0pmc0NIaW9iMzlCOGVXMXFtb0lONWhVdXZGMEV1?=
 =?utf-8?B?OXRBc21kQ0RNTU9vM21RaXM4ZHppanZDUTIydkIyaEZ2K2pLeWl0SlI0RkZV?=
 =?utf-8?B?R283M1VmbmVkRDAxTGZkeVJld2hPa2ZEVU5kUVRVMU1pM0hxclNESWxzTzYx?=
 =?utf-8?B?eThwUi91QzZTWmp1T2tVaDIyaFNZSDd3V2g3KzAvSnhidXJnK1dkYVVJZDhs?=
 =?utf-8?B?cUNoTi9PdWZnUHo0Q3lyWE5Fb2E1dEhPSVNDQ3BDbWRYbzVRMWMzYVkxNnhi?=
 =?utf-8?B?UkVGOUkxVVFwYnRlYWpSYjdtUTJ4SWtWQ2laeFplc1ViLzYwRlZaenYramRj?=
 =?utf-8?B?TVh4c3c2SWgza3VmU0V3ZXpvejNuZU01S0FCVkJ0L2hjdUJIV0hVZDRZYnQw?=
 =?utf-8?B?VTZUdG1lYkhlTW9iUGZnZUtCZlBzby9CTTRpa0t2eEs0eStYV2lMNVZESGdq?=
 =?utf-8?B?WFNhaHlJWXpuYm5mRnRhNFFYL2VIMVkyY3FSTktOak5lZS9uNXpuR1V4Tkl4?=
 =?utf-8?B?U2pJS0pycUg3NnoyK3N5eVVxQzhLK2ExbFhsNk1ibCt2T3piYk85dGd2Rkhp?=
 =?utf-8?B?djQ1eFFYaWRKUVFvemdjdGRESTViU05JVmx2NnkwbGlqY1o1N3BRL1pJZ3FY?=
 =?utf-8?B?MjVnM0p0R0dLWThIcDV6YnpPUTkwb1VySmdwbElVZmxEdnpFZEJVWEoxbVVT?=
 =?utf-8?B?VEthd1U1cUVJU3F4c2ZzeE5SNkxxdERpcUh3WEpCSmxCRlc4Q2pNVkdaa3Ev?=
 =?utf-8?B?Q1Z3TWVYQzhhZGpYZC91VkMzRHBCL3FSYm9vYkxIcWpnNEVNcWtsSWlrWmd3?=
 =?utf-8?B?WFhNaHRoQU9adkM5dDdhbDM4WmcyWWV2ZlRzTmRCVnVIR3ZkS0lUQ29HdDlY?=
 =?utf-8?B?L2VmU2piMjBpVkNFS1MzaFY4NiszTkJ5c1BGYWVkZlZwbmhWRUlLMGpwZ25D?=
 =?utf-8?B?K0lrb05VZ2VOS1lvMEJIMzhybnVrR2VtbFFxemk4UzlIVE5ld2djZTF6R1lE?=
 =?utf-8?B?ajc2bDhxeTJQbmQ0MXZCQ243eGxDRGtxeWx1Uk0zeHNDdDE1OFdoR3FSK3Zq?=
 =?utf-8?B?emM0ZGo1UG43MUxqdC9FTW8rWTVSQ0tCS09wMTZzNEpwVVNIajZsM2xtL2N5?=
 =?utf-8?B?NDJYSVZDTnhuWUF4eUI4V0I5b2hpazJsMVVTcGthaUU5TlFORGd2MmhYQWNp?=
 =?utf-8?B?bEpGa3hCczhTWUg3L3QwcGpDeUpQeFN1N05sZkpCVEhQL0xTdXBmN0tRMFhL?=
 =?utf-8?B?WXpaUkNIQzc1NXNyOEFDRmJlS1ErMzV2bUl3akFmVWwwYjlnTGhkdklQTkJC?=
 =?utf-8?B?K01saUk5VDFBQThrOGs5TG5jNEtUakdwWDZjc2dsZFZUdzZRSmpUK3cwNzRh?=
 =?utf-8?B?UHByQTFRZFR0MldRYmExRDd0YXJnM25RY1pMSDJ5VysyZlFNeGlhTklvMFRU?=
 =?utf-8?B?eUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0d7dc331-3848-4723-7f9d-08dbdbcbbd9f
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Nov 2023 17:47:11.1003
 (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: Tl8+8S0JbGsmYvrBDdQFU5Imd1dWpGI/A2c8TMWRACKSLZqlxBdLgrZMqkE6nncAOcsYTLGwD08rzSmjda1D7kUWyXI1POmkLYcXtNjyCEA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB6857
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-1.3 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,
	URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.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 (agentk.vger.email [0.0.0.0]); Thu, 02 Nov 2023 10:47:55 -0700 (PDT)

Hi Ilpo,

On 10/24/2023 2:26 AM, Ilpo Järvinen wrote:

...

> @@ -229,6 +228,31 @@ int get_cbm_mask(char *cache_type, char *cbm_mask)
>  	return 0;
>  }
>  
> +/*
> + * get_cbm_mask - Get cbm bit mask

I know you just copied code here but please keep an eye out for acronyms
to be written in caps.

This needs not be named to reflect verbatim what the function does.
Looking ahead I wonder if "get_full_mask()/get_max_mask()" may not be a
clear indication of what this does?

Something like:
	get_full_mask()/get_max_mask()  Get maximum Cache Bit Mask (CBM) 
	@cache_type:	Cache level(? or should this be "type") as "L2" or L3".
	@mask:		Full/Maximum portion of cache available for
			allocation represented by bit mask
			returned as unsigned long.
	

> + * @cache_type:		Cache level L2/L3
> + * @mask:		cbm_mask returned as unsigned long
> + *
> + * Return: = 0 on success, < 0 on failure.
> + */
> +int get_cbm_mask(const char *cache_type, unsigned long *mask)
> +{
> +	char cbm_mask_path[1024];
> +	int ret;
> +
> +	if (!cache_type)
> +		return -1;

Just to confirm ... error checking on mask is intentionally deferred 
until get_bit_mask()?

> +
> +	snprintf(cbm_mask_path, sizeof(cbm_mask_path), "%s/%s/cbm_mask",
> +		 INFO_PATH, cache_type);
> +
> +	ret = get_bit_mask(cbm_mask_path, mask);
> +	if (ret)
> +		return -1;
> +
> +	return 0;
> +}
> +
>  /*
>   * get_core_sibling - Get sibling core id from the same socket for given CPU
>   * @cpu_no:	CPU number


Reinette