Re: [PATCH 2/2] arm64/sve: Eliminate data races on sve_default_vl

From: Will Deacon <will@kernel.org>
To: Qian Cai <cai@lca.pw>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Dave Martin <Dave.Martin@arm.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] arm64/sve: Eliminate data races on sve_default_vl
Date: Tue, 16 Jun 2020 16:04:51 +0100	[thread overview]
Message-ID: <20200616150451.GA1941@willie-the-truck> (raw)
In-Reply-To: <20200616131808.GA1040@lca.pw>

On Tue, Jun 16, 2020 at 09:18:08AM -0400, Qian Cai wrote:
> On Wed, Jun 10, 2020 at 06:03:10PM +0100, Dave Martin wrote:
> > 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 <will@kernel.org>
> > Signed-off-by: Dave Martin <Dave.Martin@arm.com>
> 
> While this original patch looks correct, linux-next has this,
> 
> [will: move set_sve_default_vl() inside #ifdef to squash allnoconfig warning]
> 
> 1e570f512cbd ("arm64/sve: Eliminate data races on sve_default_vl")
> 
> which causes an error with CONFIG_ARM64_SVE=n,
> 
> This .config,
> https://raw.githubusercontent.com/cailca/linux-mm/master/arm64.config
> 
> arch/arm64/kernel/fpsimd.c: In function ‘sve_proc_do_default_vl’:
> arch/arm64/kernel/fpsimd.c:375:2: error: implicit declaration of
> function ‘set_sve_default_vl’; did you mean ‘get_sve_default_vl’?
> [-Werror=implicit-function-declaration]
>   set_sve_default_vl(find_supported_vector_length(vl));
>   ^~~~~~~~~~~~~~~~~~
>   get_sve_default_vl

Thanks, I'll take a look.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel