Skip to content

Commit 1e570f5

Browse files
Dave Martinwilldeacon
authored andcommitted
arm64/sve: Eliminate data races on sve_default_vl
sve_default_vl can be modified via the /proc/sys/abi/sve_default_vl sysctl concurrently with use, and modified concurrently by multiple threads. Adding a lock for this seems overkill, and I don't want to think any more than necessary, so just define wrappers using READ_ONCE()/ WRITE_ONCE(). This will avoid the possibility of torn accesses and repeated loads and stores. There's no evidence yet that this is going wrong in practice: this is just hygiene. For generic sysctl users, it would be better to build this kind of thing into the sysctl common code somehow. Reported-by: Will Deacon <[email protected]> Signed-off-by: Dave Martin <[email protected]> Link: https://lore.kernel.org/r/[email protected] [will: move set_sve_default_vl() inside #ifdef to squash allnoconfig warning] Signed-off-by: Will Deacon <[email protected]>
1 parent 9ba6a9e commit 1e570f5

File tree

1 file changed

+18
-7
lines changed

1 file changed

+18
-7
lines changed

arch/arm64/kernel/fpsimd.c

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <linux/bug.h>
1313
#include <linux/cache.h>
1414
#include <linux/compat.h>
15+
#include <linux/compiler.h>
1516
#include <linux/cpu.h>
1617
#include <linux/cpu_pm.h>
1718
#include <linux/kernel.h>
@@ -119,10 +120,20 @@ struct fpsimd_last_state_struct {
119120
static DEFINE_PER_CPU(struct fpsimd_last_state_struct, fpsimd_last_state);
120121

121122
/* Default VL for tasks that don't set it explicitly: */
122-
static int sve_default_vl = -1;
123+
static int __sve_default_vl = -1;
124+
125+
static int get_sve_default_vl(void)
126+
{
127+
return READ_ONCE(__sve_default_vl);
128+
}
123129

124130
#ifdef CONFIG_ARM64_SVE
125131

132+
static void set_sve_default_vl(int val)
133+
{
134+
WRITE_ONCE(__sve_default_vl, val);
135+
}
136+
126137
/* Maximum supported vector length across all CPUs (initially poisoned) */
127138
int __ro_after_init sve_max_vl = SVE_VL_MIN;
128139
int __ro_after_init sve_max_virtualisable_vl = SVE_VL_MIN;
@@ -344,7 +355,7 @@ static int sve_proc_do_default_vl(struct ctl_table *table, int write,
344355
void *buffer, size_t *lenp, loff_t *ppos)
345356
{
346357
int ret;
347-
int vl = sve_default_vl;
358+
int vl = get_sve_default_vl();
348359
struct ctl_table tmp_table = {
349360
.data = &vl,
350361
.maxlen = sizeof(vl),
@@ -361,7 +372,7 @@ static int sve_proc_do_default_vl(struct ctl_table *table, int write,
361372
if (!sve_vl_valid(vl))
362373
return -EINVAL;
363374

364-
sve_default_vl = find_supported_vector_length(vl);
375+
set_sve_default_vl(find_supported_vector_length(vl));
365376
return 0;
366377
}
367378

@@ -868,7 +879,7 @@ void __init sve_setup(void)
868879
* For the default VL, pick the maximum supported value <= 64.
869880
* VL == 64 is guaranteed not to grow the signal frame.
870881
*/
871-
sve_default_vl = find_supported_vector_length(64);
882+
set_sve_default_vl(find_supported_vector_length(64));
872883

873884
bitmap_andnot(tmp_map, sve_vq_partial_map, sve_vq_map,
874885
SVE_VQ_MAX);
@@ -889,7 +900,7 @@ void __init sve_setup(void)
889900
pr_info("SVE: maximum available vector length %u bytes per vector\n",
890901
sve_max_vl);
891902
pr_info("SVE: default vector length %u bytes per vector\n",
892-
sve_default_vl);
903+
get_sve_default_vl());
893904

894905
/* KVM decides whether to support mismatched systems. Just warn here: */
895906
if (sve_max_virtualisable_vl < sve_max_vl)
@@ -1029,13 +1040,13 @@ void fpsimd_flush_thread(void)
10291040
* vector length configured: no kernel task can become a user
10301041
* task without an exec and hence a call to this function.
10311042
* By the time the first call to this function is made, all
1032-
* early hardware probing is complete, so sve_default_vl
1043+
* early hardware probing is complete, so __sve_default_vl
10331044
* should be valid.
10341045
* If a bug causes this to go wrong, we make some noise and
10351046
* try to fudge thread.sve_vl to a safe value here.
10361047
*/
10371048
vl = current->thread.sve_vl_onexec ?
1038-
current->thread.sve_vl_onexec : sve_default_vl;
1049+
current->thread.sve_vl_onexec : get_sve_default_vl();
10391050

10401051
if (WARN_ON(!sve_vl_valid(vl)))
10411052
vl = SVE_VL_MIN;

0 commit comments

Comments
 (0)