On Thu, Dec 12, 2024 at 02:15:19PM -0600, Babu Moger wrote:
> +/*
> + * Assign a hardware counter to event @evtid of group @rdtgrp.
> + * Counter will be assigned to all the domains if rdt_mon_domain is NULL
> + * else the counter will be assigned to specific domain.
> + */
> +int rdtgroup_assign_cntr_event(struct rdt_resource *r, struct rdtgroup *rdtgrp,
> + struct rdt_mon_domain *d, enum resctrl_event_id evtid)
> +{
> + int cntr_id, ret = 0;
> +
> + if (!d) {
> + list_for_each_entry(d, &r->mon_domains, hdr.list) {
> + if (mbm_cntr_assigned(r, d, rdtgrp, evtid))
> + continue;
> +
> + cntr_id = mbm_cntr_alloc(r, d, rdtgrp, evtid);
> + if (cntr_id < 0) {
> + rdt_last_cmd_puts("Domain Out of MBM assignable counters\n");
Message could be more helpful by including the domain number.
> + continue;
Not sure whether continuing is the right thing to do here. Sure the
other domains may have available counters, but now you may have a
confused status where some requested operations succeeded and others
failed.
> + }
> +
> + ret = resctrl_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
> + rdtgrp->closid, cntr_id, true);
> + if (ret)
> + goto out_done_assign;
> + }
> + } else {
> + if (mbm_cntr_assigned(r, d, rdtgrp, evtid))
> + goto out_done_assign;
> +
> + cntr_id = mbm_cntr_alloc(r, d, rdtgrp, evtid);
> + if (cntr_id < 0) {
> + rdt_last_cmd_puts("Domain Out of MBM assignable counters\n");
Ditto helpful to include domain number.
> + goto out_done_assign;
When you run out of counters here, you still return 0 from this
function. This means that updating via write to the "mbm_assign_control"
file may return success, even though the operation failed.
E.g. with no counters available:
# cat available_mbm_cntrs
0=0;1=0
Try to set a monitor domain to record local bandwidth:
# echo 'c1/m94/0=l;1=_;' > mbm_assign_control
# echo $?
0
Looks like it worked!
But it didn't.
# cat ../last_cmd_status
Domain Out of MBM assignable counters
rdtgroup_assign_cntr_event() does say that it didn't if you think to
check here.
> + }
> +
> + ret = resctrl_config_cntr(r, d, evtid, rdtgrp->mon.rmid,
> + rdtgrp->closid, cntr_id, true);
> + }
> +
> +out_done_assign:
> + if (ret)
> + mbm_cntr_free(r, d, rdtgrp, evtid);
> +
> + return ret;
> +}
-Tony
