Re: [PATCH v3 0/8] arm64/sve: First steps towards optimizing syscalls

From: Dave Martin <Dave.Martin@arm.com>
To: Mark Brown <broonie@kernel.org>
Cc: Julien Grall <julien@xen.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	zhang.lei@jp.fujitsu.com, Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Daniel Kiss <Daniel.Kiss@arm.com>
Subject: Re: [PATCH v3 0/8] arm64/sve: First steps towards optimizing syscalls
Date: Wed, 15 Jul 2020 17:49:34 +0100	[thread overview]
Message-ID: <20200715164931.GC30452@arm.com> (raw)
In-Reply-To: <20200629133556.39825-1-broonie@kernel.org>

On Mon, Jun 29, 2020 at 02:35:48PM +0100, Mark Brown wrote:
> This is a first attempt to optimize the syscall path when the user
> application uses SVE. The patch series was originally written by Julien
> Grall but has been left for a long time, I've updated it to current
> kernels and tried to address the pending review feedback that I found
> (which was mostly documentation issues). I may have missed some things
> there, apologies if I did, and one thing I've not yet done is produced a
> diagram of the states the relevant TIF_ flags can have - I need to work
> out a sensible format for that.
> 
> Per the syscall ABI, SVE registers will be unknown after a syscall. In
> practice, the kernel will disable SVE and the registers will be zeroed
> (except the first 128-bits of each vector) on the next SVE instruction.
> In a workload mixing SVE and syscalls, this will result to 2 entry/exit
> to the kernel per syscall as we trap on the first SVE access after the
> syscall.  This series aims to avoid the second entry/exit by zeroing the
> SVE registers on syscall return with a twist when the task will get
> rescheduled.
> 
> This implementation will have an impact on application using SVE
> only once. SVE will now be turned on until the application terminates
> (unless disabling it via ptrace). Cleverer strategies for choosing
> between SVE and FPSIMD context switching are possible (see fpu_counter
> for SH in mainline, or [1]), but it is difficult to assess the benefit
> right now. We could improve the behaviour in the future as a selection
> of mature hardware platforsm emerges that we can benchmark.
> 
> It is also possible to optimize the case when the SVE vector-length
> is 128-bit (i.e the same size as the FPSIMD vectors). This could be
> explored in the future.
> 
> Note that the last patch for the series is is not here to optimize syscall
> but SVE trap access by directly converting in hardware the FPSIMD state
> to SVE state. If there are an interest to have this optimization earlier,
> I can reshuffle the patches in the series.
> 
> v3:
>  - Rebased to current kernels.
>  - Addressed review comments from v2, mostly around tweaks in the
>    documentation.

Looks reasonable overall, apart from a few questions on some details
(partly because I haven't thought deeply about this stuff for a while).

I wonder whether we ought to accompany this with a crude mechanism to
choose dynamically between setting TIF_SVE_NEEDS_FLUSH and keeping the
old behaviour.

My concern with doing this unconditionally has been that we can end up
with TIF_SVE permanently stuck on, which increases the per-task overhead.
This is not a worry if the user task really does use SVE once per
context switch, but not so good if, say, the libc startup probes for
SVE to initialise some internal logic but the task otherwise doesn't
use it.  (This is just a worry: I haven't looked for evidence to support
it.)

Either way, we should keep it pretty dumb until/unless we have
compelling rationale for doing something cleverer.

Cheers
---Dave

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