diff mbox series

[RFC,2/3] x86/resctrl: Move the task's threads to the group automatically

Message ID 167278360189.34228.2442698916556329960.stgit@bmoger-ubuntu
State New
Headers
Series x86/resctrl: Miscellaneous resctrl features |

Commit Message

Moger, Babu Jan. 3, 2023, 10:06 p.m. UTC 
  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(-)

Comments

Fenghua Yu Jan. 4, 2023, 5:55 a.m. UTC | #1 
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
Reinette Chatre Jan. 4, 2023, 4:43 p.m. UTC | #2 
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
Moger, Babu Jan. 4, 2023, 5:49 p.m. UTC | #3 
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
Moger, Babu Jan. 4, 2023, 6:06 p.m. UTC | #4 
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
diff mbox series

Patch

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;