Hi Dave,
On 18/02/2019 19:52, Dave Martin wrote:
> This patch updates fpsimd_flush_task_state() to mirror the new
> semantics of fpsimd_flush_cpu_state() introduced by commit
> d8ad71fa38a9 ("arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after
> invalidating cpu regs"). Both functions now implicitly set
NIT: Double-space before "Both"
> TIF_FOREIGN_FPSTATE to indicate that the task's FPSIMD state is not
> loaded into the cpu.
>
> As a side-effect, fpsimd_flush_task_state() now sets
> TIF_FOREIGN_FPSTATE even for non-running tasks. In the case of
NIT: Double sppace before "In".
> non-running tasks this is not useful but also harmless, because the
> flag is live only while the corresponding task is running. This
> function is not called from fast paths, so special-casing this for
> the task == current case is not really worth it.
>
> Compiler barriers previously present in restore_sve_fpsimd_context()
> are pulled into fpsimd_flush_task_state() so that it can be safely
> called with preemption enabled if necessary.
>
> Explicit calls to set TIF_FOREIGN_FPSTATE that accompany
> fpsimd_flush_task_state() calls and are now redundant are removed
> as appropriate.
>
> fpsimd_flush_task_state() is used to get exclusive access to the
> representation of the task's state via task_struct, for the purpose
> of replacing the state. Thus, the call to this function should
NIT: Double-space before "Thus".
> happen before manipulating fpsimd_state or sve_state etc. in
> task_struct. Anomalous cases are reordered appropriately in order
NIT: Double-space before "Anomalous".
> to make the code more consistent, although there should be no
> functional difference since these cases are protected by
> local_bh_disable() anyway.
>
> Signed-off-by: Dave Martin <Dave.Martin at arm.com>
> Reviewed-by: Alex Benn?e <alex.bennee at linaro.org>
Reviewed-by: Julien Grall <julien.grall at arm.com>
Cheers,
--
Julien Grall
