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/ctrl_grp1
$echo 123 > /sys/fs/resctrl/ctrl_grp1/tasks
$echo 456 > /sys/fs/resctrl/ctrl_grp1/tasks
$echo 789 > /sys/fs/resctrl/ctrl_grp1/tasks
This is not user-friendly when dealing with hundreds of tasks.
Support multiple task assignment in one command with tasks ids separated
by commas. For example:
$echo 123,456,789 > /sys/fs/resctrl/ctrl_grp1/tasks
Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
---
Documentation/arch/x86/resctrl.rst | 8 +++++++-
arch/x86/kernel/cpu/resctrl/rdtgroup.c | 25 ++++++++++++++++++++++---
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index cb05d90111b4..af234681756e 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -299,7 +299,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
+ group. Multiple tasks can be added by separating the task ids
+ with commas. Tasks will be assigned sequentially. Multiple
+ failures are not supported. A single failure encountered while
+ attempting to assign a task will cause the operation to abort.
+ Failures will be logged to /sys/fs/resctrl/info/last_cmd_status.
+
+ 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
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 725344048f85..8c91c333f9b3 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -696,11 +696,10 @@ 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)
- return -EINVAL;
rdtgrp = rdtgroup_kn_lock_live(of->kn);
if (!rdtgrp) {
rdtgroup_kn_unlock(of->kn);
@@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
goto unlock;
}
- ret = rdtgroup_move_task(pid, rdtgrp, of);
+ while (buf && buf[0] != '\0' && buf[0] != '\n') {
+ pid_str = strim(strsep(&buf, ","));
+
+ if (kstrtoint(pid_str, 0, &pid)) {
+ rdt_last_cmd_puts("Task list parsing error\n");
+ ret = -EINVAL;
+ break;
+ }
+
+ if (pid < 0) {
+ rdt_last_cmd_printf("Invalid pid %d\n", pid);
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = rdtgroup_move_task(pid, rdtgrp, of);
+ if (ret) {
+ rdt_last_cmd_printf("Error while processing task %d\n", pid);
+ break;
+ }
+ }
unlock:
rdtgroup_kn_unlock(of->kn);
--
2.34.1Hi, Babu,
On 8/21/23 16:30, 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/ctrl_grp1
> $echo 123 > /sys/fs/resctrl/ctrl_grp1/tasks
> $echo 456 > /sys/fs/resctrl/ctrl_grp1/tasks
> $echo 789 > /sys/fs/resctrl/ctrl_grp1/tasks
>
> This is not user-friendly when dealing with hundreds of tasks.
>
> Support multiple task assignment in one command with tasks ids separated
> by commas. For example:
> $echo 123,456,789 > /sys/fs/resctrl/ctrl_grp1/tasks
>
> Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
> Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> ---
> Documentation/arch/x86/resctrl.rst | 8 +++++++-
> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 25 ++++++++++++++++++++++---
> 2 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index cb05d90111b4..af234681756e 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -299,7 +299,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
> + group. Multiple tasks can be added by separating the task ids
> + with commas. Tasks will be assigned sequentially. Multiple
> + failures are not supported. A single failure encountered while
> + attempting to assign a task will cause the operation to abort.
What happens to the already moved tasks when "abort"?
Could you please add add more details on "abort"?
"A single failure encountered while attempting to assign a task will
cause the operation to abort and already added tasks before the failure
will remain in the group."
> + Failures will be logged to /sys/fs/resctrl/info/last_cmd_status.
> +
> + 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
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 725344048f85..8c91c333f9b3 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -696,11 +696,10 @@ 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)
> - return -EINVAL;
> rdtgrp = rdtgroup_kn_lock_live(of->kn);
> if (!rdtgrp) {
> rdtgroup_kn_unlock(of->kn);
> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct kernfs_open_file *of,
> goto unlock;
> }
>
> - ret = rdtgroup_move_task(pid, rdtgrp, of);
> + while (buf && buf[0] != '\0' && buf[0] != '\n') {
> + pid_str = strim(strsep(&buf, ","));
> +
> + if (kstrtoint(pid_str, 0, &pid)) {
> + rdt_last_cmd_puts("Task list parsing error\n");
It would be better to show the failed pid string in the failure report:
+ rdt_last_cmd_puts("Task list parsing error pid %s\n", pid_str);
So user will know which pid string causes the failure?
> + ret = -EINVAL;
> + break;
> + }
> +
> + if (pid < 0) {
> + rdt_last_cmd_printf("Invalid pid %d\n", pid);
> + ret = -EINVAL;
> + break;
> + }
> +
> + ret = rdtgroup_move_task(pid, rdtgrp, of);
> + if (ret) {
> + rdt_last_cmd_printf("Error while processing task %d\n", pid);
> + break;
> + }
> + }
>
> unlock:
> rdtgroup_kn_unlock(of->kn);
Thanks.
-Fenghua
Hi Fenghua,
On 9/1/2023 5:13 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 8/21/23 16:30, 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/ctrl_grp1
>> $echo 123 > /sys/fs/resctrl/ctrl_grp1/tasks
>> $echo 456 > /sys/fs/resctrl/ctrl_grp1/tasks
>> $echo 789 > /sys/fs/resctrl/ctrl_grp1/tasks
>>
>> This is not user-friendly when dealing with hundreds of tasks.
>>
>> Support multiple task assignment in one command with tasks ids separated
>> by commas. For example:
>> $echo 123,456,789 > /sys/fs/resctrl/ctrl_grp1/tasks
>>
>> Reviewed-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
>> Tested-by: Tan Shaopeng <tan.shaopeng@jp.fujitsu.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
>> ---
>> Documentation/arch/x86/resctrl.rst | 8 +++++++-
>> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 25 ++++++++++++++++++++++---
>> 2 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst
>> b/Documentation/arch/x86/resctrl.rst
>> index cb05d90111b4..af234681756e 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -299,7 +299,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
>> + group. Multiple tasks can be added by separating the task ids
>> + with commas. Tasks will be assigned sequentially. Multiple
>> + failures are not supported. A single failure encountered while
>> + attempting to assign a task will cause the operation to abort.
>
> What happens to the already moved tasks when "abort"?
>
> Could you please add add more details on "abort"?
>
> "A single failure encountered while attempting to assign a task will
> cause the operation to abort and already added tasks before the
> failure will remain in the group."
Sure.
>
>> + Failures will be logged to /sys/fs/resctrl/info/last_cmd_status.
>> +
>> + 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
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 725344048f85..8c91c333f9b3 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -696,11 +696,10 @@ 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)
>> - return -EINVAL;
>> rdtgrp = rdtgroup_kn_lock_live(of->kn);
>> if (!rdtgrp) {
>> rdtgroup_kn_unlock(of->kn);
>> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct
>> kernfs_open_file *of,
>> goto unlock;
>> }
>> - ret = rdtgroup_move_task(pid, rdtgrp, of);
>> + while (buf && buf[0] != '\0' && buf[0] != '\n') {
>> + pid_str = strim(strsep(&buf, ","));
>> +
>> + if (kstrtoint(pid_str, 0, &pid)) {
>> + rdt_last_cmd_puts("Task list parsing error\n");
>
> It would be better to show the failed pid string in the failure report:
> + rdt_last_cmd_puts("Task list parsing error pid %s\n",
> pid_str);
>
> So user will know which pid string causes the failure?
It was already discussed. Printing the characters during parsing error
may not be much useful.
Thanks
Babu
Hi, Babu,
On 9/6/23 07:56, Moger, Babu wrote:
>>> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct
>>> kernfs_open_file *of,
>>> goto unlock;
>>> }
>>> - ret = rdtgroup_move_task(pid, rdtgrp, of);
>>> + while (buf && buf[0] != '\0' && buf[0] != '\n') {
>>> + pid_str = strim(strsep(&buf, ","));
>>> +
>>> + if (kstrtoint(pid_str, 0, &pid)) {
>>> + rdt_last_cmd_puts("Task list parsing error\n");
>>
>> It would be better to show the failed pid string in the failure report:
>> + rdt_last_cmd_puts("Task list parsing error pid %s\n",
>> pid_str);
>>
>> So user will know which pid string causes the failure?
>
> It was already discussed. Printing the characters during parsing error
> may not be much useful.
Could you please let me know where printing "pid_str" is discussed?
My understanding is a similar thing is discussed in v3:
https://lore.kernel.org/all/167778866506.1053859.2329229096484796501.stgit@bmoger-ubuntu/
Then v4 has this code without printing pid_str. In v4, there is a
similar discussion of printing pid, but not pid_str.
But I cannot find a discussion of why "pid_str" is not printed.
If kstritoint(pid_str, 0, &pid) fails, without printing pid_str, how can
user know which pid string fails? e.g. user tries to move 100 pids and
the 51st pid parsing fails. It's hard for user to know the 51st pid
fails without showing pid_str in the error info and then it's hard for
the user to decide to re-do moving or aborting moving etc.
Thanks.
-Fenghua
then
Hi Fenghua,
On 9/6/2023 3:42 PM, Fenghua Yu wrote:
> Hi, Babu,
>
> On 9/6/23 07:56, Moger, Babu wrote:
>>>> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct
>>>> kernfs_open_file *of,
>>>> goto unlock;
>>>> }
>>>> - ret = rdtgroup_move_task(pid, rdtgrp, of);
>>>> + while (buf && buf[0] != '\0' && buf[0] != '\n') {
>>>> + pid_str = strim(strsep(&buf, ","));
>>>> +
>>>> + if (kstrtoint(pid_str, 0, &pid)) {
>>>> + rdt_last_cmd_puts("Task list parsing error\n");
>>>
>>> It would be better to show the failed pid string in the failure report:
>>> + rdt_last_cmd_puts("Task list parsing error pid %s\n",
>>> pid_str);
>>>
>>> So user will know which pid string causes the failure?
>>
>> It was already discussed. Printing the characters during parsing
>> error may not be much useful.
>
> Could you please let me know where printing "pid_str" is discussed?
My bad. Should have read your comments more carefully.
>
> My understanding is a similar thing is discussed in v3:
> https://lore.kernel.org/all/167778866506.1053859.2329229096484796501.stgit@bmoger-ubuntu/
>
>
> Then v4 has this code without printing pid_str. In v4, there is a
> similar discussion of printing pid, but not pid_str.
>
> But I cannot find a discussion of why "pid_str" is not printed.
>
> If kstritoint(pid_str, 0, &pid) fails, without printing pid_str, how
> can user know which pid string fails? e.g. user tries to move 100 pids
> and the 51st pid parsing fails. It's hard for user to know the 51st
> pid fails without showing pid_str in the error info and then it's hard
> for the user to decide to re-do moving or aborting moving etc.
That is correct. Will add following print statement o print the pid_str.
rdt_last_cmd_printf("Task list parsing error pid %s\n", pid_str);
Thanks
Babu
Hi, Babu,
On 9/7/23 07:44, Moger, Babu wrote:
> Hi Fenghua,
>
> On 9/6/2023 3:42 PM, Fenghua Yu wrote:
>> Hi, Babu,
>>
>> On 9/6/23 07:56, Moger, Babu wrote:
>>>>> @@ -715,7 +714,27 @@ static ssize_t rdtgroup_tasks_write(struct
>>>>> kernfs_open_file *of,
>>>>> goto unlock;
>>>>> }
>>>>> - ret = rdtgroup_move_task(pid, rdtgrp, of);
>>>>> + while (buf && buf[0] != '\0' && buf[0] != '\n') {
>>>>> + pid_str = strim(strsep(&buf, ","));
>>>>> +
>>>>> + if (kstrtoint(pid_str, 0, &pid)) {
>>>>> + rdt_last_cmd_puts("Task list parsing error\n");
>>>>
>>>> It would be better to show the failed pid string in the failure report:
>>>> + rdt_last_cmd_puts("Task list parsing error pid %s\n",
>>>> pid_str);
>>>>
>>>> So user will know which pid string causes the failure?
>>>
>>> It was already discussed. Printing the characters during parsing
>>> error may not be much useful.
>>
>> Could you please let me know where printing "pid_str" is discussed?
>
> My bad. Should have read your comments more carefully.
Thank you for your clarification.
>
>
>>
>> My understanding is a similar thing is discussed in v3:
>> https://lore.kernel.org/all/167778866506.1053859.2329229096484796501.stgit@bmoger-ubuntu/
>>
>>
>> Then v4 has this code without printing pid_str. In v4, there is a
>> similar discussion of printing pid, but not pid_str.
>>
>> But I cannot find a discussion of why "pid_str" is not printed.
>>
>> If kstritoint(pid_str, 0, &pid) fails, without printing pid_str, how
>> can user know which pid string fails? e.g. user tries to move 100 pids
>> and the 51st pid parsing fails. It's hard for user to know the 51st
>> pid fails without showing pid_str in the error info and then it's hard
>> for the user to decide to re-do moving or aborting moving etc.
>
> That is correct. Will add following print statement o print the pid_str.
>
> rdt_last_cmd_printf("Task list parsing error pid %s\n", pid_str);
Great!
Thanks.
-Fenghua
© 2016 - 2025 Red Hat, Inc.