2018-11-28 18:23:58

by Reinette Chatre

[permalink] [raw]
Subject: [PATCH] x86/resctrl: Ensure usage of CPUs are locked while needed

The user triggers the creation of a pseudo-locked region when writing
the requested schemata to the schemata resctrl file. The pseudo-locking
of a region is required to be done on a CPU that is associated with the
cache on which the pseudo-locked region will reside. In order to run the
locking code on a specific CPU the needed CPU has to be selected and
ensured to remain online during the entire locking sequence.

At this time the cpu_hotplug_lock is not taken during the pseudo-lock
region creation and it is thus possible for a CPU to be selected to run
the pseudo-locking code and then that CPU to go offline before the
thread is able to run on it.

Fix this by ensuring that the cpu_hotplug_lock is taken while the CPU on
which code has to run needs to be controlled. Since the cpu_hotplug_lock
is always taken before rdtgroup_mutex the lock order is maintained.

Fixes: e0bdfe8e36f3 ("x86/intel_rdt: Support creation/removal of pseudo-locked region")
Signed-off-by: Reinette Chatre <[email protected]>
Cc: [email protected]
---
arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
index 03ee13235a45..6f7adb3be01e 100644
--- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
+++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c
@@ -23,6 +23,7 @@

#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

+#include <linux/cpu.h>
#include <linux/kernfs.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
@@ -381,9 +382,11 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
return -EINVAL;
buf[nbytes - 1] = '\0';

+ cpus_read_lock();
rdtgrp = rdtgroup_kn_lock_live(of->kn);
if (!rdtgrp) {
rdtgroup_kn_unlock(of->kn);
+ cpus_read_unlock();
return -ENOENT;
}
rdt_last_cmd_clear();
@@ -438,6 +441,7 @@ ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,

out:
rdtgroup_kn_unlock(of->kn);
+ cpus_read_unlock();
return ret ?: nbytes;
}

--
2.17.0



2018-12-10 21:47:03

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] x86/resctrl: Ensure usage of CPUs are locked while needed

On Wed, Nov 28, 2018 at 10:22:07AM -0800, Reinette Chatre wrote:
> The user triggers the creation of a pseudo-locked region when writing
> the requested schemata to the schemata resctrl file. The pseudo-locking
> of a region is required to be done on a CPU that is associated with the
> cache on which the pseudo-locked region will reside. In order to run the
> locking code on a specific CPU the needed CPU has to be selected and
> ensured to remain online during the entire locking sequence.
>
> At this time the cpu_hotplug_lock is not taken during the pseudo-lock
> region creation and it is thus possible for a CPU to be selected to run
> the pseudo-locking code and then that CPU to go offline before the
> thread is able to run on it.
>
> Fix this by ensuring that the cpu_hotplug_lock is taken while the CPU on
> which code has to run needs to be controlled. Since the cpu_hotplug_lock
> is always taken before rdtgroup_mutex the lock order is maintained.
>
> Fixes: e0bdfe8e36f3 ("x86/intel_rdt: Support creation/removal of pseudo-locked region")
> Signed-off-by: Reinette Chatre <[email protected]>
> Cc: [email protected]
> ---
> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 ++++
> 1 file changed, 4 insertions(+)

This looks like it wants to go in now, right?

If so, pls redo this patch against tip/x86/urgent which has the old
filenames, before the resctrl rename and reorg.

Thx.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

2018-12-10 21:48:19

by Reinette Chatre

[permalink] [raw]
Subject: Re: [PATCH] x86/resctrl: Ensure usage of CPUs are locked while needed

Hi Boris,

On 12/10/2018 12:49 PM, Borislav Petkov wrote:
> On Wed, Nov 28, 2018 at 10:22:07AM -0800, Reinette Chatre wrote:
>> The user triggers the creation of a pseudo-locked region when writing
>> the requested schemata to the schemata resctrl file. The pseudo-locking
>> of a region is required to be done on a CPU that is associated with the
>> cache on which the pseudo-locked region will reside. In order to run the
>> locking code on a specific CPU the needed CPU has to be selected and
>> ensured to remain online during the entire locking sequence.
>>
>> At this time the cpu_hotplug_lock is not taken during the pseudo-lock
>> region creation and it is thus possible for a CPU to be selected to run
>> the pseudo-locking code and then that CPU to go offline before the
>> thread is able to run on it.
>>
>> Fix this by ensuring that the cpu_hotplug_lock is taken while the CPU on
>> which code has to run needs to be controlled. Since the cpu_hotplug_lock
>> is always taken before rdtgroup_mutex the lock order is maintained.
>>
>> Fixes: e0bdfe8e36f3 ("x86/intel_rdt: Support creation/removal of pseudo-locked region")
>> Signed-off-by: Reinette Chatre <[email protected]>
>> Cc: [email protected]
>> ---
>> arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>
> This looks like it wants to go in now, right?

Yes, please.

>
> If so, pls redo this patch against tip/x86/urgent which has the old
> filenames, before the resctrl rename and reorg.

Will do.

Thank you

Reinette