Messages in this thread | | | From | Brian Gerst <> | Date | Thu, 4 Mar 2021 19:55:00 -0500 | Subject | Re: [PATCH v3 05/11] x86/entry: Convert ret_from_fork to C |
| |
On Thu, Mar 4, 2021 at 2:16 PM Andy Lutomirski <luto@kernel.org> wrote: > > ret_from_fork is written in asm, slightly differently, for x86_32 and > x86_64. Convert it to C. > > This is a straight conversion without any particular cleverness. As a > further cleanup, the code that sets up the ret_from_fork argument registers > could be adjusted to put the arguments in the correct registers.
An alternative would be to stash the function pointer and argument in the pt_regs of the new kthread task, and test for PF_KTHREAD instead. That would eliminate the need to pass those two values to the C version of ret_from_fork().
> This will cause the ORC unwinder to find pt_regs even for kernel threads on > x86_64. This seems harmless. > > The 32-bit comment above the now-deleted schedule_tail_wrapper was > obsolete: the encode_frame_pointer mechanism (see copy_thread()) solves the > same problem more cleanly. > > Cc: Josh Poimboeuf <jpoimboe@redhat.com> > Signed-off-by: Andy Lutomirski <luto@kernel.org> > --- > arch/x86/entry/common.c | 23 ++++++++++++++ > arch/x86/entry/entry_32.S | 51 +++++--------------------------- > arch/x86/entry/entry_64.S | 33 +++++---------------- > arch/x86/include/asm/switch_to.h | 2 +- > arch/x86/kernel/process.c | 2 +- > arch/x86/kernel/process_32.c | 2 +- > arch/x86/kernel/unwind_orc.c | 2 +- > 7 files changed, 43 insertions(+), 72 deletions(-) > > diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c > index 95776f16c1cb..ef1c65938a6b 100644 > --- a/arch/x86/entry/common.c > +++ b/arch/x86/entry/common.c > @@ -214,6 +214,29 @@ SYSCALL_DEFINE0(ni_syscall) > return -ENOSYS; > } > > +void ret_from_fork(struct task_struct *prev, > + int (*kernel_thread_fn)(void *), > + void *kernel_thread_arg, > + struct pt_regs *user_regs); > + > +__visible void noinstr ret_from_fork(struct task_struct *prev, > + int (*kernel_thread_fn)(void *), > + void *kernel_thread_arg, > + struct pt_regs *user_regs) > +{ > + instrumentation_begin(); > + > + schedule_tail(prev); > + > + if (kernel_thread_fn) {
This should have an unlikely(), as kernel threads should be the rare case.
-- Brian Gerst
| |