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

[Yu, Fenghua]

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@xxxxxxx>
> ---
>  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