Hi Tony,
On 12/12/2024 5:37 PM, Luck, Tony wrote:
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.
Yes. We can do that. I will to use rdt_last_cmd_printf().
+ 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.
Sure.
+ 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.
Yes. Agree.
It is right thing to continue assignment if one of the domain is out of
counters. In that case how about we save the error(say error_domain) and
continue. And finally return success if both ret and error_domain are zeros.
return ret ? ret : error_domain:
+ }
+
+ 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
thanks
Babu
