On Thu, 2 Nov 2023, Reinette Chatre wrote:
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.
Yeah, Maciej also commented on this. I've already made some changes but I'll incorporate some of your suggestions too.
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)
Having max in the name sounds useful.
Also related to this, the local variables called long_mask should be renamed but perhaps not in this series to not block Maciej's work with neverending stream of cleanups :-).
@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()?
I tried to put as much as possible into get_bit_mask() since every caller will have to do the same things anyway. I cannot avoid checking cache_type here because snprintf() is using it.
Once again, very superb review of the whole series, thank you very much for all the effort! It's really appreciated!