Re: [PATCH v7 09/24] x86/resctrl: Introduce bitmap mbm_cntr_free_map to track assignable counters

[Reinette Chatre]

Hi Babu,

On 9/23/24 11:33 AM, Moger, Babu wrote:
> Hi Reinette,
> 
> On 9/19/24 11:42, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 9/4/24 3:21 PM, Babu Moger wrote:
>>> Hardware provides a set of counters when mbm_cntr_assignable feature is
>>> supported. These counters are used for assigning the events in resctrl
>>> a group when the feature is enabled. The kernel must manage and track the
>>
>> The second sentence ("These counters ...") is difficult to parse.
> 
> How about?
> 
> Counters are used for assigning the events in resctrl group.

Apologies but I am just not able to parse this. How about: "These counters
are assigned to the MBM monitoring events of a MON group that needs to
be tracked."

...

>>> + */
>>> +static int mbm_cntrs_init(struct rdt_resource *r)
>>
>> Needs __init?
> 
> Did you mean to merge this with dom_data_init and don't have to have a
> separate function. Please clarify.

Here I was referring to the actual __init storage class attribute. Since
mbm_cntrs_init() is only called by __init code, it too should have the
__init storage class attribute.
I do expect that mbm_cntrs_init() will be called by dom_data_init() and
care should be taken when making this change since it seems that dom_data_init()
itself needs the __init storage class attribute. Looks like this was missed
by commit bd334c86b5d7 ("x86/resctrl: Add __init attribute to rdt_get_mon_l3_config()")

Reinette