Hi Fenghua,
On 2/16/2023 4:27 PM, Fenghua Yu wrote:
Hi, Babu,
On 2/2/23 13:46, Babu Moger wrote:
The resctrl task assignment for MONITOR or CONTROL group needs to be
done 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.
Maybe add something like "poor performance due to syscall overhead...".
Ok. Sure
Improve the user experience by supporting the multiple task assignment
in one command with the tasks separated by commas. For example:
$echo 123,456,789 > /sys/fs/resctrl/clos1/tasks
Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
---
Documentation/x86/resctrl.rst | 9 +++++++--
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 24 +++++++++++++++++++++++-
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/Documentation/x86/resctrl.rst
b/Documentation/x86/resctrl.rst
index 058257dc56c8..58b76fc75cb7 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -292,13 +292,18 @@ 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
+ group. Multiple tasks can be assigned together in one command by
+ inputting the tasks separated by commas. Tasks will be assigned
+ sequentially in the order it is provided. Failure while assigning
+ the tasks will be aborted immediately and tasks next in the
+ sequence will not be assigned. Users may need to retry them again.
May need to add "tasks before the failure are assigned...".
Sure.
To retry movement, user needs to know which pid fails. So it's better
to add "last_command_status shows the failure pid and user can parse
it to retry assignment starting from the failure pid".
Sure.
+
+ 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
this group. Writing a mask to this file will add and remove
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index e2c1599d1b37..13b7c5f3a27c 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -683,16 +683,34 @@ 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';
+
rdtgrp = rdtgroup_kn_lock_live(of->kn);
if (!rdtgrp) {
rdtgroup_kn_unlock(of->kn);
return -ENOENT;
}
+
+next:
+ if (!buf || buf[0] == '\0')
+ goto unlock;
+
+ pid_str = strim(strsep(&buf, ","));
+
+ if (kstrtoint(pid_str, 0, &pid) || pid < 0) {
+ rdt_last_cmd_puts("Invalid pid value\n");
Better to add pid_str in failure info. Then user knows where the
failure pid happens and can re-do the movement starting from the
failed pid.
ok.
+ ret = -EINVAL;
+ goto unlock;
+ }
+
rdt_last_cmd_clear();
if (rdtgrp->mode == RDT_MODE_PSEUDO_LOCKED ||
@@ -703,6 +721,10 @@ static ssize_t rdtgroup_tasks_write(struct
kernfs_open_file *of,
}
ret = rdtgroup_move_task(pid, rdtgrp, of);
+ if (ret)
May need to report "Failed at %d\n", pid;
Ok. Dont want to overwrite the last command status. I may need to update
it with pid. Will check on this.
thanks
Babu
+ goto unlock;
+ else
+ goto next;
unlock:
rdtgroup_kn_unlock(of->kn);
Thanks.
-Fenghua
