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!
--
i.
