| Message ID | 20210930181144.10029-2-broonie@kernel.org |
|---|---|
| State | Accepted |
| Commit | 2d481bd3b6361ed16c3ddeb58537f149623d30a0 |
| Headers | show |
| Series | arm64/sme: Initial support for the Scalable Matrix Extension | expand |
On Thu, 30 Sep 2021 19:11:07 +0100 Mark Brown <broonie@kernel.org> wrote: > Currently all the active code in fpsimd_save() is inside a check for > TIF_FOREIGN_FPSTATE. Reduce the indentation level by changing to return > from the function if TIF_FOREIGN_FPSTATE is set. > > Signed-off-by: Mark Brown <broonie@kernel.org> Hi Mark Trivial formatting thing inline, but otherwise this is clearly a noop refactor as expected. > --- > arch/arm64/kernel/fpsimd.c | 38 ++++++++++++++++++++------------------ > 1 file changed, 20 insertions(+), 18 deletions(-) > > diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c > index ff4962750b3d..995f8801602b 100644 > --- a/arch/arm64/kernel/fpsimd.c > +++ b/arch/arm64/kernel/fpsimd.c > @@ -308,24 +308,26 @@ static void fpsimd_save(void) > WARN_ON(!system_supports_fpsimd()); > WARN_ON(!have_cpu_fpsimd_context()); > > - if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) { > - if (IS_ENABLED(CONFIG_ARM64_SVE) && > - test_thread_flag(TIF_SVE)) { > - if (WARN_ON(sve_get_vl() != last->sve_vl)) { > - /* > - * Can't save the user regs, so current would > - * re-enter user with corrupt state. > - * There's no way to recover, so kill it: > - */ > - force_signal_inject(SIGKILL, SI_KERNEL, 0, 0); > - return; > - } > - > - sve_save_state((char *)last->sve_state + > - sve_ffr_offset(last->sve_vl), > - &last->st->fpsr); > - } else > - fpsimd_save_state(last->st); > + if (test_thread_flag(TIF_FOREIGN_FPSTATE)) > + return; > + > + if (IS_ENABLED(CONFIG_ARM64_SVE) && > + test_thread_flag(TIF_SVE)) { Trivial, but - the above now fits nicely on oneline under 80 chars. Looks like you have another instance of this in patch 21 in roughly the same place. Then you drop at least some of this code later. hohum, maybe not worth tidying up. > + if (WARN_ON(sve_get_vl() != last->sve_vl)) { > + /* > + * Can't save the user regs, so current would > + * re-enter user with corrupt state. > + * There's no way to recover, so kill it: > + */ > + force_signal_inject(SIGKILL, SI_KERNEL, 0, 0); > + return; > + } > + > + sve_save_state((char *)last->sve_state + > + sve_ffr_offset(last->sve_vl), > + &last->st->fpsr); > + } else { > + fpsimd_save_state(last->st); > } > } >
On Mon, Oct 11, 2021 at 10:39:59AM +0100, Jonathan Cameron wrote: > > + if (IS_ENABLED(CONFIG_ARM64_SVE) && > > + test_thread_flag(TIF_SVE)) { > Trivial, but - the above now fits nicely on oneline under 80 chars. > Looks like you have another instance of this in patch 21 in roughly the same place. > Then you drop at least some of this code later. hohum, maybe not worth tidying up. Yes, indeed. I think I deliberately didn't reindent to help make it a bit more obviously mechanical given that as you've noticed there's going to be meaningful changes replacing the actual code later. Given that this is at the start of a very large series I'll just leave things as they are unless Catalin or Will really wants the reformatting here.
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c index ff4962750b3d..995f8801602b 100644 --- a/arch/arm64/kernel/fpsimd.c +++ b/arch/arm64/kernel/fpsimd.c @@ -308,24 +308,26 @@ static void fpsimd_save(void) WARN_ON(!system_supports_fpsimd()); WARN_ON(!have_cpu_fpsimd_context()); - if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) { - if (IS_ENABLED(CONFIG_ARM64_SVE) && - test_thread_flag(TIF_SVE)) { - if (WARN_ON(sve_get_vl() != last->sve_vl)) { - /* - * Can't save the user regs, so current would - * re-enter user with corrupt state. - * There's no way to recover, so kill it: - */ - force_signal_inject(SIGKILL, SI_KERNEL, 0, 0); - return; - } - - sve_save_state((char *)last->sve_state + - sve_ffr_offset(last->sve_vl), - &last->st->fpsr); - } else - fpsimd_save_state(last->st); + if (test_thread_flag(TIF_FOREIGN_FPSTATE)) + return; + + if (IS_ENABLED(CONFIG_ARM64_SVE) && + test_thread_flag(TIF_SVE)) { + if (WARN_ON(sve_get_vl() != last->sve_vl)) { + /* + * Can't save the user regs, so current would + * re-enter user with corrupt state. + * There's no way to recover, so kill it: + */ + force_signal_inject(SIGKILL, SI_KERNEL, 0, 0); + return; + } + + sve_save_state((char *)last->sve_state + + sve_ffr_offset(last->sve_vl), + &last->st->fpsr); + } else { + fpsimd_save_state(last->st); } }
Currently all the active code in fpsimd_save() is inside a check for TIF_FOREIGN_FPSTATE. Reduce the indentation level by changing to return from the function if TIF_FOREIGN_FPSTATE is set. Signed-off-by: Mark Brown <broonie@kernel.org> --- arch/arm64/kernel/fpsimd.c | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) -- 2.20.1