lkml.org 
[lkml]   [2020]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] entry: split lockdep and syscall work functions
Date
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 <svens@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

\
 
 \ /
  Last update: 2020-12-01 12:11    [W:0.058 / U:0.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site