Re: [PATCH v6 12/22] x86/resctrl: Introduce mbm_cntr_map to track counters at domain

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 8/6/24 3:00 PM, Babu Moger wrote:
> The MBM counters are allocated at resctrl group level. It is tracked by

Are they not allocated globally? (but maybe that is about to change?

> mbm_cntrs_free_map. Then it is assigned to the domain based on the user
> input. It needs to be tracked at domain level also.

Please elaborate why it needs to be tracked at domain level.

> 
> Add the mbm_cntr_map bitmap in rdt_mon_domain structure to keep track of

"rdt_mon_domain structure" -> "struct rdt_mon_domain"

> assignment at domain level. The global counter at mbm_cntrs_free_map can
> be released when assignment at all the domain are cleared.

"all the domain" -> "all the domains"?

> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v6: New patch to add domain level assignment.
> ---
>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 10 ++++++++++
>   include/linux/resctrl.h                |  2 ++
>   2 files changed, 12 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 0047b4eb0ff5..1a90c671a027 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -4127,6 +4127,7 @@ static void __init rdtgroup_setup_default(void)
>   
>   static void domain_destroy_mon_state(struct rdt_mon_domain *d)
>   {
> +	bitmap_free(d->mbm_cntr_map);
>   	bitmap_free(d->rmid_busy_llc);
>   	kfree(d->mbm_total);
>   	kfree(d->mbm_local);
> @@ -4200,6 +4201,15 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
>   			return -ENOMEM;
>   		}
>   	}
> +	if (is_mbm_enabled()) {

This should also depend on whether the resource supports counter assignment, and that it
is enabled to ensure that r->mon.num_mbm_cntrs is valid.

> +		d->mbm_cntr_map = bitmap_zalloc(r->mon.num_mbm_cntrs, GFP_KERNEL);
> +		if (!d->mbm_cntr_map) {
> +			bitmap_free(d->rmid_busy_llc);
> +			kfree(d->mbm_total);
> +			kfree(d->mbm_local);
> +			return -ENOMEM;
> +		}
> +	}
>   
>   	return 0;
>   }
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index ef08f75191f2..034fa994e84f 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -105,6 +105,7 @@ struct rdt_ctrl_domain {
>    * @cqm_limbo:		worker to periodically read CQM h/w counters
>    * @mbm_work_cpu:	worker CPU for MBM h/w counters
>    * @cqm_work_cpu:	worker CPU for CQM h/w counters
> + * @mbm_cntr_map:	bitmap to track domain counter assignment
>    */
>   struct rdt_mon_domain {
>   	struct rdt_domain_hdr		hdr;
> @@ -116,6 +117,7 @@ struct rdt_mon_domain {
>   	struct delayed_work		cqm_limbo;
>   	int				mbm_work_cpu;
>   	int				cqm_work_cpu;
> +	unsigned long			*mbm_cntr_map;
>   };
>   
>   /**

Reinette