linux-kernel - Re: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group automatically

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <e3b5286f-469e-90ba-e4a9-09153c9562d4@amd.com>
Date:   Wed, 4 Jan 2023 11:49:57 -0600
From:   "Moger, Babu" <babu.moger@....com>
To:     "Yu, Fenghua" <fenghua.yu@...el.com>,
        "Chatre, Reinette" <reinette.chatre@...el.com>
Cc:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "Eranian, Stephane" <eranian@...gle.com>,
        "peternewman@...gle.com" <peternewman@...gle.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>
Subject: Re: [RFC PATCH 2/3] x86/resctrl: Move the task's threads to the group
 automatically

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