Sven,
On Tue, Dec 01 2020 at 09:35, Sven Schnelle wrote:
> On s390, we can not call one function which sets lockdep
> state and do the syscall work at the same time. There add
> make enter_from_user_mode() and exit_to_user_mode() public, and
> add syscall_exit_to_user_mode1() which does the same as
> syscall_exit_to_user_mode() but skips the final exit_to_user_mode().
the explanation in the "cover letter" made at least sense, but the above
is unparseable word salad.
> Signed-off-by: Sven Schnelle <sv...@linux.ibm.com>
> ---
> include/linux/entry-common.h | 4 +++-
> kernel/entry/common.c | 35 +++++++++++++++++++++++++++--------
> 2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
> index 474f29638d2c..496c9a47eab4 100644
> --- a/include/linux/entry-common.h
> +++ b/include/linux/entry-common.h
> @@ -124,7 +124,7 @@ static inline __must_check int
> arch_syscall_enter_tracehook(struct pt_regs *regs
> * to be done between establishing state and handling user mode entry work.
> */
> void syscall_enter_from_user_mode_prepare(struct pt_regs *regs);
> -
> +void enter_from_user_mode(struct pt_regs *regs);
You might have noticed, that all of these function prototypes have
proper kernel documentation. So just glueing this on to the previous
prototype does not cut it. enter_from/exit_to_user_mode() want to go
together into a seperate section.
> /**
> * syscall_enter_from_user_mode_work - Check and handle work before invoking
> * a syscall
> @@ -311,6 +311,8 @@ static inline void arch_syscall_exit_tracehook(struct
> pt_regs *regs, bool step)
> * arch_exit_to_user_mode() to handle e.g. speculation mitigations
> */
> void syscall_exit_to_user_mode(struct pt_regs *regs);
> +void syscall_exit_to_user_mode1(struct pt_regs *regs);
Same here and as you mentioned ...mode1() is a pretty horrible name.
syscall_exit_to_user_mode_work() perhaps?
> +void exit_to_user_mode(void);
>
> /**
> * irqentry_enter_from_user_mode - Establish state before invoking the irq
> handler
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index e9e2df3f3f9e..3ad462ebfa15 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -18,7 +18,7 @@
> * 2) Invoke context tracking if enabled to reactivate RCU
> * 3) Trace interrupts off state
> */
> -static __always_inline void enter_from_user_mode(struct pt_regs *regs)
> +static __always_inline void __enter_from_user_mode(struct pt_regs
> *regs)
Can you please split the renaming into a seperate preparatory patch?
> +__visible noinstr void syscall_exit_to_user_mode1(struct pt_regs *regs)
What's the point of marking this function noinstr? Everything it does is
instrumentable.
Thanks,
tglx
