Re: [PATCH v2 0/3] x86/intel_rdt: task_work vs task_struct rmid/closid write race

[Reinette Chatre <reinette.chatre@intel.com>]

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