linux-kernel - Re: [PATCH v2 0/3] x86/intel_rdt: task_work vs task

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: <87be8915-21b0-5214-9742-ccc7515c298b@intel.com>
Date:   Tue, 24 Nov 2020 13:37:36 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Valentin Schneider <valentin.schneider@....com>,
        linux-kernel@...r.kernel.org, x86@...nel.org
Cc:     Fenghua Yu <fenghua.yu@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>, James Morse <James.Morse@....com>
Subject: Re: [PATCH v2 0/3] x86/intel_rdt: task_work vs task_struct
 rmid/closid write race

Hi Valentin,

On 11/22/2020 6:24 PM, Valentin Schneider wrote:
> This is a small cleanup + a fix for a race I stumbled upon while staring at
> resctrl stuff.
> 
> Briefly tested on a Xeon Gold 5120 (m2.xlarge.x86 on Equinix) by bouncing
> a few tasks around control groups.
> 

...

Thank you very much for taking this on. Unfortunately this race is one 
of a few issues with the way in which tasks moving to a new resource 
group is handled.

Other issues are:

1.
Until the queued work is run, the moved task runs with old (and even
invalid in the case when its original resource group has been removed)
closid and rmid.

2.
Work to update the PQR_ASSOC register is queued every time the user 
writes a task id to the "tasks" file, even if the task already belongs 
to the resource group and in addition to any other pending work for that 
task. This could result in multiple pending work items associated with a 
single task even if they are all identical and even though only a single 
update with most recent values is needed. This could result in 
significant system resource waste, especially on tasks sleeping for a 
long time.

Fenghua solved these issues by replacing the callback with a synchronous 
update, similar to how tasks are currently moved when a resource group 
is deleted. We plan to submit this work next week.

This new solution will make patch 1 and 2 of this series unnecessary. As 
I understand it patch 3 would still be a welcome addition but would 
require changes. As you prefer you could either submit patch 3 on its 
own for the code as it is now and we will rework the task related 
changes on top of that, or you could wait for the task related changes 
to land first?

> 
> Valentin Schneider (3):
>    x86/intel_rdt: Check monitor group vs control group membership earlier
>    x86/intel_rdt: Plug task_work vs task_struct {rmid,closid} update race
>    x86/intel_rdt: Apply READ_ONCE/WRITE_ONCE to task_struct .rmid &
>      .closid
> 

Thank you very much

Reinette