Introduce rdtgroup_mbm_assign_control_write to assign mbm events.
Assignment state can be updated by writing to this interface.
Assignment states are applied on all the domains. Assignment on one
domain applied on all the domains. User can pass one valid domain and
assignment will be updated on all the available domains.
Format is similar to the list format with addition of op-code for the
assignment operation.
* Default CTRL_MON group:
"//<domain_id><op-code><assignment_flags>"
* Non-default CTRL_MON group:
"<CTRL_MON group>//<domain_id><op-code><assignment_flags>"
* Child MON group of default CTRL_MON group:
"/<MON group>/<domain_id><op-code><assignment_flags>"
* Child MON group of non-default CTRL_MON group:
"<CTRL_MON group>/<MON group>/<domain_id><op-code><assignment_flags>"
Op-code can be one of the following:
= Update the assignment to match the flags
+ Assign a new state
- Unassign a new state
_ Unassign all the states
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v3: New patch.
Addresses the feedback to provide the global assignment interface.
https://lore.kernel.org/lkml/c73f444b-83a1-4e9a-95d3-54c5165ee782@intel.com/
---
Documentation/arch/x86/resctrl.rst | 71 ++++++++
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 236 ++++++++++++++++++++++++-
2 files changed, 306 insertions(+), 1 deletion(-)
diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 2d96565501ab..64ec70637c66 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -328,6 +328,77 @@ with the following files:
None of events are assigned on this mon group. This is a child
monitor group of the non default control mon group.
+ Assignment state can be updated by writing to this interface.
+
+ NOTE: Assignment on one domain applied on all the domains. User can
+ pass one valid domain and assignment will be updated on all the
+ available domains.
+
+ Format is similar to the list format with addition of op-code for the
+ assignment operation.
+
+ * Default CTRL_MON group:
+ "//<domain_id><op-code><assignment_flags>"
+
+ * Non-default CTRL_MON group:
+ "<CTRL_MON group>//<domain_id><op-code><assignment_flags>"
+
+ * Child MON group of default CTRL_MON group:
+ "/<MON group>/<domain_id><op-code><assignment_flags>"
+
+ * Child MON group of non-default CTRL_MON group:
+ "<CTRL_MON group>/<MON group>/<domain_id><op-code><assignment_flags>"
+
+ Op-code can be one of the following:
+ ::
+
+ = Update the assignment to match the flags
+ + Assign a new state
+ - Unassign a new state
+ _ Unassign all the states
+
+ Examples:
+ ::
+
+ Initial group status:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ //0=tl;1=tl;
+ /child_default_mon_grp/0=tl;1=tl;
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+
+ To update the default group to assign only total event.
+ # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ //0=t;1=t;
+ /child_default_mon_grp/0=tl;1=tl;
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+
+ To update the MON group child_default_mon_grp to remove local event:
+ # echo "/child_default_mon_grp/0-l" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ //0=t;1=t;
+ /child_default_mon_grp/0=t;1=t;
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=tl;1=tl;
+
+ To update the MON group non_default_ctrl_mon_grp/child_non_default_mon_grp to
+ remove both local and total events:
+ # echo "non_default_ctrl_mon_grp/child_non_default_mon_grp/0_" >
+ /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+
+ Assignment status after the update:
+ # cat /sys/fs/resctrl/info/L3_MON/mbm_assign_control
+ //0=t;1=t;
+ /child_default_mon_grp/0=t;1=t;
+ non_default_ctrl_mon_grp//0=tl;1=tl;
+ non_default_ctrl_mon_grp/child_non_default_mon_grp/0=_;1=_;
+
"max_threshold_occupancy":
Read/write file provides the largest value (in
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 9fd37b6c3b24..7f8b1386287a 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -958,6 +958,30 @@ static char *mon_state_to_str(int mon_state, char *str)
return str;
}
+static int str_to_mon_state(char *flag)
+{
+ int i, mon_state = 0;
+
+ for (i = 0; i < strlen(flag); i++) {
+ switch (*(flag + i)) {
+ case 't':
+ mon_state |= ASSIGN_TOTAL;
+ break;
+ case 'l':
+ mon_state |= ASSIGN_LOCAL;
+ break;
+ case '_':
+ mon_state = ASSIGN_NONE;
+ break;
+ default:
+ mon_state = ASSIGN_NONE;
+ break;
+ }
+ }
+
+ return mon_state;
+}
+
static int rdtgroup_mbm_assign_control_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -1011,6 +1035,215 @@ static int rdtgroup_mbm_assign_control_show(struct kernfs_open_file *of,
return 0;
}
+static struct rdtgroup *resctrl_get_rdtgroup(enum rdt_group_type rtype, char *p_grp, char *c_grp)
+{
+ struct rdtgroup *rdtg, *crg;
+
+ if (rtype == RDTCTRL_GROUP && *p_grp == '\0') {
+ return &rdtgroup_default;
+ } else if (rtype == RDTCTRL_GROUP) {
+ list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list)
+ if (!strcmp(p_grp, rdtg->kn->name))
+ return rdtg;
+ } else if (rtype == RDTMON_GROUP) {
+ list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
+ if (!strcmp(p_grp, rdtg->kn->name)) {
+ list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
+ mon.crdtgrp_list) {
+ if (!strcmp(c_grp, crg->kn->name))
+ return crg;
+ }
+ }
+ }
+ }
+
+ return NULL;
+}
+
+static int resctrl_process_flags(enum rdt_group_type rtype, char *p_grp, char *c_grp, char *tok)
+{
+ struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
+ int op, mon_state, assign_state, unassign_state;
+ char *dom_str, *id_str, *op_str;
+ struct rdtgroup *rdt_grp;
+ struct rdt_domain *d;
+ unsigned long dom_id;
+ int ret, found = 0;
+
+ rdt_grp = resctrl_get_rdtgroup(rtype, p_grp, c_grp);
+
+ if (!rdt_grp) {
+ rdt_last_cmd_puts("Not a valid resctrl group\n");
+ return -EINVAL;
+ }
+
+next:
+ if (!tok || tok[0] == '\0')
+ return 0;
+
+ /* Start processing the strings for each domain */
+ dom_str = strim(strsep(&tok, ";"));
+
+ op_str = strpbrk(dom_str, "=+-_");
+
+ if (op_str) {
+ op = *op_str;
+ } else {
+ rdt_last_cmd_puts("Missing operation =, +, -, _ character\n");
+ return -EINVAL;
+ }
+
+ id_str = strsep(&dom_str, "=+-_");
+
+ if (!id_str || kstrtoul(id_str, 10, &dom_id)) {
+ rdt_last_cmd_puts("Missing domain id\n");
+ return -EINVAL;
+ }
+
+ /* Verify if the dom_id is valid */
+ list_for_each_entry(d, &r->domains, list) {
+ if (d->id == dom_id) {
+ found = 1;
+ break;
+ }
+ }
+ if (!found) {
+ rdt_last_cmd_printf("Invalid domain id %ld\n", dom_id);
+ return -EINVAL;
+ }
+
+ if (op != '_')
+ mon_state = str_to_mon_state(dom_str);
+
+ assign_state = 0;
+ unassign_state = 0;
+
+ switch (op) {
+ case '+':
+ assign_state = mon_state;
+ break;
+ case '-':
+ unassign_state = mon_state;
+ break;
+ case '=':
+ assign_state = mon_state;
+ unassign_state = (ASSIGN_TOTAL | ASSIGN_LOCAL) & ~assign_state;
+ break;
+ case '_':
+ unassign_state = ASSIGN_TOTAL | ASSIGN_LOCAL;
+ break;
+ default:
+ break;
+ }
+
+ if (assign_state & ASSIGN_TOTAL)
+ ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
+ ASSIGN_TOTAL);
+ if (ret)
+ goto out_fail;
+
+ if (assign_state & ASSIGN_LOCAL)
+ ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
+ ASSIGN_LOCAL);
+
+ if (ret)
+ goto out_fail;
+
+ if (unassign_state & ASSIGN_TOTAL)
+ ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
+ ASSIGN_TOTAL);
+ if (ret)
+ goto out_fail;
+
+ if (unassign_state & ASSIGN_LOCAL)
+ ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
+ ASSIGN_LOCAL);
+ if (ret)
+ goto out_fail;
+
+ goto next;
+
+out_fail:
+
+ return -EINVAL;
+}
+
+static ssize_t rdtgroup_mbm_assign_control_write(struct kernfs_open_file *of,
+ char *buf, size_t nbytes, loff_t off)
+{
+ struct rdt_resource *r = of->kn->parent->priv;
+ char *token, *cmon_grp, *mon_grp;
+ struct rdt_hw_resource *hw_res;
+ int ret;
+
+ hw_res = resctrl_to_arch_res(r);
+ if (!hw_res->abmc_enabled)
+ return -EINVAL;
+
+ /* Valid input requires a trailing newline */
+ if (nbytes == 0 || buf[nbytes - 1] != '\n')
+ return -EINVAL;
+
+ buf[nbytes - 1] = '\0';
+ rdt_last_cmd_clear();
+
+ cpus_read_lock();
+ mutex_lock(&rdtgroup_mutex);
+
+ while ((token = strsep(&buf, "\n")) != NULL) {
+ if (strstr(token, "//")) {
+ /*
+ * The control mon group processing:
+ * default CTRL_MON group: "//<flags>"
+ * non-default CTRL_MON group: "<CTRL_MON group>//flags"
+ * The CTRL_MON group will be empty string if it is a
+ * default group.
+ */
+ cmon_grp = strsep(&token, "//");
+
+ /*
+ * strsep returns empty string for contiguous delimiters.
+ * Make sure check for two consicutive delimiters and
+ * advance the token.
+ */
+ mon_grp = strsep(&token, "//");
+ if (*mon_grp != '\0') {
+ rdt_last_cmd_printf("Invalid CTRL_MON group format %s\n", token);
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = resctrl_process_flags(RDTCTRL_GROUP, cmon_grp, mon_grp, token);
+ if (ret)
+ break;
+ } else if (strstr(token, "/")) {
+ /*
+ * Mon group processing:
+ * MON_GROUP inside default CTRL_MON group: "/<MON group>/<flags>"
+ * MON_GROUP within CTRL_MON group: "<CTRL_MON group>/<MON group>/<flags>"
+ */
+ cmon_grp = strsep(&token, "/");
+
+ /* Extract the MON_GROUP. It cannot be empty string */
+ mon_grp = strsep(&token, "/");
+ if (*mon_grp == '\0') {
+ rdt_last_cmd_printf("Invalid MON_GROUP format %s\n", token);
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = resctrl_process_flags(RDTMON_GROUP, cmon_grp, mon_grp, token);
+ if (ret)
+ break;
+ }
+ }
+
+ mutex_unlock(&rdtgroup_mutex);
+ cpus_read_unlock();
+
+ return ret ?: nbytes;
+}
+
static int rdtgroup_mbm_assign_cntrs_show(struct kernfs_open_file *of,
struct seq_file *s, void *v)
{
@@ -2200,9 +2433,10 @@ static struct rftype res_common_files[] = {
},
{
.name = "mbm_assign_control",
- .mode = 0444,
+ .mode = 0644,
.kf_ops = &rdtgroup_kf_single_ops,
.seq_show = rdtgroup_mbm_assign_control_show,
+ .write = rdtgroup_mbm_assign_control_write,
},
{
.name = "mbm_assign_cntrs",
--
2.34.1Hi Babu,
On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote:
>
> Introduce rdtgroup_mbm_assign_control_write to assign mbm events.
> Assignment state can be updated by writing to this interface.
> Assignment states are applied on all the domains. Assignment on one
> domain applied on all the domains. User can pass one valid domain and
> assignment will be updated on all the available domains.
It sounds like you said the same thing 3 times in a row.
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index 2d96565501ab..64ec70637c66 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -328,6 +328,77 @@ with the following files:
> None of events are assigned on this mon group. This is a child
> monitor group of the non default control mon group.
>
> + Assignment state can be updated by writing to this interface.
> +
> + NOTE: Assignment on one domain applied on all the domains. User can
> + pass one valid domain and assignment will be updated on all the
> + available domains.
How would different assignments to different domains work? If the
allocations are global, then the allocated monitor ID is available to
all domains whether they use it or not.
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 9fd37b6c3b24..7f8b1386287a 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -1011,6 +1035,215 @@ static int rdtgroup_mbm_assign_control_show(struct kernfs_open_file *of,
> return 0;
> }
>
> +static struct rdtgroup *resctrl_get_rdtgroup(enum rdt_group_type rtype, char *p_grp, char *c_grp)
> +{
> + struct rdtgroup *rdtg, *crg;
> +
> + if (rtype == RDTCTRL_GROUP && *p_grp == '\0') {
> + return &rdtgroup_default;
> + } else if (rtype == RDTCTRL_GROUP) {
> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list)
> + if (!strcmp(p_grp, rdtg->kn->name))
> + return rdtg;
> + } else if (rtype == RDTMON_GROUP) {
> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
> + if (!strcmp(p_grp, rdtg->kn->name)) {
> + list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
> + mon.crdtgrp_list) {
> + if (!strcmp(c_grp, crg->kn->name))
> + return crg;
> + }
> + }
> + }
> + }
> +
> + return NULL;
> +}
> +
> +static int resctrl_process_flags(enum rdt_group_type rtype, char *p_grp, char *c_grp, char *tok)
> +{
> + struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + int op, mon_state, assign_state, unassign_state;
> + char *dom_str, *id_str, *op_str;
> + struct rdtgroup *rdt_grp;
> + struct rdt_domain *d;
> + unsigned long dom_id;
> + int ret, found = 0;
> +
> + rdt_grp = resctrl_get_rdtgroup(rtype, p_grp, c_grp);
> +
> + if (!rdt_grp) {
> + rdt_last_cmd_puts("Not a valid resctrl group\n");
> + return -EINVAL;
> + }
> +
> +next:
> + if (!tok || tok[0] == '\0')
> + return 0;
> +
> + /* Start processing the strings for each domain */
> + dom_str = strim(strsep(&tok, ";"));
> +
> + op_str = strpbrk(dom_str, "=+-_");
> +
> + if (op_str) {
> + op = *op_str;
> + } else {
> + rdt_last_cmd_puts("Missing operation =, +, -, _ character\n");
> + return -EINVAL;
> + }
> +
> + id_str = strsep(&dom_str, "=+-_");
> +
> + if (!id_str || kstrtoul(id_str, 10, &dom_id)) {
> + rdt_last_cmd_puts("Missing domain id\n");
> + return -EINVAL;
> + }
> +
> + /* Verify if the dom_id is valid */
> + list_for_each_entry(d, &r->domains, list) {
> + if (d->id == dom_id) {
> + found = 1;
> + break;
> + }
> + }
> + if (!found) {
> + rdt_last_cmd_printf("Invalid domain id %ld\n", dom_id);
> + return -EINVAL;
> + }
> +
> + if (op != '_')
> + mon_state = str_to_mon_state(dom_str);
> +
> + assign_state = 0;
> + unassign_state = 0;
> +
> + switch (op) {
> + case '+':
> + assign_state = mon_state;
> + break;
> + case '-':
> + unassign_state = mon_state;
> + break;
> + case '=':
> + assign_state = mon_state;
> + unassign_state = (ASSIGN_TOTAL | ASSIGN_LOCAL) & ~assign_state;
> + break;
> + case '_':
> + unassign_state = ASSIGN_TOTAL | ASSIGN_LOCAL;
> + break;
> + default:
> + break;
> + }
> +
> + if (assign_state & ASSIGN_TOTAL)
> + ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
> + ASSIGN_TOTAL);
Related to my comments yesterday[1], it seems redundant for an
interface to need two names for the same event.
> + if (ret)
> + goto out_fail;
> +
> + if (assign_state & ASSIGN_LOCAL)
> + ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
> + ASSIGN_LOCAL);
> +
> + if (ret)
> + goto out_fail;
> +
> + if (unassign_state & ASSIGN_TOTAL)
> + ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
> + ASSIGN_TOTAL);
> + if (ret)
> + goto out_fail;
> +
> + if (unassign_state & ASSIGN_LOCAL)
> + ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
> + ASSIGN_LOCAL);
> + if (ret)
> + goto out_fail;
> +
> + goto next;
I saw that each call to rdtgroup_assign_abmc() allocates a counter.
Does that mean assigning to multiple domains (in the same or multiple
commands) allocates a new counter (or pair of counters) in every
domain?
Thanks!
-Peter
[1] https://lore.kernel.org/lkml/CALPaoCj_yb_muT78jFQ5gL0wkifohSAVwxMDTm2FX_2YVpANdw@mail.gmail.com/
Hi Peter,
On 4/17/24 12:45, Peter Newman wrote:
> Hi Babu,
>
> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote:
>>
>> Introduce rdtgroup_mbm_assign_control_write to assign mbm events.
>> Assignment state can be updated by writing to this interface.
>> Assignment states are applied on all the domains. Assignment on one
>> domain applied on all the domains. User can pass one valid domain and
>> assignment will be updated on all the available domains.
>
> It sounds like you said the same thing 3 times in a row.
Sure. Will change it. With the introduction of domain specific assignment,
I can change it to something like this below.
------------------
"Introduce rdtgroup_mbm_assign_control_write to assign mbm events.
By default, the assignment is applied on all the domains when a new group
is created if the hardware counter is available at the time. This
interface provides the option to modify the assignment specific to each
domain."
------------------
>
>
>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>> index 2d96565501ab..64ec70637c66 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -328,6 +328,77 @@ with the following files:
>> None of events are assigned on this mon group. This is a child
>> monitor group of the non default control mon group.
>>
>> + Assignment state can be updated by writing to this interface.
>> +
>> + NOTE: Assignment on one domain applied on all the domains. User can
>> + pass one valid domain and assignment will be updated on all the
>> + available domains.
>
> How would different assignments to different domains work? If the
> allocations are global, then the allocated monitor ID is available to
> all domains whether they use it or not.
That is correct.
[A] Hardware counters(max 2 per group) are allocated at the group level.
So, those counters are available to all the domains on that group. I will
maintain a bitmap at the domain level. The bitmap will be set on the
domains where assignment is applied and IPIs are sent. IPIs will not be
sent to other domains.
>
>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 9fd37b6c3b24..7f8b1386287a 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -1011,6 +1035,215 @@ static int rdtgroup_mbm_assign_control_show(struct kernfs_open_file *of,
>> return 0;
>> }
>>
>> +static struct rdtgroup *resctrl_get_rdtgroup(enum rdt_group_type rtype, char *p_grp, char *c_grp)
>> +{
>> + struct rdtgroup *rdtg, *crg;
>> +
>> + if (rtype == RDTCTRL_GROUP && *p_grp == '\0') {
>> + return &rdtgroup_default;
>> + } else if (rtype == RDTCTRL_GROUP) {
>> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list)
>> + if (!strcmp(p_grp, rdtg->kn->name))
>> + return rdtg;
>> + } else if (rtype == RDTMON_GROUP) {
>> + list_for_each_entry(rdtg, &rdt_all_groups, rdtgroup_list) {
>> + if (!strcmp(p_grp, rdtg->kn->name)) {
>> + list_for_each_entry(crg, &rdtg->mon.crdtgrp_list,
>> + mon.crdtgrp_list) {
>> + if (!strcmp(c_grp, crg->kn->name))
>> + return crg;
>> + }
>> + }
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static int resctrl_process_flags(enum rdt_group_type rtype, char *p_grp, char *c_grp, char *tok)
>> +{
>> + struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
>> + int op, mon_state, assign_state, unassign_state;
>> + char *dom_str, *id_str, *op_str;
>> + struct rdtgroup *rdt_grp;
>> + struct rdt_domain *d;
>> + unsigned long dom_id;
>> + int ret, found = 0;
>> +
>> + rdt_grp = resctrl_get_rdtgroup(rtype, p_grp, c_grp);
>> +
>> + if (!rdt_grp) {
>> + rdt_last_cmd_puts("Not a valid resctrl group\n");
>> + return -EINVAL;
>> + }
>> +
>> +next:
>> + if (!tok || tok[0] == '\0')
>> + return 0;
>> +
>> + /* Start processing the strings for each domain */
>> + dom_str = strim(strsep(&tok, ";"));
>> +
>> + op_str = strpbrk(dom_str, "=+-_");
>> +
>> + if (op_str) {
>> + op = *op_str;
>> + } else {
>> + rdt_last_cmd_puts("Missing operation =, +, -, _ character\n");
>> + return -EINVAL;
>> + }
>> +
>> + id_str = strsep(&dom_str, "=+-_");
>> +
>> + if (!id_str || kstrtoul(id_str, 10, &dom_id)) {
>> + rdt_last_cmd_puts("Missing domain id\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Verify if the dom_id is valid */
>> + list_for_each_entry(d, &r->domains, list) {
>> + if (d->id == dom_id) {
>> + found = 1;
>> + break;
>> + }
>> + }
>> + if (!found) {
>> + rdt_last_cmd_printf("Invalid domain id %ld\n", dom_id);
>> + return -EINVAL;
>> + }
>> +
>> + if (op != '_')
>> + mon_state = str_to_mon_state(dom_str);
>> +
>> + assign_state = 0;
>> + unassign_state = 0;
>> +
>> + switch (op) {
>> + case '+':
>> + assign_state = mon_state;
>> + break;
>> + case '-':
>> + unassign_state = mon_state;
>> + break;
>> + case '=':
>> + assign_state = mon_state;
>> + unassign_state = (ASSIGN_TOTAL | ASSIGN_LOCAL) & ~assign_state;
>> + break;
>> + case '_':
>> + unassign_state = ASSIGN_TOTAL | ASSIGN_LOCAL;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + if (assign_state & ASSIGN_TOTAL)
>> + ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
>> + ASSIGN_TOTAL);
>
> Related to my comments yesterday[1], it seems redundant for an
> interface to need two names for the same event.
Yea. I will remove one of this parameter.
>
>
>> + if (ret)
>> + goto out_fail;
>> +
>> + if (assign_state & ASSIGN_LOCAL)
>> + ret = rdtgroup_assign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
>> + ASSIGN_LOCAL);
>> +
>> + if (ret)
>> + goto out_fail;
>> +
>> + if (unassign_state & ASSIGN_TOTAL)
>> + ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_TOTAL_EVENT_ID,
>> + ASSIGN_TOTAL);
>> + if (ret)
>> + goto out_fail;
>> +
>> + if (unassign_state & ASSIGN_LOCAL)
>> + ret = rdtgroup_unassign_abmc(rdt_grp, QOS_L3_MBM_LOCAL_EVENT_ID,
>> + ASSIGN_LOCAL);
>> + if (ret)
>> + goto out_fail;
>> +
>> + goto next;
>
> I saw that each call to rdtgroup_assign_abmc() allocates a counter.
> Does that mean assigning to multiple domains (in the same or multiple
> commands) allocates a new counter (or pair of counters) in every
> domain?
No. Counters allocation is at group level which is global. Will maintain a
bitmap at the domain to determine if the counter is assigned or unassigned
at the specific domain. Please see the comment above [A].
>
> Thanks!
> -Peter
>
> [1] https://lore.kernel.org/lkml/CALPaoCj_yb_muT78jFQ5gL0wkifohSAVwxMDTm2FX_2YVpANdw@mail.gmail.com/
--
Thanks
Babu Moger
Hi Babu, On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: > On 4/17/24 12:45, Peter Newman wrote: > > On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: > >> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst > >> index 2d96565501ab..64ec70637c66 100644 > >> --- a/Documentation/arch/x86/resctrl.rst > >> +++ b/Documentation/arch/x86/resctrl.rst > >> @@ -328,6 +328,77 @@ with the following files: > >> None of events are assigned on this mon group. This is a child > >> monitor group of the non default control mon group. > >> > >> + Assignment state can be updated by writing to this interface. > >> + > >> + NOTE: Assignment on one domain applied on all the domains. User can > >> + pass one valid domain and assignment will be updated on all the > >> + available domains. > > > > How would different assignments to different domains work? If the > > allocations are global, then the allocated monitor ID is available to > > all domains whether they use it or not. > > That is correct. > [A] Hardware counters(max 2 per group) are allocated at the group level. > So, those counters are available to all the domains on that group. I will > maintain a bitmap at the domain level. The bitmap will be set on the > domains where assignment is applied and IPIs are sent. IPIs will not be > sent to other domains. Unless the monitor allocation is scoped at the domain level, I don't see much point in implementing the per-domain parsing today, as the only benefit is avoiding IPIs to domains whose counters you don't plan to read. -Peter
Hi Peter, On 4/17/2024 3:56 PM, Peter Newman wrote: > Hi Babu, > > On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: >> On 4/17/24 12:45, Peter Newman wrote: >>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: >>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >>>> index 2d96565501ab..64ec70637c66 100644 >>>> --- a/Documentation/arch/x86/resctrl.rst >>>> +++ b/Documentation/arch/x86/resctrl.rst >>>> @@ -328,6 +328,77 @@ with the following files: >>>> None of events are assigned on this mon group. This is a child >>>> monitor group of the non default control mon group. >>>> >>>> + Assignment state can be updated by writing to this interface. >>>> + >>>> + NOTE: Assignment on one domain applied on all the domains. User can >>>> + pass one valid domain and assignment will be updated on all the >>>> + available domains. >>> How would different assignments to different domains work? If the >>> allocations are global, then the allocated monitor ID is available to >>> all domains whether they use it or not. >> That is correct. >> [A] Hardware counters(max 2 per group) are allocated at the group level. >> So, those counters are available to all the domains on that group. I will >> maintain a bitmap at the domain level. The bitmap will be set on the >> domains where assignment is applied and IPIs are sent. IPIs will not be >> sent to other domains. > Unless the monitor allocation is scoped at the domain level, I don't > see much point in implementing the per-domain parsing today, as the > only benefit is avoiding IPIs to domains whose counters you don't plan > to read. In that case lets remove the domain specific assignments. We can avoid some code complexity. thanks Babu
Hi Babu, On 4/17/2024 3:52 PM, Moger, Babu wrote: > Hi Peter, > > On 4/17/2024 3:56 PM, Peter Newman wrote: >> Hi Babu, >> >> On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: >>> On 4/17/24 12:45, Peter Newman wrote: >>>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: >>>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >>>>> index 2d96565501ab..64ec70637c66 100644 >>>>> --- a/Documentation/arch/x86/resctrl.rst >>>>> +++ b/Documentation/arch/x86/resctrl.rst >>>>> @@ -328,6 +328,77 @@ with the following files: >>>>> None of events are assigned on this mon group. This is a child >>>>> monitor group of the non default control mon group. >>>>> >>>>> + Assignment state can be updated by writing to this interface. >>>>> + >>>>> + NOTE: Assignment on one domain applied on all the domains. User can >>>>> + pass one valid domain and assignment will be updated on all the >>>>> + available domains. >>>> How would different assignments to different domains work? If the >>>> allocations are global, then the allocated monitor ID is available to >>>> all domains whether they use it or not. >>> That is correct. >>> [A] Hardware counters(max 2 per group) are allocated at the group level. >>> So, those counters are available to all the domains on that group. I will >>> maintain a bitmap at the domain level. The bitmap will be set on the >>> domains where assignment is applied and IPIs are sent. IPIs will not be >>> sent to other domains. >> Unless the monitor allocation is scoped at the domain level, I don't >> see much point in implementing the per-domain parsing today, as the >> only benefit is avoiding IPIs to domains whose counters you don't plan >> to read. > > In that case lets remove the domain specific assignments. We can avoid some code complexity. > As I understand counters are scoped at the domain level and it is an implementation choice to make the allocation global. (Similar to the decision to make CLOSIDs global.) Could you please elaborate how you plan to remove domain specific assignments? I do think it needs to remain as part of the user interface so I wonder if this may look like only "*=<flags>" is supported on these systems and attempting to assign an individual domain may fail with "not supported". Reinette
Hi Reinette, Email issues. Responding again.. On 5/2/2024 6:00 PM, Reinette Chatre wrote: > Hi Babu, > > On 4/17/2024 3:52 PM, Moger, Babu wrote: >> Hi Peter, >> >> On 4/17/2024 3:56 PM, Peter Newman wrote: >>> Hi Babu, >>> >>> On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: >>>> On 4/17/24 12:45, Peter Newman wrote: >>>>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: >>>>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >>>>>> index 2d96565501ab..64ec70637c66 100644 >>>>>> --- a/Documentation/arch/x86/resctrl.rst >>>>>> +++ b/Documentation/arch/x86/resctrl.rst >>>>>> @@ -328,6 +328,77 @@ with the following files: >>>>>> None of events are assigned on this mon group. This is a child >>>>>> monitor group of the non default control mon group. >>>>>> >>>>>> + Assignment state can be updated by writing to this interface. >>>>>> + >>>>>> + NOTE: Assignment on one domain applied on all the domains. User can >>>>>> + pass one valid domain and assignment will be updated on all the >>>>>> + available domains. >>>>> How would different assignments to different domains work? If the >>>>> allocations are global, then the allocated monitor ID is available to >>>>> all domains whether they use it or not. >>>> That is correct. >>>> [A] Hardware counters(max 2 per group) are allocated at the group level. >>>> So, those counters are available to all the domains on that group. I will >>>> maintain a bitmap at the domain level. The bitmap will be set on the >>>> domains where assignment is applied and IPIs are sent. IPIs will not be >>>> sent to other domains. >>> Unless the monitor allocation is scoped at the domain level, I don't >>> see much point in implementing the per-domain parsing today, as the >>> only benefit is avoiding IPIs to domains whose counters you don't plan >>> to read. >> >> In that case lets remove the domain specific assignments. We can avoid some code complexity. >> > > As I understand counters are scoped at the domain level and it is > an implementation choice to make the allocation global. (Similar to > the decision to make CLOSIDs global.) > > Could you please elaborate how you plan to remove domain specific > assignments? I do think it needs to remain as part of the user interface > so I wonder if this may look like only "*=<flags>" is supported on > these systems and attempting to assign an individual domain may fail > with "not supported". This series applies the assignment to all the domains. For example: # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control User here wants to assign a monitor to total event on domain 0. But this series applies monitor to all the domains in the system. IPIs will be sent to all the domains. Basically this is equivalent to # echo "//*=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control I was thinking of adding domain specific assignment in next version. That involves adding a new field in rdt_domain to keep track of assignment. Peter suggested it may not be much of a value add for his usage model. Thanks - Babu Moger
Hi Babu, On 5/3/2024 9:14 AM, Moger, Babu wrote: > On 5/2/2024 6:00 PM, Reinette Chatre wrote: >> On 4/17/2024 3:52 PM, Moger, Babu wrote: >>> On 4/17/2024 3:56 PM, Peter Newman wrote: >>>> On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: >>>>> On 4/17/24 12:45, Peter Newman wrote: >>>>>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: >>>>>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >>>>>>> index 2d96565501ab..64ec70637c66 100644 >>>>>>> --- a/Documentation/arch/x86/resctrl.rst >>>>>>> +++ b/Documentation/arch/x86/resctrl.rst >>>>>>> @@ -328,6 +328,77 @@ with the following files: >>>>>>> None of events are assigned on this mon group. This is a child >>>>>>> monitor group of the non default control mon group. >>>>>>> >>>>>>> + Assignment state can be updated by writing to this interface. >>>>>>> + >>>>>>> + NOTE: Assignment on one domain applied on all the domains. User can >>>>>>> + pass one valid domain and assignment will be updated on all the >>>>>>> + available domains. >>>>>> How would different assignments to different domains work? If the >>>>>> allocations are global, then the allocated monitor ID is available to >>>>>> all domains whether they use it or not. >>>>> That is correct. >>>>> [A] Hardware counters(max 2 per group) are allocated at the group level. >>>>> So, those counters are available to all the domains on that group. I will >>>>> maintain a bitmap at the domain level. The bitmap will be set on the >>>>> domains where assignment is applied and IPIs are sent. IPIs will not be >>>>> sent to other domains. >>>> Unless the monitor allocation is scoped at the domain level, I don't >>>> see much point in implementing the per-domain parsing today, as the >>>> only benefit is avoiding IPIs to domains whose counters you don't plan >>>> to read. >>> >>> In that case lets remove the domain specific assignments. We can avoid some code complexity. >>> >> >> As I understand counters are scoped at the domain level and it is >> an implementation choice to make the allocation global. (Similar to >> the decision to make CLOSIDs global.) >> >> Could you please elaborate how you plan to remove domain specific >> assignments? I do think it needs to remain as part of the user interface >> so I wonder if this may look like only "*=<flags>" is supported on >> these systems and attempting to assign an individual domain may fail >> with "not supported". > > This series applies the assignment to all the domains. > > For example: > > # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control > > User here wants to assign a monitor to total event on domain 0. > But this series applies monitor to all the domains in the system. IPIs will be sent to all the domains. I would like to recommend against this. (a) this is not what the API says will happen, (b) behavior like this may result in users having scripts with syntax like above expecting changes to all domains and when/if AMD or another architecture decides to implement per-domain assignment it will break user space. > Basically this is equivalent to > > # echo "//*=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control > > > I was thinking of adding domain specific assignment in next version. > That involves adding a new field in rdt_domain to keep track of > assignment. Peter suggested it may not be much of a value add for his > usage model. I do not have insight into how all users will end up using this. Reinette
Hi Reinette, On 5/3/24 16:16, Reinette Chatre wrote: > Hi Babu, > > On 5/3/2024 9:14 AM, Moger, Babu wrote: >> On 5/2/2024 6:00 PM, Reinette Chatre wrote: >>> On 4/17/2024 3:52 PM, Moger, Babu wrote: >>>> On 4/17/2024 3:56 PM, Peter Newman wrote: >>>>> On Wed, Apr 17, 2024 at 12:39 PM Moger, Babu <babu.moger@amd.com> wrote: >>>>>> On 4/17/24 12:45, Peter Newman wrote: >>>>>>> On Thu, Mar 28, 2024 at 6:10 PM Babu Moger <babu.moger@amd.com> wrote: >>>>>>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >>>>>>>> index 2d96565501ab..64ec70637c66 100644 >>>>>>>> --- a/Documentation/arch/x86/resctrl.rst >>>>>>>> +++ b/Documentation/arch/x86/resctrl.rst >>>>>>>> @@ -328,6 +328,77 @@ with the following files: >>>>>>>> None of events are assigned on this mon group. This is a child >>>>>>>> monitor group of the non default control mon group. >>>>>>>> >>>>>>>> + Assignment state can be updated by writing to this interface. >>>>>>>> + >>>>>>>> + NOTE: Assignment on one domain applied on all the domains. User can >>>>>>>> + pass one valid domain and assignment will be updated on all the >>>>>>>> + available domains. >>>>>>> How would different assignments to different domains work? If the >>>>>>> allocations are global, then the allocated monitor ID is available to >>>>>>> all domains whether they use it or not. >>>>>> That is correct. >>>>>> [A] Hardware counters(max 2 per group) are allocated at the group level. >>>>>> So, those counters are available to all the domains on that group. I will >>>>>> maintain a bitmap at the domain level. The bitmap will be set on the >>>>>> domains where assignment is applied and IPIs are sent. IPIs will not be >>>>>> sent to other domains. >>>>> Unless the monitor allocation is scoped at the domain level, I don't >>>>> see much point in implementing the per-domain parsing today, as the >>>>> only benefit is avoiding IPIs to domains whose counters you don't plan >>>>> to read. >>>> >>>> In that case lets remove the domain specific assignments. We can avoid some code complexity. >>>> >>> >>> As I understand counters are scoped at the domain level and it is >>> an implementation choice to make the allocation global. (Similar to >>> the decision to make CLOSIDs global.) >>> >>> Could you please elaborate how you plan to remove domain specific >>> assignments? I do think it needs to remain as part of the user interface >>> so I wonder if this may look like only "*=<flags>" is supported on >>> these systems and attempting to assign an individual domain may fail >>> with "not supported". >> >> This series applies the assignment to all the domains. >> >> For example: >> >> # echo "//0=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control >> >> User here wants to assign a monitor to total event on domain 0. >> But this series applies monitor to all the domains in the system. IPIs will be sent to all the domains. > > I would like to recommend against this. (a) this is not what the API > says will happen, (b) behavior like this may result in users having scripts > with syntax like above expecting changes to all domains and when/if > AMD or another architecture decides to implement per-domain assignment > it will break user space. Sure. We keep per-domain specific assignment. "//0=t" This will apply assignment only domain 0. "//*=t" This will apply assignment one all the domains. Hope it clarifies. > >> Basically this is equivalent to >> >> # echo "//*=t" > /sys/fs/resctrl/info/L3_MON/mbm_assign_control >> >> >> I was thinking of adding domain specific assignment in next version. >> That involves adding a new field in rdt_domain to keep track of >> assignment. Peter suggested it may not be much of a value add for his >> usage model. > > I do not have insight into how all users will end up using this. > > Reinette > -- Thanks Babu Moger
© 2016 - 2026 Red Hat, Inc.