Hi Babu,
On 10/17/24 4:11 PM, Moger, Babu wrote:
> On 10/15/2024 10:29 PM, Reinette Chatre wrote:
>> On 10/9/24 10:39 AM, Babu Moger wrote:
>>> + if (!d) {
>>> + list_for_each_entry(d, &r->mon_domains, hdr.list) {
>>> + ret = resctrl_arch_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
>>> + rdtgrp->closid, cntr_id, false);
>>> + if (ret)
>>> + goto out_done_unassign;
>>> +
>>> + clear_bit(cntr_id, d->mbm_cntr_map);
>>> + }
>>> + } else {
>>> + ret = resctrl_arch_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
>>> + rdtgrp->closid, cntr_id, false);
>>> + if (ret)
>>> + goto out_done_unassign;
>>> +
>>> + clear_bit(cntr_id, d->mbm_cntr_map);
>>
>> Please see comment to previous patch about the duplicate snippets. Snippets can be
>> replaced with single function that also resets architectural state.
>
> Sure.
>
> will combine rdtgroup_assign_cntr_event() and
> rdtgroup_unassign_cntr_event().
That is not what I suggested. I attempted to clarify in response to patch
with original feedback:
https://lore.kernel.org/all/c36e0c76-1666-4a31-984e-1ee6aed2e414@xxxxxxxxx/
>
> I need to rename the function. How about resctrl_configure_cntr_event()?
>
>
>>
>>> + }
>>> +
>>> + /* Update the counter bitmap */
>>
>> What is the update?
>
> Clear the counter bitmap.
Could you please update the comment to be more specific? What is
written can be seen from code.
Reinette
