* [RFC PATCH 0/3] x86/resctrl: Miscellaneous resctrl features @ 2023-01-03 22:06 Babu Moger 2023-01-03 22:06 ` [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Babu Moger @ 2023-01-03 22:06 UTC (permalink / raw) To: fenghua.yu, reinette.chatre Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman These series adds support for 3 minor features. 1. Support assigning multiple tasks to control/mon groups 2. Detect and move task's threads automatically to the groups 3. Add RMID and CLOSID in resctrl inteface. These feature requests are coming from our test team. They have been asking me to add these features to QoS for a while. Please review and comment if these changes make sense. Thanks --- Babu Moger (3): x86/resctrl: Add multiple tasks to the resctrl group at once x86/resctrl: Move the task's threads to the group automatically x86/resctrl: Display the RMID and COSID for resctrl groups Documentation/x86/resctrl.rst | 28 ++++++-- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 90 +++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 14 deletions(-) -- ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once 2023-01-03 22:06 [RFC PATCH 0/3] x86/resctrl: Miscellaneous resctrl features Babu Moger @ 2023-01-03 22:06 ` Babu Moger 2023-01-04 5:46 ` Yu, Fenghua 2023-01-03 22:06 ` [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically Babu Moger 2023-01-03 22:06 ` [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger 2 siblings, 1 reply; 16+ messages in thread From: Babu Moger @ 2023-01-03 22:06 UTC (permalink / raw) To: fenghua.yu, reinette.chatre Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman Right now, the resctrl task assignment for the MONITOR or CONTROL group needs to be one at a time. For example: $mount -t resctrl resctrl /sys/fs/resctrl/ $mkdir /sys/fs/resctrl/clos1 $echo 123 > /sys/fs/resctrl/clos1/tasks $echo 456 > /sys/fs/resctrl/clos1/tasks $echo 789 > /sys/fs/resctrl/clos1/tasks This is not user-friendly when dealing with hundreds of tasks. Improve the user experience by supporting the multiple task assignment in one command with tasks separated by commas. For example: $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks Signed-off-by: Babu Moger <babu.moger@amd.com> --- Documentation/x86/resctrl.rst | 13 ++++++------ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 35 ++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst index 71a531061e4e..f26e16412bcb 100644 --- a/Documentation/x86/resctrl.rst +++ b/Documentation/x86/resctrl.rst @@ -208,12 +208,13 @@ All groups contain the following files: "tasks": Reading this file shows the list of all tasks that belong to this group. Writing a task id to the file will add a task to the - group. If the group is a CTRL_MON group the task is removed from - whichever previous CTRL_MON group owned the task and also from - any MON group that owned the task. If the group is a MON group, - then the task must already belong to the CTRL_MON parent of this - group. The task is removed from any previous MON group. - + group. Multiple tasks can be assigned at once with each task + separated by commas. If the group is a CTRL_MON group the task + is removed from whichever previous CTRL_MON group owned the task + and also from any MON group that owned the task. If the group is + a MON group, then the task must already belong to the CTRL_MON + parent of this group. The task is removed from any previous MON + group. "cpus": Reading this file shows a bitmask of the logical CPUs owned by diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index e5a48f05e787..344607853f4c 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -686,28 +686,49 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { struct rdtgroup *rdtgrp; + char *pid_str; int ret = 0; pid_t pid; - if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0) + /* Valid input requires a trailing newline */ + if (nbytes == 0 || buf[nbytes - 1] != '\n') return -EINVAL; + + buf[nbytes - 1] = '\0'; + + cpus_read_lock(); rdtgrp = rdtgroup_kn_lock_live(of->kn); if (!rdtgrp) { - rdtgroup_kn_unlock(of->kn); - return -ENOENT; + ret = -ENOENT; + goto exit; + } + +next: + if (!buf || buf[0] == '\0') + goto exit; + + pid_str = strim(strsep(&buf, ",")); + + if (kstrtoint(pid_str, 0, &pid) || pid < 0) { + ret = -EINVAL; + goto exit; } + rdt_last_cmd_clear(); if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED || - rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { - ret = -EINVAL; + rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { rdt_last_cmd_puts("Pseudo-locking in progress\n"); - goto unlock; + ret = -EINVAL; + goto exit; } ret = rdtgroup_move_task(pid, rdtgrp, of); -unlock: + goto next; + +exit: + cpus_read_unlock(); rdtgroup_kn_unlock(of->kn); return ret ?: nbytes; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once 2023-01-03 22:06 ` [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger @ 2023-01-04 5:46 ` Yu, Fenghua 2023-01-04 17:20 ` Moger, Babu 0 siblings, 1 reply; 16+ messages in thread From: Yu, Fenghua @ 2023-01-04 5:46 UTC (permalink / raw) To: Babu Moger, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi, Babu, > Right now, the resctrl task assignment for the MONITOR or CONTROL group > needs to be one at a time. For example: > $mount -t resctrl resctrl /sys/fs/resctrl/ > $mkdir /sys/fs/resctrl/clos1 > $echo 123 > /sys/fs/resctrl/clos1/tasks > $echo 456 > /sys/fs/resctrl/clos1/tasks > $echo 789 > /sys/fs/resctrl/clos1/tasks > > This is not user-friendly when dealing with hundreds of tasks. > > Improve the user experience by supporting the multiple task assignment in one > command with tasks separated by commas. For example: > $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > Documentation/x86/resctrl.rst | 13 ++++++------ > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 35 ++++++++++++++++++++++++++-- > ---- > 2 files changed, 35 insertions(+), 13 deletions(-) > > diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst > index 71a531061e4e..f26e16412bcb 100644 > --- a/Documentation/x86/resctrl.rst > +++ b/Documentation/x86/resctrl.rst > @@ -208,12 +208,13 @@ All groups contain the following files: > "tasks": > Reading this file shows the list of all tasks that belong to > this group. Writing a task id to the file will add a task to the > - group. If the group is a CTRL_MON group the task is removed from > - whichever previous CTRL_MON group owned the task and also from > - any MON group that owned the task. If the group is a MON group, > - then the task must already belong to the CTRL_MON parent of this > - group. The task is removed from any previous MON group. > - > + group. Multiple tasks can be assigned at once with each task > + separated by commas. If the group is a CTRL_MON group the task > + is removed from whichever previous CTRL_MON group owned the task > + and also from any MON group that owned the task. If the group is > + a MON group, then the task must already belong to the CTRL_MON > + parent of this group. The task is removed from any previous MON > + group. Multiple tasks movement may fail in the middle. How to handle the failure in the middle? Abort on all previous success movements? Seems simple way is to exit from the failed task movement. That means all previous successful movements will not be reversed and all tasks won't be moved since the failure. Then this info needs to be explained in the doc. > > "cpus": > Reading this file shows a bitmask of the logical CPUs owned by diff --git > a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index e5a48f05e787..344607853f4c 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -686,28 +686,49 @@ static ssize_t rdtgroup_tasks_write(struct > kernfs_open_file *of, > char *buf, size_t nbytes, loff_t off) { > struct rdtgroup *rdtgrp; > + char *pid_str; > int ret = 0; > pid_t pid; > > - if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0) > + /* Valid input requires a trailing newline */ > + if (nbytes == 0 || buf[nbytes - 1] != '\n') > return -EINVAL; > + > + buf[nbytes - 1] = '\0'; > + > + cpus_read_lock(); > rdtgrp = rdtgroup_kn_lock_live(of->kn); > if (!rdtgrp) { > - rdtgroup_kn_unlock(of->kn); > - return -ENOENT; > + ret = -ENOENT; > + goto exit; > + } > + > +next: > + if (!buf || buf[0] == '\0') > + goto exit; > + > + pid_str = strim(strsep(&buf, ",")); > + > + if (kstrtoint(pid_str, 0, &pid) || pid < 0) { > + ret = -EINVAL; rdt_last_cmd_puts() to record the error. > + goto exit; > } > + > rdt_last_cmd_clear(); > > if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED || > - rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { > - ret = -EINVAL; > + rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { > rdt_last_cmd_puts("Pseudo-locking in progress\n"); > - goto unlock; > + ret = -EINVAL; > + goto exit; > } > > ret = rdtgroup_move_task(pid, rdtgrp, of); Do you want to exit if there is error in rdtgroup_move_task()? Otherwise, the failure won't be captured if later take movement succeeds. > > -unlock: > + goto next; > + > +exit: > + cpus_read_unlock(); > rdtgroup_kn_unlock(of->kn); > > return ret ?: nbytes; > Thanks. -Fenghua ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once 2023-01-04 5:46 ` Yu, Fenghua @ 2023-01-04 17:20 ` Moger, Babu 0 siblings, 0 replies; 16+ messages in thread From: Moger, Babu @ 2023-01-04 17:20 UTC (permalink / raw) To: Yu, Fenghua, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi Fenghua, On 1/3/23 23:46, Yu, Fenghua wrote: > Hi, Babu, > >> Right now, the resctrl task assignment for the MONITOR or CONTROL group >> needs to be one at a time. For example: >> $mount -t resctrl resctrl /sys/fs/resctrl/ >> $mkdir /sys/fs/resctrl/clos1 >> $echo 123 > /sys/fs/resctrl/clos1/tasks >> $echo 456 > /sys/fs/resctrl/clos1/tasks >> $echo 789 > /sys/fs/resctrl/clos1/tasks >> >> This is not user-friendly when dealing with hundreds of tasks. >> >> Improve the user experience by supporting the multiple task assignment in one >> command with tasks separated by commas. For example: >> $echo 123,456,789 > /sys/fs/resctrl/clos1/tasks >> >> Signed-off-by: Babu Moger <babu.moger@amd.com> >> --- >> Documentation/x86/resctrl.rst | 13 ++++++------ >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 35 ++++++++++++++++++++++++++-- >> ---- >> 2 files changed, 35 insertions(+), 13 deletions(-) >> >> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst >> index 71a531061e4e..f26e16412bcb 100644 >> --- a/Documentation/x86/resctrl.rst >> +++ b/Documentation/x86/resctrl.rst >> @@ -208,12 +208,13 @@ All groups contain the following files: >> "tasks": >> Reading this file shows the list of all tasks that belong to >> this group. Writing a task id to the file will add a task to the >> - group. If the group is a CTRL_MON group the task is removed from >> - whichever previous CTRL_MON group owned the task and also from >> - any MON group that owned the task. If the group is a MON group, >> - then the task must already belong to the CTRL_MON parent of this >> - group. The task is removed from any previous MON group. >> - >> + group. Multiple tasks can be assigned at once with each task >> + separated by commas. If the group is a CTRL_MON group the task >> + is removed from whichever previous CTRL_MON group owned the task >> + and also from any MON group that owned the task. If the group is >> + a MON group, then the task must already belong to the CTRL_MON >> + parent of this group. The task is removed from any previous MON >> + group. > Multiple tasks movement may fail in the middle. How to handle the failure > in the middle? Abort on all previous success movements? > > Seems simple way is to exit from the failed task movement. That means > all previous successful movements will not be reversed and all tasks won't > be moved since the failure. Yes. That is what even I am thinking. Exit on a failed movement and record the error. Don't need to reverse the successful movements. > > Then this info needs to be explained in the doc. Sure. >> "cpus": >> Reading this file shows a bitmask of the logical CPUs owned by diff --git >> a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index e5a48f05e787..344607853f4c 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -686,28 +686,49 @@ static ssize_t rdtgroup_tasks_write(struct >> kernfs_open_file *of, >> char *buf, size_t nbytes, loff_t off) { >> struct rdtgroup *rdtgrp; >> + char *pid_str; >> int ret = 0; >> pid_t pid; >> >> - if (kstrtoint(strstrip(buf), 0, &pid) || pid < 0) >> + /* Valid input requires a trailing newline */ >> + if (nbytes == 0 || buf[nbytes - 1] != '\n') >> return -EINVAL; >> + >> + buf[nbytes - 1] = '\0'; >> + >> + cpus_read_lock(); >> rdtgrp = rdtgroup_kn_lock_live(of->kn); >> if (!rdtgrp) { >> - rdtgroup_kn_unlock(of->kn); >> - return -ENOENT; >> + ret = -ENOENT; >> + goto exit; >> + } >> + >> +next: >> + if (!buf || buf[0] == '\0') >> + goto exit; >> + >> + pid_str = strim(strsep(&buf, ",")); >> + >> + if (kstrtoint(pid_str, 0, &pid) || pid < 0) { >> + ret = -EINVAL; > rdt_last_cmd_puts() to record the error. Sure. > >> + goto exit; >> } >> + >> rdt_last_cmd_clear(); >> >> if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED || >> - rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { >> - ret = -EINVAL; >> + rdtgrp->mode == RDT_MODE_PSEUDO_LOCKSETUP) { >> rdt_last_cmd_puts("Pseudo-locking in progress\n"); >> - goto unlock; >> + ret = -EINVAL; >> + goto exit; >> } >> >> ret = rdtgroup_move_task(pid, rdtgrp, of); > Do you want to exit if there is error in rdtgroup_move_task()? > Otherwise, the failure won't be captured if later take movement succeeds. Yes, that makes more sense. Exit on a failed movement and record the error. Thanks Babu > >> -unlock: >> + goto next; >> + >> +exit: >> + cpus_read_unlock(); >> rdtgroup_kn_unlock(of->kn); >> >> return ret ?: nbytes; >> > Thanks. > > -Fenghua -- Thanks Babu Moger ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically 2023-01-03 22:06 [RFC PATCH 0/3] x86/resctrl: Miscellaneous resctrl features Babu Moger 2023-01-03 22:06 ` [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger @ 2023-01-03 22:06 ` Babu Moger 2023-01-04 5:55 ` Yu, Fenghua 2023-01-04 16:43 ` Reinette Chatre 2023-01-03 22:06 ` [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger 2 siblings, 2 replies; 16+ messages in thread From: Babu Moger @ 2023-01-03 22:06 UTC (permalink / raw) To: fenghua.yu, reinette.chatre Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman Some micro benchmarks run multiple threads when started. Monitoring (or controlling) the benchmark using the task id is bit tricky. Users need to track all the threads and assign them individually to monitor or control. For example: $stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32 -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 $pidof stream_lowOverhead 6793 This benchmark actually runs multiple threads underneath on the cores listed above. It can be seen with the command: $ps -T -p 6793 PID SPID TTY TIME CMD 6793 6793 pts/2 00:00:00 stream_lowOverh 6793 6802 pts/2 00:01:25 stream_lowOverh 6793 6803 pts/2 00:01:25 stream_lowOverh 6793 6804 pts/2 00:01:25 stream_lowOverh 6793 6805 pts/2 00:01:25 stream_lowOverh Users need to assign these threads individually to the resctrl group for monitoring or controlling. $echo 6793 > /sys/fs/restrl/clos1/tasks $echo 6802 > /sys/fs/restrl/clos1/tasks $echo 6803 > /sys/fs/restrl/clos1/tasks $echo 6804 > /sys/fs/restrl/clos1/tasks $echo 6805 > /sys/fs/restrl/clos1/tasks That is not easy when dealing with numerous threads. Detect the task's threads automatically and assign them to the resctrl group when parent task is assigned. For example: $echo 6793 > /sys/fs/restrl/clos1/tasks All the threads will be assigned to the group automatically. $cat /sys/fs/restrl/clos1/tasks 6793 6793 6802 6803 6804 6805 Signed-off-by: Babu Moger <babu.moger@amd.com> --- arch/x86/kernel/cpu/resctrl/rdtgroup.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 344607853f4c..0d71ed22cfa9 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -685,6 +685,7 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup *rdtgrp, static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of, char *buf, size_t nbytes, loff_t off) { + struct task_struct *task, *thread; struct rdtgroup *rdtgrp; char *pid_str; int ret = 0; @@ -723,7 +724,13 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of, goto exit; } - ret = rdtgroup_move_task(pid, rdtgrp, of); + task = find_task_by_vpid(pid); + thread = task; + do { + ret = rdtgroup_move_task(thread->pid, rdtgrp, of); + if (ret) + goto exit; + } while_each_thread(task, thread); goto next; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically 2023-01-03 22:06 ` [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically Babu Moger @ 2023-01-04 5:55 ` Yu, Fenghua 2023-01-04 17:49 ` Moger, Babu 2023-01-04 16:43 ` Reinette Chatre 1 sibling, 1 reply; 16+ messages in thread From: Yu, Fenghua @ 2023-01-04 5:55 UTC (permalink / raw) To: Babu Moger, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi, Babu, > Some micro benchmarks run multiple threads when started. Monitoring (or > controlling) the benchmark using the task id is bit tricky. Users need to track all > the threads and assign them individually to monitor or control. For example: > $stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32 > -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 > > $pidof stream_lowOverhead > 6793 > > This benchmark actually runs multiple threads underneath on the cores listed > above. It can be seen with the command: > $ps -T -p 6793 > PID SPID TTY TIME CMD > 6793 6793 pts/2 00:00:00 stream_lowOverh > 6793 6802 pts/2 00:01:25 stream_lowOverh > 6793 6803 pts/2 00:01:25 stream_lowOverh > 6793 6804 pts/2 00:01:25 stream_lowOverh > 6793 6805 pts/2 00:01:25 stream_lowOverh > > Users need to assign these threads individually to the resctrl group for > monitoring or controlling. > > $echo 6793 > /sys/fs/restrl/clos1/tasks > $echo 6802 > /sys/fs/restrl/clos1/tasks > $echo 6803 > /sys/fs/restrl/clos1/tasks > $echo 6804 > /sys/fs/restrl/clos1/tasks > $echo 6805 > /sys/fs/restrl/clos1/tasks > > That is not easy when dealing with numerous threads. > > Detect the task's threads automatically and assign them to the resctrl group > when parent task is assigned. For example: But user may choose not to move threads along with the parent. You will need to have an option to opt in. > $echo 6793 > /sys/fs/restrl/clos1/tasks > > All the threads will be assigned to the group automatically. > $cat /sys/fs/restrl/clos1/tasks > 6793 > 6793 > 6802 > 6803 > 6804 > 6805 > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index 344607853f4c..0d71ed22cfa9 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -685,6 +685,7 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup > *rdtgrp, static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of, > char *buf, size_t nbytes, loff_t off) { > + struct task_struct *task, *thread; > struct rdtgroup *rdtgrp; > char *pid_str; > int ret = 0; > @@ -723,7 +724,13 @@ static ssize_t rdtgroup_tasks_write(struct > kernfs_open_file *of, > goto exit; > } > > - ret = rdtgroup_move_task(pid, rdtgrp, of); > + task = find_task_by_vpid(pid); > + thread = task; > + do { > + ret = rdtgroup_move_task(thread->pid, rdtgrp, of); > + if (ret) > + goto exit; If failure happens in the middle of threads, will you reverse the previous moved threads (or even the task) or will you report this failure and move to the next thread? Seems to me you need to either move all threads or no thread moved at all. > + } while_each_thread(task, thread); > > goto next; > > Thanks. -Fenghua ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically 2023-01-04 5:55 ` Yu, Fenghua @ 2023-01-04 17:49 ` Moger, Babu 0 siblings, 0 replies; 16+ messages in thread From: Moger, Babu @ 2023-01-04 17:49 UTC (permalink / raw) To: Yu, Fenghua, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi Fenghua, On 1/3/23 23:55, Yu, Fenghua wrote: > Hi, Babu, > >> Some micro benchmarks run multiple threads when started. Monitoring (or >> controlling) the benchmark using the task id is bit tricky. Users need to track all >> the threads and assign them individually to monitor or control. For example: >> $stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32 >> -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 >> >> $pidof stream_lowOverhead >> 6793 >> >> This benchmark actually runs multiple threads underneath on the cores listed >> above. It can be seen with the command: >> $ps -T -p 6793 >> PID SPID TTY TIME CMD >> 6793 6793 pts/2 00:00:00 stream_lowOverh >> 6793 6802 pts/2 00:01:25 stream_lowOverh >> 6793 6803 pts/2 00:01:25 stream_lowOverh >> 6793 6804 pts/2 00:01:25 stream_lowOverh >> 6793 6805 pts/2 00:01:25 stream_lowOverh >> >> Users need to assign these threads individually to the resctrl group for >> monitoring or controlling. >> >> $echo 6793 > /sys/fs/restrl/clos1/tasks >> $echo 6802 > /sys/fs/restrl/clos1/tasks >> $echo 6803 > /sys/fs/restrl/clos1/tasks >> $echo 6804 > /sys/fs/restrl/clos1/tasks >> $echo 6805 > /sys/fs/restrl/clos1/tasks >> >> That is not easy when dealing with numerous threads. >> >> Detect the task's threads automatically and assign them to the resctrl group >> when parent task is assigned. For example: > But user may choose not to move threads along with the parent. > You will need to have an option to opt in. Yes. I agree. > >> $echo 6793 > /sys/fs/restrl/clos1/tasks >> >> All the threads will be assigned to the group automatically. >> $cat /sys/fs/restrl/clos1/tasks >> 6793 >> 6793 >> 6802 >> 6803 >> 6804 >> 6805 >> >> Signed-off-by: Babu Moger <babu.moger@amd.com> >> --- >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 344607853f4c..0d71ed22cfa9 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -685,6 +685,7 @@ static int rdtgroup_move_task(pid_t pid, struct rdtgroup >> *rdtgrp, static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of, >> char *buf, size_t nbytes, loff_t off) { >> + struct task_struct *task, *thread; >> struct rdtgroup *rdtgrp; >> char *pid_str; >> int ret = 0; >> @@ -723,7 +724,13 @@ static ssize_t rdtgroup_tasks_write(struct >> kernfs_open_file *of, >> goto exit; >> } >> >> - ret = rdtgroup_move_task(pid, rdtgrp, of); >> + task = find_task_by_vpid(pid); >> + thread = task; >> + do { >> + ret = rdtgroup_move_task(thread->pid, rdtgrp, of); >> + if (ret) >> + goto exit; > If failure happens in the middle of threads, will you reverse the previous > moved threads (or even the task) or will you report this failure and move > to the next thread? Seems to me you need to either move all threads or > no thread moved at all. Yes. We should handle the failures properly. Reversing all previous movements requires quite a bit of book keeping. As a work-around Reinette's comment "echo $$ > /sys/fs/resctrl/clos1/tasks" seems to move all the threads. I will have to think about this more closely. Thanks Babu > >> + } while_each_thread(task, thread); >> >> goto next; >> >> > Thanks. > > -Fenghua -- Thanks Babu Moger ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically 2023-01-03 22:06 ` [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically Babu Moger 2023-01-04 5:55 ` Yu, Fenghua @ 2023-01-04 16:43 ` Reinette Chatre 2023-01-04 18:06 ` Moger, Babu 1 sibling, 1 reply; 16+ messages in thread From: Reinette Chatre @ 2023-01-04 16:43 UTC (permalink / raw) To: Babu Moger, fenghua.yu Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman Hi Babu, On 1/3/2023 2:06 PM, Babu Moger wrote: > Some micro benchmarks run multiple threads when started. Monitoring > (or controlling) the benchmark using the task id is bit tricky. Users > need to track all the threads and assign them individually to monitor > or control. For example: > $stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32 > -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 > > $pidof stream_lowOverhead > 6793 > > This benchmark actually runs multiple threads underneath on the cores > listed above. It can be seen with the command: > $ps -T -p 6793 > PID SPID TTY TIME CMD > 6793 6793 pts/2 00:00:00 stream_lowOverh > 6793 6802 pts/2 00:01:25 stream_lowOverh > 6793 6803 pts/2 00:01:25 stream_lowOverh > 6793 6804 pts/2 00:01:25 stream_lowOverh > 6793 6805 pts/2 00:01:25 stream_lowOverh > > Users need to assign these threads individually to the resctrl group for > monitoring or controlling. > > $echo 6793 > /sys/fs/restrl/clos1/tasks > $echo 6802 > /sys/fs/restrl/clos1/tasks > $echo 6803 > /sys/fs/restrl/clos1/tasks > $echo 6804 > /sys/fs/restrl/clos1/tasks > $echo 6805 > /sys/fs/restrl/clos1/tasks > > That is not easy when dealing with numerous threads. How about: # echo $$ > /sys/fs/resctrl/clos1/tasks # stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32\ -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 Reinette ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically 2023-01-04 16:43 ` Reinette Chatre @ 2023-01-04 18:06 ` Moger, Babu 0 siblings, 0 replies; 16+ messages in thread From: Moger, Babu @ 2023-01-04 18:06 UTC (permalink / raw) To: Reinette Chatre, fenghua.yu Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman Hi Reinette, On 1/4/23 10:43, Reinette Chatre wrote: > Hi Babu, > > On 1/3/2023 2:06 PM, Babu Moger wrote: >> Some micro benchmarks run multiple threads when started. Monitoring >> (or controlling) the benchmark using the task id is bit tricky. Users >> need to track all the threads and assign them individually to monitor >> or control. For example: >> $stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32 >> -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 >> >> $pidof stream_lowOverhead >> 6793 >> >> This benchmark actually runs multiple threads underneath on the cores >> listed above. It can be seen with the command: >> $ps -T -p 6793 >> PID SPID TTY TIME CMD >> 6793 6793 pts/2 00:00:00 stream_lowOverh >> 6793 6802 pts/2 00:01:25 stream_lowOverh >> 6793 6803 pts/2 00:01:25 stream_lowOverh >> 6793 6804 pts/2 00:01:25 stream_lowOverh >> 6793 6805 pts/2 00:01:25 stream_lowOverh >> >> Users need to assign these threads individually to the resctrl group for >> monitoring or controlling. >> >> $echo 6793 > /sys/fs/restrl/clos1/tasks >> $echo 6802 > /sys/fs/restrl/clos1/tasks >> $echo 6803 > /sys/fs/restrl/clos1/tasks >> $echo 6804 > /sys/fs/restrl/clos1/tasks >> $echo 6805 > /sys/fs/restrl/clos1/tasks >> >> That is not easy when dealing with numerous threads. > How about: > > # echo $$ > /sys/fs/resctrl/clos1/tasks > # stream_lowOverhead -codeAlg 13 -nRep 100000 -cores 0 1 2 3 -memMB 32\ > -alignKB 8192 -aPadB 0 -bPadB 0 -cPadB 0 -testMask 1 Yes, That works. We can use that as a work-around for now. I will drop this patch for now. I will have to figure out how to handle failures in the middle of task/thread movement. Will have to revisit this in the future. Will re-submit patch 1 and 3. Thanks Babu > > Reinette -- Thanks Babu Moger ^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-03 22:06 [RFC PATCH 0/3] x86/resctrl: Miscellaneous resctrl features Babu Moger 2023-01-03 22:06 ` [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger 2023-01-03 22:06 ` [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically Babu Moger @ 2023-01-03 22:06 ` Babu Moger 2023-01-04 6:06 ` Yu, Fenghua 2 siblings, 1 reply; 16+ messages in thread From: Babu Moger @ 2023-01-03 22:06 UTC (permalink / raw) To: fenghua.yu, reinette.chatre Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, eranian, peternewman When a user creates a control or monitor group, the CLOSID or RMID are not visible to the user. These are architecturally defined entities. There is no harm in displaying these in resctrl groups. Sometimes it can help to debug the issues. Add CLOSID and RMID to the control/monitor groups display in resctrl interface. $cat /sys/fs/resctrl/clos1/closid 1 $cat /sys/fs/resctrl/mon_groups/mon1/rmid 3 Signed-off-by: Babu Moger <babu.moger@amd.com> --- Documentation/x86/resctrl.rst | 15 ++++++++++ arch/x86/kernel/cpu/resctrl/rdtgroup.c | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst index f26e16412bcb..8520514bc8b5 100644 --- a/Documentation/x86/resctrl.rst +++ b/Documentation/x86/resctrl.rst @@ -231,6 +231,14 @@ All groups contain the following files: Just like "cpus", only using ranges of CPUs instead of bitmasks. +"rmid": + Reading this file shows the resource monitoring id (RMID) for + monitoring the resource utilization. Monitoring is performed by + tagging each core(or thread) or process via a Resource Monitoring + ID (RMID). Kernel assigns a new RMID when a group is created + depending on the available RMIDs. Multiple cores(or threads) or + processes can share a same RMID in a resctrl domain. + When control is enabled all CTRL_MON groups will also contain: "schemata": @@ -252,6 +260,13 @@ When control is enabled all CTRL_MON groups will also contain: file. On successful pseudo-locked region creation the mode will automatically change to "pseudo-locked". +"closid": + Reading this file shows the Class of Service (CLOS) id which acts + as a resource control tag on which the resources can be throttled. + Kernel assigns a new CLOSID a control group is created depending + on the available CLOSIDs. Multiple cores(or threads) or processes + can share a same CLOSID in a resctrl domain. + When monitoring is enabled all MON groups will also contain: "mon_data": diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c index 0d71ed22cfa9..98b4798e5cae 100644 --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c @@ -769,6 +769,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file *of, return ret; } +static int rdtgroup_closid_show(struct kernfs_open_file *of, + struct seq_file *s, void *v) +{ + struct rdtgroup *rdtgrp; + int ret = 0; + + rdtgrp = rdtgroup_kn_lock_live(of->kn); + if (rdtgrp) + seq_printf(s, "%u\n", rdtgrp->closid); + else + ret = -ENOENT; + rdtgroup_kn_unlock(of->kn); + + return ret; +} + +static int rdtgroup_rmid_show(struct kernfs_open_file *of, + struct seq_file *s, void *v) +{ + struct rdtgroup *rdtgrp; + int ret = 0; + + rdtgrp = rdtgroup_kn_lock_live(of->kn); + if (rdtgrp) + seq_printf(s, "%u\n", rdtgrp->mon.rmid); + else + ret = -ENOENT; + rdtgroup_kn_unlock(of->kn); + + return ret; +} + #ifdef CONFIG_PROC_CPU_RESCTRL /* @@ -1593,6 +1625,20 @@ static struct rftype res_common_files[] = { .seq_show = rdtgroup_size_show, .fflags = RF_CTRL_BASE, }, + { + .name = "closid", + .mode = 0444, + .kf_ops = &rdtgroup_kf_single_ops, + .seq_show = rdtgroup_closid_show, + .fflags = RF_CTRL_BASE, + }, + { + .name = "rmid", + .mode = 0444, + .kf_ops = &rdtgroup_kf_single_ops, + .seq_show = rdtgroup_rmid_show, + .fflags = RFTYPE_BASE, + }, }; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-03 22:06 ` [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger @ 2023-01-04 6:06 ` Yu, Fenghua 2023-01-04 6:45 ` Stephane Eranian 2023-01-04 17:58 ` Moger, Babu 0 siblings, 2 replies; 16+ messages in thread From: Yu, Fenghua @ 2023-01-04 6:06 UTC (permalink / raw) To: Babu Moger, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi, Babu, > When a user creates a control or monitor group, the CLOSID or RMID are not > visible to the user. These are architecturally defined entities. > There is no harm in displaying these in resctrl groups. Sometimes it can help to > debug the issues. Although "no harm" to show them, it's not useful for generic user either and may cause confusion sometimes. CLOSID and RMID are supposed to be invisible to generic users. Maybe introduce a new resctrl mount option called "debug" and show the files and maybe other future debug info only in debug mode? > > Add CLOSID and RMID to the control/monitor groups display in resctrl interface. > > $cat /sys/fs/resctrl/clos1/closid > 1 > $cat /sys/fs/resctrl/mon_groups/mon1/rmid > 3 > > Signed-off-by: Babu Moger <babu.moger@amd.com> > --- > Documentation/x86/resctrl.rst | 15 ++++++++++ > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 46 > ++++++++++++++++++++++++++++++++ > 2 files changed, 61 insertions(+) > > diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst > index f26e16412bcb..8520514bc8b5 100644 > --- a/Documentation/x86/resctrl.rst > +++ b/Documentation/x86/resctrl.rst > @@ -231,6 +231,14 @@ All groups contain the following files: > Just like "cpus", only using ranges of CPUs instead of bitmasks. > > > +"rmid": > + Reading this file shows the resource monitoring id (RMID) for > + monitoring the resource utilization. Monitoring is performed by > + tagging each core(or thread) or process via a Resource Monitoring > + ID (RMID). Kernel assigns a new RMID when a group is created > + depending on the available RMIDs. Multiple cores(or threads) or > + processes can share a same RMID in a resctrl domain. > + > When control is enabled all CTRL_MON groups will also contain: > > "schemata": > @@ -252,6 +260,13 @@ When control is enabled all CTRL_MON groups will > also contain: > file. On successful pseudo-locked region creation the mode will > automatically change to "pseudo-locked". > > +"closid": > + Reading this file shows the Class of Service (CLOS) id which acts > + as a resource control tag on which the resources can be throttled. > + Kernel assigns a new CLOSID a control group is created depending > + on the available CLOSIDs. Multiple cores(or threads) or processes > + can share a same CLOSID in a resctrl domain. > + > When monitoring is enabled all MON groups will also contain: > > "mon_data": > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index 0d71ed22cfa9..98b4798e5cae 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -769,6 +769,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file > *of, > return ret; > } > > +static int rdtgroup_closid_show(struct kernfs_open_file *of, > + struct seq_file *s, void *v) > +{ > + struct rdtgroup *rdtgrp; > + int ret = 0; > + > + rdtgrp = rdtgroup_kn_lock_live(of->kn); > + if (rdtgrp) > + seq_printf(s, "%u\n", rdtgrp->closid); > + else > + ret = -ENOENT; > + rdtgroup_kn_unlock(of->kn); > + > + return ret; > +} > + > +static int rdtgroup_rmid_show(struct kernfs_open_file *of, > + struct seq_file *s, void *v) > +{ > + struct rdtgroup *rdtgrp; > + int ret = 0; > + > + rdtgrp = rdtgroup_kn_lock_live(of->kn); > + if (rdtgrp) > + seq_printf(s, "%u\n", rdtgrp->mon.rmid); > + else > + ret = -ENOENT; > + rdtgroup_kn_unlock(of->kn); > + > + return ret; > +} > + > #ifdef CONFIG_PROC_CPU_RESCTRL > > /* > @@ -1593,6 +1625,20 @@ static struct rftype res_common_files[] = { > .seq_show = rdtgroup_size_show, > .fflags = RF_CTRL_BASE, > }, > + { > + .name = "closid", > + .mode = 0444, > + .kf_ops = &rdtgroup_kf_single_ops, > + .seq_show = rdtgroup_closid_show, > + .fflags = RF_CTRL_BASE, > + }, > + { > + .name = "rmid", > + .mode = 0444, > + .kf_ops = &rdtgroup_kf_single_ops, > + .seq_show = rdtgroup_rmid_show, > + .fflags = RFTYPE_BASE, > + }, > > }; > > Thanks. -Fenghua ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-04 6:06 ` Yu, Fenghua @ 2023-01-04 6:45 ` Stephane Eranian 2023-01-04 18:01 ` Moger, Babu 2023-01-04 17:58 ` Moger, Babu 1 sibling, 1 reply; 16+ messages in thread From: Stephane Eranian @ 2023-01-04 6:45 UTC (permalink / raw) To: Yu, Fenghua Cc: Babu Moger, Chatre, Reinette, tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, peternewman, Shankar, Ravi V On Tue, Jan 3, 2023 at 10:06 PM Yu, Fenghua <fenghua.yu@intel.com> wrote: > > Hi, Babu, > > > When a user creates a control or monitor group, the CLOSID or RMID are not > > visible to the user. These are architecturally defined entities. > > There is no harm in displaying these in resctrl groups. Sometimes it can help to > > debug the issues. > Although "no harm" to show them, it's not useful for generic user either and may > cause confusion sometimes. CLOSID and RMID are supposed to be invisible to > generic users. > > Maybe introduce a new resctrl mount option called "debug" and show the files > and maybe other future debug info only in debug mode? > On other non-x86 architectures, these have no meaning or no direct mapping. Take ARM MPAM, it is called PARTID and it does not map to either RMID or CLOSID, it is combined. Why would you call this closid/rmid at the user level? You could instead use a more generic name such as mon_hw_id, ctrl_hw_id. And on ARM they would be the same. Just my suggestion. > > > > > Add CLOSID and RMID to the control/monitor groups display in resctrl interface. > > > > $cat /sys/fs/resctrl/clos1/closid > > 1 > > $cat /sys/fs/resctrl/mon_groups/mon1/rmid > > 3 > > > > Signed-off-by: Babu Moger <babu.moger@amd.com> > > --- > > Documentation/x86/resctrl.rst | 15 ++++++++++ > > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 46 > > ++++++++++++++++++++++++++++++++ > > 2 files changed, 61 insertions(+) > > > > diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst > > index f26e16412bcb..8520514bc8b5 100644 > > --- a/Documentation/x86/resctrl.rst > > +++ b/Documentation/x86/resctrl.rst > > @@ -231,6 +231,14 @@ All groups contain the following files: > > Just like "cpus", only using ranges of CPUs instead of bitmasks. > > > > > > +"rmid": > > + Reading this file shows the resource monitoring id (RMID) for > > + monitoring the resource utilization. Monitoring is performed by > > + tagging each core(or thread) or process via a Resource Monitoring > > + ID (RMID). Kernel assigns a new RMID when a group is created > > + depending on the available RMIDs. Multiple cores(or threads) or > > + processes can share a same RMID in a resctrl domain. > > + > > When control is enabled all CTRL_MON groups will also contain: > > > > "schemata": > > @@ -252,6 +260,13 @@ When control is enabled all CTRL_MON groups will > > also contain: > > file. On successful pseudo-locked region creation the mode will > > automatically change to "pseudo-locked". > > > > +"closid": > > + Reading this file shows the Class of Service (CLOS) id which acts > > + as a resource control tag on which the resources can be throttled. > > + Kernel assigns a new CLOSID a control group is created depending > > + on the available CLOSIDs. Multiple cores(or threads) or processes > > + can share a same CLOSID in a resctrl domain. > > + > > When monitoring is enabled all MON groups will also contain: > > > > "mon_data": > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > index 0d71ed22cfa9..98b4798e5cae 100644 > > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > > @@ -769,6 +769,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file > > *of, > > return ret; > > } > > > > +static int rdtgroup_closid_show(struct kernfs_open_file *of, > > + struct seq_file *s, void *v) > > +{ > > + struct rdtgroup *rdtgrp; > > + int ret = 0; > > + > > + rdtgrp = rdtgroup_kn_lock_live(of->kn); > > + if (rdtgrp) > > + seq_printf(s, "%u\n", rdtgrp->closid); > > + else > > + ret = -ENOENT; > > + rdtgroup_kn_unlock(of->kn); > > + > > + return ret; > > +} > > + > > +static int rdtgroup_rmid_show(struct kernfs_open_file *of, > > + struct seq_file *s, void *v) > > +{ > > + struct rdtgroup *rdtgrp; > > + int ret = 0; > > + > > + rdtgrp = rdtgroup_kn_lock_live(of->kn); > > + if (rdtgrp) > > + seq_printf(s, "%u\n", rdtgrp->mon.rmid); > > + else > > + ret = -ENOENT; > > + rdtgroup_kn_unlock(of->kn); > > + > > + return ret; > > +} > > + > > #ifdef CONFIG_PROC_CPU_RESCTRL > > > > /* > > @@ -1593,6 +1625,20 @@ static struct rftype res_common_files[] = { > > .seq_show = rdtgroup_size_show, > > .fflags = RF_CTRL_BASE, > > }, > > + { > > + .name = "closid", > > + .mode = 0444, > > + .kf_ops = &rdtgroup_kf_single_ops, > > + .seq_show = rdtgroup_closid_show, > > + .fflags = RF_CTRL_BASE, > > + }, > > + { > > + .name = "rmid", > > + .mode = 0444, > > + .kf_ops = &rdtgroup_kf_single_ops, > > + .seq_show = rdtgroup_rmid_show, > > + .fflags = RFTYPE_BASE, > > + }, > > > > }; > > > > > Thanks. > > -Fenghua ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-04 6:45 ` Stephane Eranian @ 2023-01-04 18:01 ` Moger, Babu 0 siblings, 0 replies; 16+ messages in thread From: Moger, Babu @ 2023-01-04 18:01 UTC (permalink / raw) To: Stephane Eranian, Yu, Fenghua Cc: Chatre, Reinette, tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, peternewman, Shankar, Ravi V Hi Stephane, On 1/4/23 00:45, Stephane Eranian wrote: > On Tue, Jan 3, 2023 at 10:06 PM Yu, Fenghua <fenghua.yu@intel.com> wrote: >> Hi, Babu, >> >>> When a user creates a control or monitor group, the CLOSID or RMID are not >>> visible to the user. These are architecturally defined entities. >>> There is no harm in displaying these in resctrl groups. Sometimes it can help to >>> debug the issues. >> Although "no harm" to show them, it's not useful for generic user either and may >> cause confusion sometimes. CLOSID and RMID are supposed to be invisible to >> generic users. >> >> Maybe introduce a new resctrl mount option called "debug" and show the files >> and maybe other future debug info only in debug mode? >> > On other non-x86 architectures, these have no meaning or no direct mapping. > Take ARM MPAM, it is called PARTID and it does not map to either RMID > or CLOSID, it is combined. > Why would you call this closid/rmid at the user level? > You could instead use a more generic name such as mon_hw_id, > ctrl_hw_id. And on ARM they would be the same. > Just my suggestion. Sure. We can change the names to mon_hw_id and ctrl_hw_id. Thanks Babu > > >>> Add CLOSID and RMID to the control/monitor groups display in resctrl interface. >>> >>> $cat /sys/fs/resctrl/clos1/closid >>> 1 >>> $cat /sys/fs/resctrl/mon_groups/mon1/rmid >>> 3 >>> >>> Signed-off-by: Babu Moger <babu.moger@amd.com> >>> --- >>> Documentation/x86/resctrl.rst | 15 ++++++++++ >>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 46 >>> ++++++++++++++++++++++++++++++++ >>> 2 files changed, 61 insertions(+) >>> >>> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst >>> index f26e16412bcb..8520514bc8b5 100644 >>> --- a/Documentation/x86/resctrl.rst >>> +++ b/Documentation/x86/resctrl.rst >>> @@ -231,6 +231,14 @@ All groups contain the following files: >>> Just like "cpus", only using ranges of CPUs instead of bitmasks. >>> >>> >>> +"rmid": >>> + Reading this file shows the resource monitoring id (RMID) for >>> + monitoring the resource utilization. Monitoring is performed by >>> + tagging each core(or thread) or process via a Resource Monitoring >>> + ID (RMID). Kernel assigns a new RMID when a group is created >>> + depending on the available RMIDs. Multiple cores(or threads) or >>> + processes can share a same RMID in a resctrl domain. >>> + >>> When control is enabled all CTRL_MON groups will also contain: >>> >>> "schemata": >>> @@ -252,6 +260,13 @@ When control is enabled all CTRL_MON groups will >>> also contain: >>> file. On successful pseudo-locked region creation the mode will >>> automatically change to "pseudo-locked". >>> >>> +"closid": >>> + Reading this file shows the Class of Service (CLOS) id which acts >>> + as a resource control tag on which the resources can be throttled. >>> + Kernel assigns a new CLOSID a control group is created depending >>> + on the available CLOSIDs. Multiple cores(or threads) or processes >>> + can share a same CLOSID in a resctrl domain. >>> + >>> When monitoring is enabled all MON groups will also contain: >>> >>> "mon_data": >>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>> index 0d71ed22cfa9..98b4798e5cae 100644 >>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >>> @@ -769,6 +769,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file >>> *of, >>> return ret; >>> } >>> >>> +static int rdtgroup_closid_show(struct kernfs_open_file *of, >>> + struct seq_file *s, void *v) >>> +{ >>> + struct rdtgroup *rdtgrp; >>> + int ret = 0; >>> + >>> + rdtgrp = rdtgroup_kn_lock_live(of->kn); >>> + if (rdtgrp) >>> + seq_printf(s, "%u\n", rdtgrp->closid); >>> + else >>> + ret = -ENOENT; >>> + rdtgroup_kn_unlock(of->kn); >>> + >>> + return ret; >>> +} >>> + >>> +static int rdtgroup_rmid_show(struct kernfs_open_file *of, >>> + struct seq_file *s, void *v) >>> +{ >>> + struct rdtgroup *rdtgrp; >>> + int ret = 0; >>> + >>> + rdtgrp = rdtgroup_kn_lock_live(of->kn); >>> + if (rdtgrp) >>> + seq_printf(s, "%u\n", rdtgrp->mon.rmid); >>> + else >>> + ret = -ENOENT; >>> + rdtgroup_kn_unlock(of->kn); >>> + >>> + return ret; >>> +} >>> + >>> #ifdef CONFIG_PROC_CPU_RESCTRL >>> >>> /* >>> @@ -1593,6 +1625,20 @@ static struct rftype res_common_files[] = { >>> .seq_show = rdtgroup_size_show, >>> .fflags = RF_CTRL_BASE, >>> }, >>> + { >>> + .name = "closid", >>> + .mode = 0444, >>> + .kf_ops = &rdtgroup_kf_single_ops, >>> + .seq_show = rdtgroup_closid_show, >>> + .fflags = RF_CTRL_BASE, >>> + }, >>> + { >>> + .name = "rmid", >>> + .mode = 0444, >>> + .kf_ops = &rdtgroup_kf_single_ops, >>> + .seq_show = rdtgroup_rmid_show, >>> + .fflags = RFTYPE_BASE, >>> + }, >>> >>> }; >>> >>> >> Thanks. >> >> -Fenghua -- Thanks Babu Moger ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-04 6:06 ` Yu, Fenghua 2023-01-04 6:45 ` Stephane Eranian @ 2023-01-04 17:58 ` Moger, Babu 2023-01-04 23:54 ` Yu, Fenghua 1 sibling, 1 reply; 16+ messages in thread From: Moger, Babu @ 2023-01-04 17:58 UTC (permalink / raw) To: Yu, Fenghua, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi Fenghua, On 1/4/23 00:06, Yu, Fenghua wrote: > Hi, Babu, > >> When a user creates a control or monitor group, the CLOSID or RMID are not >> visible to the user. These are architecturally defined entities. >> There is no harm in displaying these in resctrl groups. Sometimes it can help to >> debug the issues. > Although "no harm" to show them, it's not useful for generic user either and may > cause confusion sometimes. CLOSID and RMID are supposed to be invisible to > generic users. > > Maybe introduce a new resctrl mount option called "debug" and show the files > and maybe other future debug info only in debug mode? Actually, test team feels very strongly about this. Whenever there is some issue, first question is what is the rmid or closid are you running on? We normally don't have an answer for that. In my opinion, adding debug mode just for these two fields seems way overkill. Thanks Babu > >> Add CLOSID and RMID to the control/monitor groups display in resctrl interface. >> >> $cat /sys/fs/resctrl/clos1/closid >> 1 >> $cat /sys/fs/resctrl/mon_groups/mon1/rmid >> 3 >> >> Signed-off-by: Babu Moger <babu.moger@amd.com> >> --- >> Documentation/x86/resctrl.rst | 15 ++++++++++ >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 46 >> ++++++++++++++++++++++++++++++++ >> 2 files changed, 61 insertions(+) >> >> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst >> index f26e16412bcb..8520514bc8b5 100644 >> --- a/Documentation/x86/resctrl.rst >> +++ b/Documentation/x86/resctrl.rst >> @@ -231,6 +231,14 @@ All groups contain the following files: >> Just like "cpus", only using ranges of CPUs instead of bitmasks. >> >> >> +"rmid": >> + Reading this file shows the resource monitoring id (RMID) for >> + monitoring the resource utilization. Monitoring is performed by >> + tagging each core(or thread) or process via a Resource Monitoring >> + ID (RMID). Kernel assigns a new RMID when a group is created >> + depending on the available RMIDs. Multiple cores(or threads) or >> + processes can share a same RMID in a resctrl domain. >> + >> When control is enabled all CTRL_MON groups will also contain: >> >> "schemata": >> @@ -252,6 +260,13 @@ When control is enabled all CTRL_MON groups will >> also contain: >> file. On successful pseudo-locked region creation the mode will >> automatically change to "pseudo-locked". >> >> +"closid": >> + Reading this file shows the Class of Service (CLOS) id which acts >> + as a resource control tag on which the resources can be throttled. >> + Kernel assigns a new CLOSID a control group is created depending >> + on the available CLOSIDs. Multiple cores(or threads) or processes >> + can share a same CLOSID in a resctrl domain. >> + >> When monitoring is enabled all MON groups will also contain: >> >> "mon_data": >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 0d71ed22cfa9..98b4798e5cae 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -769,6 +769,38 @@ static int rdtgroup_tasks_show(struct kernfs_open_file >> *of, >> return ret; >> } >> >> +static int rdtgroup_closid_show(struct kernfs_open_file *of, >> + struct seq_file *s, void *v) >> +{ >> + struct rdtgroup *rdtgrp; >> + int ret = 0; >> + >> + rdtgrp = rdtgroup_kn_lock_live(of->kn); >> + if (rdtgrp) >> + seq_printf(s, "%u\n", rdtgrp->closid); >> + else >> + ret = -ENOENT; >> + rdtgroup_kn_unlock(of->kn); >> + >> + return ret; >> +} >> + >> +static int rdtgroup_rmid_show(struct kernfs_open_file *of, >> + struct seq_file *s, void *v) >> +{ >> + struct rdtgroup *rdtgrp; >> + int ret = 0; >> + >> + rdtgrp = rdtgroup_kn_lock_live(of->kn); >> + if (rdtgrp) >> + seq_printf(s, "%u\n", rdtgrp->mon.rmid); >> + else >> + ret = -ENOENT; >> + rdtgroup_kn_unlock(of->kn); >> + >> + return ret; >> +} >> + >> #ifdef CONFIG_PROC_CPU_RESCTRL >> >> /* >> @@ -1593,6 +1625,20 @@ static struct rftype res_common_files[] = { >> .seq_show = rdtgroup_size_show, >> .fflags = RF_CTRL_BASE, >> }, >> + { >> + .name = "closid", >> + .mode = 0444, >> + .kf_ops = &rdtgroup_kf_single_ops, >> + .seq_show = rdtgroup_closid_show, >> + .fflags = RF_CTRL_BASE, >> + }, >> + { >> + .name = "rmid", >> + .mode = 0444, >> + .kf_ops = &rdtgroup_kf_single_ops, >> + .seq_show = rdtgroup_rmid_show, >> + .fflags = RFTYPE_BASE, >> + }, >> >> }; >> >> > Thanks. > > -Fenghua -- Thanks Babu Moger ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-04 17:58 ` Moger, Babu @ 2023-01-04 23:54 ` Yu, Fenghua 2023-01-05 15:48 ` Moger, Babu 0 siblings, 1 reply; 16+ messages in thread From: Yu, Fenghua @ 2023-01-04 23:54 UTC (permalink / raw) To: babu.moger, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi, Babu, > >> When a user creates a control or monitor group, the CLOSID or RMID > >> are not visible to the user. These are architecturally defined entities. > >> There is no harm in displaying these in resctrl groups. Sometimes it > >> can help to debug the issues. > > Although "no harm" to show them, it's not useful for generic user > > either and may cause confusion sometimes. CLOSID and RMID are supposed > > to be invisible to generic users. > > > > Maybe introduce a new resctrl mount option called "debug" and show the > > files and maybe other future debug info only in debug mode? > > Actually, test team feels very strongly about this. Whenever there is some issue, > first question is what is the rmid or closid are you running on? We normally don't > have an answer for that. > > In my opinion, adding debug mode just for these two fields seems way overkill. Yes, they are useful for "test team" (quoted from your statement) and developers. Not for end users. A debug mode is useful not just for these two files. I'm working on another resctrl project where much more complex hardware info needs to be dumped for debug purpose only. It's obvious not to show it in generic use. It's more obvious to just show the info file in debug mode in my case. I think these CLOSID and RMID files and future debug files belong to a new debug mode. It would be better to introduce the debug mode now rather than later so that it can be extended easily in the future. Maybe we can enable debug mode in a separate debug mode patch: 1. Add RFTYPE_DEBUG as a new file type. Files with this flag are for debug purpose and only be visible in\ debug mode. 2. Add RFTYPE_INVISIBLE as a new file type. Files with this flag will be invisible/not be added in resctrl fs. 3. Add mount parameter "debug" so that ctx->debug=true if mount -o debug is given. 4. If ctx->debug is true, in rdt_enable_ctx(), go through RFTYPE_DEBUG files in res_common_files[] and mark fflags with RFTYPE_INVISIBLE. 5. In rdtgroup_add_file(), if (rft->fflags & RFTYPE_INVISIBLE) return. So the debug files will be visible only in debug mode. With the debug mode patch in place, it's simple to extend to any debug files: In your case, update this patch by just adding RFTYPE_DEBUG in fflags. Then the debug mode will work for this patch automatically. In my case or any future debug files, we just simply add RFTYPE_DEBUG in fflags and the debug mode will work automatically. Does it make sense? Thanks. -Fenghua ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups 2023-01-04 23:54 ` Yu, Fenghua @ 2023-01-05 15:48 ` Moger, Babu 0 siblings, 0 replies; 16+ messages in thread From: Moger, Babu @ 2023-01-05 15:48 UTC (permalink / raw) To: Yu, Fenghua, Chatre, Reinette Cc: tglx, mingo, bp, dave.hansen, x86, hpa, corbet, linux-kernel, linux-doc, Eranian, Stephane, peternewman, Shankar, Ravi V Hi Fenghua, On 1/4/23 17:54, Yu, Fenghua wrote: > Hi, Babu, > >>>> When a user creates a control or monitor group, the CLOSID or RMID >>>> are not visible to the user. These are architecturally defined entities. >>>> There is no harm in displaying these in resctrl groups. Sometimes it >>>> can help to debug the issues. >>> Although "no harm" to show them, it's not useful for generic user >>> either and may cause confusion sometimes. CLOSID and RMID are supposed >>> to be invisible to generic users. >>> >>> Maybe introduce a new resctrl mount option called "debug" and show the >>> files and maybe other future debug info only in debug mode? >> Actually, test team feels very strongly about this. Whenever there is some issue, >> first question is what is the rmid or closid are you running on? We normally don't >> have an answer for that. >> >> In my opinion, adding debug mode just for these two fields seems way overkill. > Yes, they are useful for "test team" (quoted from your statement) and developers. > Not for end users. > > A debug mode is useful not just for these two files. I'm working on another resctrl > project where much more complex hardware info needs to be dumped for debug purpose > only. It's obvious not to show it in generic use. It's more obvious to just show the info file in > debug mode in my case. > > I think these CLOSID and RMID files and future debug files belong to a new debug mode. > It would be better to introduce the debug mode now rather than later so that it can be extended > easily in the future. > > Maybe we can enable debug mode in a separate debug mode patch: > 1. Add RFTYPE_DEBUG as a new file type. Files with this flag are for debug purpose and only be visible in\ > debug mode. > 2. Add RFTYPE_INVISIBLE as a new file type. Files with this flag will be invisible/not be added in resctrl fs. > 3. Add mount parameter "debug" so that ctx->debug=true if mount -o debug is given. > 4. If ctx->debug is true, in rdt_enable_ctx(), go through RFTYPE_DEBUG files in res_common_files[] and mark > fflags with RFTYPE_INVISIBLE. > 5. In rdtgroup_add_file(), if (rft->fflags & RFTYPE_INVISIBLE) return. So the debug files will be visible only > in debug mode. > > With the debug mode patch in place, it's simple to extend to any debug files: > In your case, update this patch by just adding RFTYPE_DEBUG in fflags. Then the debug mode will work > for this patch automatically. > In my case or any future debug files, we just simply add RFTYPE_DEBUG in fflags and the debug mode will > work automatically. > > Does it make sense? Yes. Sure. The debug mode needs to be resctrl mount option. I will take a look at this further to see what can be done. Thanks Babu ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-01-05 15:48 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-03 22:06 [RFC PATCH 0/3] x86/resctrl: Miscellaneous resctrl features Babu Moger 2023-01-03 22:06 ` [RFC PATCH 1/3] x86/resctrl: Add multiple tasks to the resctrl group at once Babu Moger 2023-01-04 5:46 ` Yu, Fenghua 2023-01-04 17:20 ` Moger, Babu 2023-01-03 22:06 ` [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically Babu Moger 2023-01-04 5:55 ` Yu, Fenghua 2023-01-04 17:49 ` Moger, Babu 2023-01-04 16:43 ` Reinette Chatre 2023-01-04 18:06 ` Moger, Babu 2023-01-03 22:06 ` [RFC PATCH 3/3] x86/resctrl: Display the RMID and COSID for resctrl groups Babu Moger 2023-01-04 6:06 ` Yu, Fenghua 2023-01-04 6:45 ` Stephane Eranian 2023-01-04 18:01 ` Moger, Babu 2023-01-04 17:58 ` Moger, Babu 2023-01-04 23:54 ` Yu, Fenghua 2023-01-05 15:48 ` Moger, Babu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.