Re: [PATCH v16 29/34] fs/resctrl: Introduce mbm_L3_assignments to list assignments in a group

[Moger, Babu]

Hi Reinette,

On 7/30/2025 3:09 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 7/25/25 11:29 AM, Babu Moger wrote:
> > Introduce the mbm_L3_assignments resctrl file associated with CTRL_MON and
> > MON resource groups to display the counter assignment states of the
> > resource group when "mbm_event" counter assignment mode is enabled.
> >
> > The list is displayed in the following format:
> needs imperative:
>   "Display the list ..."
Sure.
> > <Event>:<Domain id>=<Assignment state>;<Domain id>=<Assignment state>
> >
> > Event: A valid MBM event listed in
> >         /sys/fs/resctrl/info/L3_MON/event_configs directory.
> >
> > Domain ID: A valid domain ID.
> >
> > The assignment state can be one of the following:
> >
> > _ : No counter assigned.
> >
> > e : Counter assigned exclusively.
> >
> > Example:
> > To list the assignment states for the default group
> > $ cd /sys/fs/resctrl
> > $ cat /sys/fs/resctrl/mbm_L3_assignments
> > mbm_total_bytes:0=e;1=e
> > mbm_local_bytes:0=e;1=e
> >
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> ...
>
>
> > diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> > index 5cf1b79c17f5..ebc049105949 100644
> > --- a/fs/resctrl/monitor.c
> > +++ b/fs/resctrl/monitor.c
> > @@ -1080,6 +1080,7 @@ int resctrl_mon_resource_init(void)
> >   		resctrl_file_fflags_init("event_filter", RFTYPE_ASSIGN_CONFIG);
> >   		resctrl_file_fflags_init("mbm_assign_on_mkdir", RFTYPE_MON_INFO |
> >   					 RFTYPE_RES_CACHE);
> > +		resctrl_file_fflags_init("mbm_L3_assignments", RFTYPE_MON_BASE);
> >   	}
> >   
> >   	return 0;
> > diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> > index d087ba990cd3..47716e623a9c 100644
> > --- a/fs/resctrl/rdtgroup.c
> > +++ b/fs/resctrl/rdtgroup.c
> > @@ -1931,6 +1931,54 @@ static ssize_t resctrl_mbm_assign_on_mkdir_write(struct kernfs_open_file *of,
> >   	return nbytes;
> >   }
> >   
> > +static int mbm_L3_assignments_show(struct kernfs_open_file *of, struct seq_file *s, void *v)
> Please move to monitor.c (then mbm_cntr_get() can be private to monitor.c also).


Sure.

> > +{
> > +	struct rdt_resource *r = resctrl_arch_get_resource(RDT_RESOURCE_L3);
> > +	struct rdt_mon_domain *d;
> > +	struct rdtgroup *rdtgrp;
> > +	struct mon_evt *mevt;
> > +	int ret = 0;
> > +	bool sep;
> > +
> > +	rdtgrp = rdtgroup_kn_lock_live(of->kn);
> > +	if (!rdtgrp) {
> > +		ret = -ENOENT;
> > +		goto out_unlock;
> > +	}
> > +
> > +	rdt_last_cmd_clear();
> > +	if (!resctrl_arch_mbm_cntr_assign_enabled(r)) {
> > +		rdt_last_cmd_puts("mbm_event mode is not enabled\n");
> > +		ret = -ENOENT;
> The error returned by the files when "mbm_event" is disabled (but supported) is
> inconsistent. All but this one return EINVAL. Please make return code consistent.


Yes. Sure.

> > +		goto out_unlock;
> > +	}
> > +
> > +	for_each_mon_event(mevt) {
> > +		if (mevt->rid != r->rid || !mevt->enabled || !resctrl_is_mbm_event(mevt->evtid))
> > +			continue;
> > +
> > +		sep = false;
> > +		seq_printf(s, "%s:", mevt->name);
> > +		list_for_each_entry(d, &r->mon_domains, hdr.list) {
> > +			if (sep)
> > +				seq_putc(s, ';');
> > +
> > +			if (mbm_cntr_get(r, d, rdtgrp, mevt->evtid) < 0)
> > +				seq_printf(s, "%d=_", d->hdr.id);
> > +			else
> > +				seq_printf(s, "%d=e", d->hdr.id);
> > +
> > +			sep = true;
> > +		}
> > +		seq_putc(s, '\n');
> > +	}
> > +
> > +out_unlock:
> > +	rdtgroup_kn_unlock(of->kn);
> > +
> > +	return ret;
> > +}
> > +
> >   /* rdtgroup information files for one cache resource. */
> >   static struct rftype res_common_files[] = {
> >   	{
> Reinette