Re: [PATCH 03/24] selftests/resctrl: Refactor get_cbm_mask()

[Reinette Chatre]

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