Hi, Tony,
On 11/1/24 16:55, Luck, Tony wrote:
+ if (!strcmp(buf, "mbm_local_bytes")) {
+ if (is_mbm_local_enabled())
+ rdtgrp->mba_mbps_event = QOS_L3_MBM_LOCAL_EVENT_ID;
+ else
+ ret = -ENXIO;
+ } else if (!strcmp(buf, "mbm_total_bytes")) {
+ if (is_mbm_total_enabled())
+ rdtgrp->mba_mbps_event = QOS_L3_MBM_TOTAL_EVENT_ID;
User may think each time toggling the local/total event will effect MBA.
And they may create usage case like frequently changing the events to
maintain/adjust both total and local within bw boundary.
But toggling mba_mbps_event faster than 1sec doesn't have any effect on
MBA SC because MBA SC is called every one second.
Maybe need to add a ratelimit of 1 second on calling this function? And
adding info in the document that toggling speed should be slower than 1
second?
The limit would need to be per ctrl_mon group, not on calls to this function.
It's perfectly ok to switch multiple groups in a short interval.
Agree.
I'm not sure how to rate limit here. I could add a delay so that the write()
call blocks until enough time passes before making the change. But
what should I do if a user submits more writes to the file? Queue them
all and apply at one second intervals?
Maybe define "mba_mbps_last_time" in rdtgroup. Then
if (time_before(jiffies, rdtgrp->mba_mbps_last_time + HZ) {
rdt_last_cmd_printf("Too fast (>1/s) mba_MBps event change)\n");
rdtgroup_kn_unlock(of->kn);
return -EAGAIN;
}
rdtgrp->mba_mbps_last_time = jiffies;
Maybe it would be better to just to add some additional text to the
documentation pointing out that resctrl only checks bandwidth once
per second to make throttling adjustments. So changes to the event
will only have effect after some seconds have passed?
Add additional text would be great.
Thanks.
-Fenghua
