Re: [PATCH v5 02/26] arm64: fpsimd: Always set TIF_FOREIGN_FPSTATE on task state flush

[Dave Martin <Dave.Martin@arm.com>]

On Thu, Feb 21, 2019 at 12:39:39PM +0000, Julien Grall wrote:
> 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".

A habit rather than a mistake [1], and I don't propose to change it ;)


> >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@arm.com>
> >Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 
> Reviewed-by: Julien Grall <julien.grall@arm.com>

Thanks for the review
--Dave


[1] https://en.wikipedia.org/wiki/Sentence_spacing

Interestingly around 11% of commits in mainline appear to follow the
two-space convention for their commit messages.  I won't speculate as
to why...

$ git log --oneline --grep='[a-z]\.  [A-Z]' | wc -l
94638
$ git log --oneline | wc -l
812542