LinuxLists.cc - [RFC 0/2] allow to inline generic entry

2023-05-16 14:06:35

Subject: [RFC 0/2] allow to inline generic entry

Hi,

i looked into the syscall performance on s390 with the latest
kernel. For that reason i wrote a small syscall test program,
which just calls getpid() in a loop:

#include <stdio.h>
#include <time.h>
#include <bsd/sys/time.h>
#include <unistd.h>
static const double nsec_per_sec = 1000000000;

int main(int argc, char **argv)
{
struct timespec start, end, res;
double diff;
int i;
(void)argc;
(void)argv;

clock_gettime(CLOCK_REALTIME, &start);
for (i = 0; i < 150000000; i++) {
volatile int a = getpid();
(void)a;
}

clock_gettime(CLOCK_REALTIME, &end);
timespecsub(&end, &start, &res);
diff = ((double)res.tv_sec * nsec_per_sec + (double)res.tv_nsec) / nsec_per_sec;
printf("%f\n", diff);
return 0;
}

Analyzing performance data i see some overhead in the generic entry C
functions, which are not inlined because they are defined in
kernel/entry/common.c. Moving them to include/linux/entry-common.h
gives me the following runtime for the loop above:

with entry common code inlined: 12.8s
not inlined: 13.8s

While i prefer to have C functions in C files instead of header files,
7% performance gain is quite a lot, so i wonder what people think about
moving them to header files. I made this a small patchset for reference,
if there is interest in merging that i'll clean it up and submit it.

Any thoughts?

Sven Schnelle (2):
entry: move the exit path to header files
entry: move the enter path to header files

include/linux/entry-common.h | 305 ++++++++++++++++++++++++++++++++++-
kernel/entry/common.c | 281 --------------------------------
2 files changed, 297 insertions(+), 289 deletions(-)

--
2.39.2

2023-05-16 14:21:22

by Sven Schnelle

[permalink] [raw]

Subject: [PATCH 1/2] entry: move the exit path to header files

In order to allow inlining the generic entry C functions,
move them to include/linux/entry-common.h.

Signed-off-by: Sven Schnelle <[email protected]>
---
include/linux/entry-common.h | 182 ++++++++++++++++++++++++++++++++++-
kernel/entry/common.c | 168 --------------------------------
2 files changed, 179 insertions(+), 171 deletions(-)

diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h
index d95ab85f96ba..b409fbcbd3ac 100644
--- a/include/linux/entry-common.h
+++ b/include/linux/entry-common.h
@@ -7,8 +7,13 @@
#include <linux/syscalls.h>
#include <linux/seccomp.h>
#include <linux/sched.h>
-
+#include <linux/livepatch.h>
#include <asm/entry-common.h>
+#include <linux/context_tracking.h>
+#include <linux/resume_user_mode.h>
+#include <linux/tick.h>
+
+#include <trace/events/syscalls.h>

/*
* Define dummy _TIF work flags if not defined by the architecture or for
@@ -291,7 +296,7 @@ void exit_to_user_mode(void);
* make the final state transitions. Interrupts must stay disabled between
* return from this function and the invocation of exit_to_user_mode().
*/
-void syscall_exit_to_user_mode_work(struct pt_regs *regs);
+static void syscall_exit_to_user_mode_work(struct pt_regs *regs);

/**
* syscall_exit_to_user_mode - Handle work before returning to user mode
@@ -350,7 +355,7 @@ void irqentry_enter_from_user_mode(struct pt_regs *regs);
* Interrupt exit is not invoking #1 which is the syscall specific one time
* work.
*/
-void irqentry_exit_to_user_mode(struct pt_regs *regs);
+static void irqentry_exit_to_user_mode(struct pt_regs *regs);

#ifndef irqentry_state
/**
@@ -465,4 +470,175 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
*/
void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);

+static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
+ unsigned long ti_work)
+{
+ /*
+ * Before returning to user space ensure that all pending work
+ * items have been completed.
+ */
+ while (ti_work & EXIT_TO_USER_MODE_WORK) {
+
+ local_irq_enable_exit_to_user(ti_work);
+
+ if (ti_work & _TIF_NEED_RESCHED)
+ schedule();
+
+ if (ti_work & _TIF_UPROBE)
+ uprobe_notify_resume(regs);
+
+ if (ti_work & _TIF_PATCH_PENDING)
+ klp_update_patch_state(current);
+
+ if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
+ arch_do_signal_or_restart(regs);
+
+ if (ti_work & _TIF_NOTIFY_RESUME)
+ resume_user_mode_work(regs);
+
+ /* Architecture specific TIF work */
+ arch_exit_to_user_mode_work(regs, ti_work);
+
+ /*
+ * Disable interrupts and reevaluate the work flags as they
+ * might have changed while interrupts and preemption was
+ * enabled above.
+ */
+ local_irq_disable_exit_to_user();
+
+ /* Check if any of the above work has queued a deferred wakeup */
+ tick_nohz_user_enter_prepare();
+
+ ti_work = read_thread_flags();
+ }
+
+ /* Return the latest work state for arch_exit_to_user_mode() */
+ return ti_work;
+}
+
+
+static void exit_to_user_mode_prepare(struct pt_regs *regs)
+{
+ unsigned long ti_work;
+
+ lockdep_assert_irqs_disabled();
+
+ /* Flush pending rcuog wakeup before the last need_resched() check */
+ tick_nohz_user_enter_prepare();
+
+ ti_work = read_thread_flags();
+ if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
+ ti_work = exit_to_user_mode_loop(regs, ti_work);
+
+ arch_exit_to_user_mode_prepare(regs, ti_work);
+
+ /* Ensure that the address limit is intact and no locks are held */
+ addr_limit_user_check();
+ kmap_assert_nomap();
+ lockdep_assert_irqs_disabled();
+ lockdep_sys_exit();
+}
+
+/*
+ * If SYSCALL_EMU is set, then the only reason to report is when
+ * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP). This syscall
+ * instruction has been already reported in syscall_enter_from_user_mode().
+ */
+static inline bool report_single_step(unsigned long work)
+{
+ if (work & SYSCALL_WORK_SYSCALL_EMU)
+ return false;
+
+ return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
+}
+
+static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
+{
+ bool step;
+
+ /*
+ * If the syscall was rolled back due to syscall user dispatching,
+ * then the tracers below are not invoked for the same reason as
+ * the entry side was not invoked in syscall_trace_enter(): The ABI
+ * of these syscalls is unknown.
+ */
+ if (work & SYSCALL_WORK_SYSCALL_USER_DISPATCH) {
+ if (unlikely(current->syscall_dispatch.on_dispatch)) {
+ current->syscall_dispatch.on_dispatch = false;
+ return;
+ }
+ }
+
+ audit_syscall_exit(regs);
+
+ if (work & SYSCALL_WORK_SYSCALL_TRACEPOINT)
+ trace_sys_exit(regs, syscall_get_return_value(current, regs));
+
+ step = report_single_step(work);
+ if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
+ ptrace_report_syscall_exit(regs, step);
+}
+
+/*
+ * Syscall specific exit to user mode preparation. Runs with interrupts
+ * enabled.
+ */
+static __always_inline void syscall_exit_to_user_mode_prepare(struct pt_regs *regs)
+{
+ unsigned long work = READ_ONCE(current_thread_info()->syscall_work);
+ unsigned long nr = syscall_get_nr(current, regs);
+
+ CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
+
+ if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
+ if (WARN(irqs_disabled(), "syscall %lu left IRQs disabled", nr))
+ local_irq_enable();
+ }
+
+ rseq_syscall(regs);
+
+ /*
+ * Do one-time syscall specific work. If these work items are
+ * enabled, we want to run them exactly once per syscall exit with
+ * interrupts enabled.
+ */
+ if (unlikely(work & SYSCALL_WORK_EXIT))
+ syscall_exit_work(regs, work);
+}
+
+static __always_inline void __syscall_exit_to_user_mode_work(struct pt_regs *regs)
+{
+ syscall_exit_to_user_mode_prepare(regs);
+ local_irq_disable_exit_to_user();
+ exit_to_user_mode_prepare(regs);
+}
+
+static __always_inline void syscall_exit_to_user_mode_work(struct pt_regs *regs)
+{
+ __syscall_exit_to_user_mode_work(regs);
+}
+
+/* See comment for exit_to_user_mode() in entry-common.h */
+static __always_inline void __exit_to_user_mode(void)
+{
+ instrumentation_begin();
+ trace_hardirqs_on_prepare();
+ lockdep_hardirqs_on_prepare();
+ instrumentation_end();
+
+ user_enter_irqoff();
+ arch_exit_to_user_mode();
+ lockdep_hardirqs_on(CALLER_ADDR0);
+}
+
+
+static __always_inline void irqentry_exit_to_user_mode(struct pt_regs *regs)
+{
+ instrumentation_begin();
+ exit_to_user_mode_prepare(regs);
+ instrumentation_end();
+ __exit_to_user_mode();
+}
+
+
#endif
diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index be61332c66b5..66af971c3fe4 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -123,19 +123,6 @@ noinstr void syscall_enter_from_user_mode_prepare(struct pt_regs *regs)
instrumentation_end();
}

-/* See comment for exit_to_user_mode() in entry-common.h */
-static __always_inline void __exit_to_user_mode(void)
-{
- instrumentation_begin();
- trace_hardirqs_on_prepare();
- lockdep_hardirqs_on_prepare();
- instrumentation_end();
-
- user_enter_irqoff();
- arch_exit_to_user_mode();
- lockdep_hardirqs_on(CALLER_ADDR0);
-}
-
void noinstr exit_to_user_mode(void)
{
__exit_to_user_mode();
@@ -144,153 +131,6 @@ void noinstr exit_to_user_mode(void)
/* Workaround to allow gradual conversion of architecture code */
void __weak arch_do_signal_or_restart(struct pt_regs *regs) { }

-static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
- unsigned long ti_work)
-{
- /*
- * Before returning to user space ensure that all pending work
- * items have been completed.
- */
- while (ti_work & EXIT_TO_USER_MODE_WORK) {
-
- local_irq_enable_exit_to_user(ti_work);
-
- if (ti_work & _TIF_NEED_RESCHED)
- schedule();
-
- if (ti_work & _TIF_UPROBE)
- uprobe_notify_resume(regs);
-
- if (ti_work & _TIF_PATCH_PENDING)
- klp_update_patch_state(current);
-
- if (ti_work & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
- arch_do_signal_or_restart(regs);
-
- if (ti_work & _TIF_NOTIFY_RESUME)
- resume_user_mode_work(regs);
-
- /* Architecture specific TIF work */
- arch_exit_to_user_mode_work(regs, ti_work);
-
- /*
- * Disable interrupts and reevaluate the work flags as they
- * might have changed while interrupts and preemption was
- * enabled above.
- */
- local_irq_disable_exit_to_user();
-
- /* Check if any of the above work has queued a deferred wakeup */
- tick_nohz_user_enter_prepare();
-
- ti_work = read_thread_flags();
- }
-
- /* Return the latest work state for arch_exit_to_user_mode() */
- return ti_work;
-}
-
-static void exit_to_user_mode_prepare(struct pt_regs *regs)
-{
- unsigned long ti_work;
-
- lockdep_assert_irqs_disabled();
-
- /* Flush pending rcuog wakeup before the last need_resched() check */
- tick_nohz_user_enter_prepare();
-
- ti_work = read_thread_flags();
- if (unlikely(ti_work & EXIT_TO_USER_MODE_WORK))
- ti_work = exit_to_user_mode_loop(regs, ti_work);
-
- arch_exit_to_user_mode_prepare(regs, ti_work);
-
- /* Ensure that the address limit is intact and no locks are held */
- addr_limit_user_check();
- kmap_assert_nomap();
- lockdep_assert_irqs_disabled();
- lockdep_sys_exit();
-}
-
-/*
- * If SYSCALL_EMU is set, then the only reason to report is when
- * SINGLESTEP is set (i.e. PTRACE_SYSEMU_SINGLESTEP). This syscall
- * instruction has been already reported in syscall_enter_from_user_mode().
- */
-static inline bool report_single_step(unsigned long work)
-{
- if (work & SYSCALL_WORK_SYSCALL_EMU)
- return false;
-
- return work & SYSCALL_WORK_SYSCALL_EXIT_TRAP;
-}
-
-static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
-{
- bool step;
-
- /*
- * If the syscall was rolled back due to syscall user dispatching,
- * then the tracers below are not invoked for the same reason as
- * the entry side was not invoked in syscall_trace_enter(): The ABI
- * of these syscalls is unknown.
- */
- if (work & SYSCALL_WORK_SYSCALL_USER_DISPATCH) {
- if (unlikely(current->syscall_dispatch.on_dispatch)) {
- current->syscall_dispatch.on_dispatch = false;
- return;
- }
- }
-
- audit_syscall_exit(regs);
-
- if (work & SYSCALL_WORK_SYSCALL_TRACEPOINT)
- trace_sys_exit(regs, syscall_get_return_value(current, regs));
-
- step = report_single_step(work);
- if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
- ptrace_report_syscall_exit(regs, step);
-}
-
-/*
- * Syscall specific exit to user mode preparation. Runs with interrupts
- * enabled.
- */
-static void syscall_exit_to_user_mode_prepare(struct pt_regs *regs)
-{
- unsigned long work = READ_ONCE(current_thread_info()->syscall_work);
- unsigned long nr = syscall_get_nr(current, regs);
-
- CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
-
- if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
- if (WARN(irqs_disabled(), "syscall %lu left IRQs disabled", nr))
- local_irq_enable();
- }
-
- rseq_syscall(regs);
-
- /*
- * Do one-time syscall specific work. If these work items are
- * enabled, we want to run them exactly once per syscall exit with
- * interrupts enabled.
- */
- if (unlikely(work & SYSCALL_WORK_EXIT))
- syscall_exit_work(regs, work);
-}
-
-static __always_inline void __syscall_exit_to_user_mode_work(struct pt_regs *regs)
-{
- syscall_exit_to_user_mode_prepare(regs);
- local_irq_disable_exit_to_user();
- exit_to_user_mode_prepare(regs);
-}
-
-void syscall_exit_to_user_mode_work(struct pt_regs *regs)
-{
- __syscall_exit_to_user_mode_work(regs);
-}
-
__visible noinstr void syscall_exit_to_user_mode(struct pt_regs *regs)
{
instrumentation_begin();
@@ -304,14 +144,6 @@ noinstr void irqentry_enter_from_user_mode(struct pt_regs *regs)
__enter_from_user_mode(regs);
}

-noinstr void irqentry_exit_to_user_mode(struct pt_regs *regs)
-{
- instrumentation_begin();
- exit_to_user_mode_prepare(regs);
- instrumentation_end();
- __exit_to_user_mode();
-}
-
noinstr irqentry_state_t irqentry_enter(struct pt_regs *regs)
{
irqentry_state_t ret = {
--
2.39.2

2023-05-16 16:44:56

by Peter Zijlstra

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

On Tue, May 16, 2023 at 03:38:09PM +0200, Sven Schnelle wrote:
> @@ -465,4 +470,175 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
> */
> void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
>
> +static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
> + unsigned long ti_work)

Should these things not grow __always_inline/inline when moved into a header?

> +{

> +}
> +
> +
> +static void exit_to_user_mode_prepare(struct pt_regs *regs)

idem

> +{

> +}

> +static void syscall_exit_work(struct pt_regs *regs, unsigned long work)

and more..

> +{

> +}
> +

2023-05-16 17:10:56

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

Hi Sven,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/core/entry]
[also build test WARNING on linus/master v6.4-rc2 next-20230516]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/entry-move-the-exit-path-to-header-files/20230516-230146
base: tip/core/entry
patch link: https://lore.kernel.org/r/20230516133810.171487-2-svens%40linux.ibm.com
patch subject: [PATCH 1/2] entry: move the exit path to header files
config: loongarch-allyesconfig
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/30656e2ff97c145a735ce61be030ba945ec99ace
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sven-Schnelle/entry-move-the-exit-path-to-header-files/20230516-230146
git checkout 30656e2ff97c145a735ce61be030ba945ec99ace
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=loongarch SHELL=/bin/bash kernel/entry/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

In file included from kernel/entry/common.c:4:
include/linux/entry-common.h: In function 'syscall_exit_work':
include/linux/entry-common.h:572:9: error: implicit declaration of function 'audit_syscall_exit' [-Werror=implicit-function-declaration]
572 | audit_syscall_exit(regs);
| ^~~~~~~~~~~~~~~~~~
In file included from kernel/entry/common.c:10:
include/linux/audit.h: At top level:
>> include/linux/audit.h:350:20: warning: conflicting types for 'audit_syscall_exit'; have 'void(void *)'
350 | static inline void audit_syscall_exit(void *pt_regs)
| ^~~~~~~~~~~~~~~~~~
include/linux/audit.h:350:20: error: static declaration of 'audit_syscall_exit' follows non-static declaration
include/linux/entry-common.h:572:9: note: previous implicit declaration of 'audit_syscall_exit' with type 'void(void *)'
572 | audit_syscall_exit(regs);
| ^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +350 include/linux/audit.h

cdfb6b341f0f24 Richard Guy Briggs 2018-05-12 317
36734810488e61 Yaowei Bai 2015-11-04 318 static inline bool audit_dummy_context(void)
d51374adf5f2f8 Al Viro 2006-08-03 319 {
cdfb6b341f0f24 Richard Guy Briggs 2018-05-12 320 void *p = audit_context();
d51374adf5f2f8 Al Viro 2006-08-03 321 return !p || *(int *)p;
d51374adf5f2f8 Al Viro 2006-08-03 322 }
a4ff8dba7d8ce5 Eric Paris 2012-01-03 323 static inline void audit_free(struct task_struct *task)
a4ff8dba7d8ce5 Eric Paris 2012-01-03 324 {
a4ff8dba7d8ce5 Eric Paris 2012-01-03 325 if (unlikely(task->audit_context))
a4ff8dba7d8ce5 Eric Paris 2012-01-03 326 __audit_free(task);
a4ff8dba7d8ce5 Eric Paris 2012-01-03 327 }
5bd2182d58e9d9 Paul Moore 2021-02-16 328 static inline void audit_uring_entry(u8 op)
5bd2182d58e9d9 Paul Moore 2021-02-16 329 {
5bd2182d58e9d9 Paul Moore 2021-02-16 330 /*
5bd2182d58e9d9 Paul Moore 2021-02-16 331 * We intentionally check audit_context() before audit_enabled as most
5bd2182d58e9d9 Paul Moore 2021-02-16 332 * Linux systems (as of ~2021) rely on systemd which forces audit to
5bd2182d58e9d9 Paul Moore 2021-02-16 333 * be enabled regardless of the user's audit configuration.
5bd2182d58e9d9 Paul Moore 2021-02-16 334 */
5bd2182d58e9d9 Paul Moore 2021-02-16 335 if (unlikely(audit_context() && audit_enabled))
5bd2182d58e9d9 Paul Moore 2021-02-16 336 __audit_uring_entry(op);
5bd2182d58e9d9 Paul Moore 2021-02-16 337 }
5bd2182d58e9d9 Paul Moore 2021-02-16 338 static inline void audit_uring_exit(int success, long code)
5bd2182d58e9d9 Paul Moore 2021-02-16 339 {
69e9cd66ae1392 Julian Orth 2022-05-17 340 if (unlikely(audit_context()))
5bd2182d58e9d9 Paul Moore 2021-02-16 341 __audit_uring_exit(success, code);
5bd2182d58e9d9 Paul Moore 2021-02-16 342 }
91397401bb5072 Eric Paris 2014-03-11 343 static inline void audit_syscall_entry(int major, unsigned long a0,
b05d8447e78216 Eric Paris 2012-01-03 344 unsigned long a1, unsigned long a2,
b05d8447e78216 Eric Paris 2012-01-03 345 unsigned long a3)
b05d8447e78216 Eric Paris 2012-01-03 346 {
cdfb6b341f0f24 Richard Guy Briggs 2018-05-12 347 if (unlikely(audit_context()))
b4f0d3755c5e9c Richard Guy Briggs 2014-03-04 348 __audit_syscall_entry(major, a0, a1, a2, a3);
b05d8447e78216 Eric Paris 2012-01-03 349 }
d7e7528bcd456f Eric Paris 2012-01-03 @350 static inline void audit_syscall_exit(void *pt_regs)
d7e7528bcd456f Eric Paris 2012-01-03 351 {
cdfb6b341f0f24 Richard Guy Briggs 2018-05-12 352 if (unlikely(audit_context())) {
d7e7528bcd456f Eric Paris 2012-01-03 353 int success = is_syscall_success(pt_regs);
06bdadd7634551 AKASHI Takahiro 2014-01-13 354 long return_code = regs_return_value(pt_regs);
d7e7528bcd456f Eric Paris 2012-01-03 355
d7e7528bcd456f Eric Paris 2012-01-03 356 __audit_syscall_exit(success, return_code);
d7e7528bcd456f Eric Paris 2012-01-03 357 }
d7e7528bcd456f Eric Paris 2012-01-03 358 }
7ac86265dc8f66 Jeff Layton 2012-10-10 359 static inline struct filename *audit_reusename(const __user char *name)
7ac86265dc8f66 Jeff Layton 2012-10-10 360 {
7ac86265dc8f66 Jeff Layton 2012-10-10 361 if (unlikely(!audit_dummy_context()))
7ac86265dc8f66 Jeff Layton 2012-10-10 362 return __audit_reusename(name);
7ac86265dc8f66 Jeff Layton 2012-10-10 363 return NULL;
7ac86265dc8f66 Jeff Layton 2012-10-10 364 }
91a27b2a756784 Jeff Layton 2012-10-10 365 static inline void audit_getname(struct filename *name)
d8945bb51a2bb6 Al Viro 2006-05-18 366 {
5ac3a9c26c1cc4 Al Viro 2006-07-16 367 if (unlikely(!audit_dummy_context()))
d8945bb51a2bb6 Al Viro 2006-05-18 368 __audit_getname(name);
d8945bb51a2bb6 Al Viro 2006-05-18 369 }
79f6530cb59e2a Jeff Layton 2013-07-08 370 static inline void audit_inode(struct filename *name,
79f6530cb59e2a Jeff Layton 2013-07-08 371 const struct dentry *dentry,
c9b07eab0c8760 Al Viro 2019-07-14 372 unsigned int aflags) {
c9b07eab0c8760 Al Viro 2019-07-14 373 if (unlikely(!audit_dummy_context()))
57d4657716aca8 Richard Guy Briggs 2019-01-23 374 __audit_inode(name, dentry, aflags);
79f6530cb59e2a Jeff Layton 2013-07-08 375 }
9f45f5bf302daa Al Viro 2014-10-31 376 static inline void audit_file(struct file *file)
9f45f5bf302daa Al Viro 2014-10-31 377 {
9f45f5bf302daa Al Viro 2014-10-31 378 if (unlikely(!audit_dummy_context()))
9f45f5bf302daa Al Viro 2014-10-31 379 __audit_file(file);
9f45f5bf302daa Al Viro 2014-10-31 380 }
79f6530cb59e2a Jeff Layton 2013-07-08 381 static inline void audit_inode_parent_hidden(struct filename *name,
79f6530cb59e2a Jeff Layton 2013-07-08 382 const struct dentry *dentry)
79f6530cb59e2a Jeff Layton 2013-07-08 383 {
5ac3a9c26c1cc4 Al Viro 2006-07-16 384 if (unlikely(!audit_dummy_context()))
79f6530cb59e2a Jeff Layton 2013-07-08 385 __audit_inode(name, dentry,
79f6530cb59e2a Jeff Layton 2013-07-08 386 AUDIT_INODE_PARENT | AUDIT_INODE_HIDDEN);
73241ccca0f778 Amy Griffis 2005-11-03 387 }
d6335d77a7622a Andreas Gruenbacher 2015-12-24 388 static inline void audit_inode_child(struct inode *parent,
4fa6b5ecbf092c Jeff Layton 2012-10-10 389 const struct dentry *dentry,
4fa6b5ecbf092c Jeff Layton 2012-10-10 390 const unsigned char type) {
5ac3a9c26c1cc4 Al Viro 2006-07-16 391 if (unlikely(!audit_dummy_context()))
4fa6b5ecbf092c Jeff Layton 2012-10-10 392 __audit_inode_child(parent, dentry, type);
73241ccca0f778 Amy Griffis 2005-11-03 393 }
0a4ff8c2598b72 Steve Grubb 2007-04-19 394 void audit_core_dumps(long signr);
^1da177e4c3f41 Linus Torvalds 2005-04-16 395

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Attachments:

(No filename) (9.23 kB)
config (342.09 kB)
Download all attachments

2023-05-16 18:42:59

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

Attachments:

(No filename) (3.43 kB)
config (141.12 kB)
Download all attachments

2023-05-16 19:23:04

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

Hi Sven,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/core/entry]
[also build test ERROR on linus/master v6.4-rc2 next-20230516]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/entry-move-the-exit-path-to-header-files/20230516-230146
base: tip/core/entry
patch link: https://lore.kernel.org/r/20230516133810.171487-2-svens%40linux.ibm.com
patch subject: [PATCH 1/2] entry: move the exit path to header files
config: i386-buildonly-randconfig-r001-20230515
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/30656e2ff97c145a735ce61be030ba945ec99ace
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Sven-Schnelle/entry-move-the-exit-path-to-header-files/20230516-230146
git checkout 30656e2ff97c145a735ce61be030ba945ec99ace
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash arch/x86/kernel/ kernel/entry/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from arch/x86/kernel/signal.c:28:
>> include/linux/entry-common.h:572:2: error: implicit declaration of function 'audit_syscall_exit' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
audit_syscall_exit(regs);
^
1 error generated.
--
In file included from arch/x86/kernel/doublefault_32.c:11:
In file included from arch/x86/include/asm/traps.h:9:
In file included from arch/x86/include/asm/idtentry.h:11:
>> include/linux/entry-common.h:572:2: error: implicit declaration of function 'audit_syscall_exit' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
audit_syscall_exit(regs);
^
arch/x86/kernel/doublefault_32.c:23:36: warning: no previous prototype for function 'doublefault_shim' [-Wmissing-prototypes]
asmlinkage noinstr void __noreturn doublefault_shim(void)
^
arch/x86/kernel/doublefault_32.c:23:20: note: declare 'static' if the function is not intended to be used outside of this translation unit
asmlinkage noinstr void __noreturn doublefault_shim(void)
^
static
arch/x86/kernel/doublefault_32.c:114:6: warning: no previous prototype for function 'doublefault_init_cpu_tss' [-Wmissing-prototypes]
void doublefault_init_cpu_tss(void)
^
arch/x86/kernel/doublefault_32.c:114:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void doublefault_init_cpu_tss(void)
^
static
2 warnings and 1 error generated.
--
In file included from kernel/entry/common.c:4:
>> include/linux/entry-common.h:572:2: error: implicit declaration of function 'audit_syscall_exit' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
audit_syscall_exit(regs);
^
In file included from kernel/entry/common.c:10:
include/linux/audit.h:591:20: error: static declaration of 'audit_syscall_exit' follows non-static declaration
static inline void audit_syscall_exit(void *pt_regs)
^
include/linux/entry-common.h:572:2: note: previous implicit declaration is here
audit_syscall_exit(regs);
^
2 errors generated.
--
In file included from arch/x86/kernel/fpu/core.c:14:
In file included from arch/x86/include/asm/traps.h:9:
In file included from arch/x86/include/asm/idtentry.h:11:
>> include/linux/entry-common.h:572:2: error: implicit declaration of function 'audit_syscall_exit' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
audit_syscall_exit(regs);
^
In file included from arch/x86/kernel/fpu/core.c:29:
In file included from arch/x86/include/asm/trace/fpu.h:99:
>> include/trace/define_trace.h:95:10: fatal error: 'asm/trace//syscalls.h' file not found
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/trace/define_trace.h:90:32: note: expanded from macro 'TRACE_INCLUDE'
# define TRACE_INCLUDE(system) __TRACE_INCLUDE(system)
^~~~~~~~~~~~~~~~~~~~~~~
include/trace/define_trace.h:87:34: note: expanded from macro '__TRACE_INCLUDE'
# define __TRACE_INCLUDE(system) __stringify(TRACE_INCLUDE_PATH/system.h)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/stringify.h:10:27: note: expanded from macro '__stringify'
#define __stringify(x...) __stringify_1(x)
^~~~~~~~~~~~~~~~
include/linux/stringify.h:9:29: note: expanded from macro '__stringify_1'
#define __stringify_1(x...) #x
^~
<scratch space>:264:1: note: expanded from here
"asm/trace//syscalls.h"
^~~~~~~~~~~~~~~~~~~~~~~
2 errors generated.

vim +/audit_syscall_exit +572 include/linux/entry-common.h

554
555 static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
556 {
557 bool step;
558
559 /*
560 * If the syscall was rolled back due to syscall user dispatching,
561 * then the tracers below are not invoked for the same reason as
562 * the entry side was not invoked in syscall_trace_enter(): The ABI
563 * of these syscalls is unknown.
564 */
565 if (work & SYSCALL_WORK_SYSCALL_USER_DISPATCH) {
566 if (unlikely(current->syscall_dispatch.on_dispatch)) {
567 current->syscall_dispatch.on_dispatch = false;
568 return;
569 }
570 }
571
> 572 audit_syscall_exit(regs);
573
574 if (work & SYSCALL_WORK_SYSCALL_TRACEPOINT)
575 trace_sys_exit(regs, syscall_get_return_value(current, regs));
576
577 step = report_single_step(work);
578 if (step || work & SYSCALL_WORK_SYSCALL_TRACE)
579 ptrace_report_syscall_exit(regs, step);
580 }
581

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Attachments:

(No filename) (6.84 kB)
config (199.28 kB)
Download all attachments

2023-05-16 20:28:48

by Eric W. Biederman

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

Peter Zijlstra <[email protected]> writes:

> On Tue, May 16, 2023 at 03:38:09PM +0200, Sven Schnelle wrote:
>> @@ -465,4 +470,175 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
>> */
>> void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
>>
>> +static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
>> + unsigned long ti_work)
>
> Should these things not grow __always_inline/inline when moved into a header?

Is that actually what is desired?

This is a header file that should only be included once isn't it?

>> +{
>
>> +}
>> +
>> +
>> +static void exit_to_user_mode_prepare(struct pt_regs *regs)
>
> idem
>
>> +{
>
>> +}
>
>> +static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>
> and more..
>
>> +{
>
>> +}
>> +

Perhaps it would make most sense just to change the idiom to include
the .c file. That would give the optimizer every opportunity to inline
static functions, while strongly suggesting this file should be included
only once.

Is this maybe a s390 specific problem because the s390 has something
like ancient calling conventions that are not as efficient as they
should be.

Eric

2023-05-17 05:59:50

by Sven Schnelle

[permalink] [raw]

Subject: Re: [PATCH 1/2] entry: move the exit path to header files

Peter Zijlstra <[email protected]> writes:

> On Tue, May 16, 2023 at 03:38:09PM +0200, Sven Schnelle wrote:
>> @@ -465,4 +470,175 @@ irqentry_state_t noinstr irqentry_nmi_enter(struct pt_regs *regs);
>> */
>> void noinstr irqentry_nmi_exit(struct pt_regs *regs, irqentry_state_t irq_state);
>>
>> +static unsigned long exit_to_user_mode_loop(struct pt_regs *regs,
>> + unsigned long ti_work)
>
> Should these things not grow __always_inline/inline when moved into a header?

Yes, indeed. I missed that while doing a quick move of the functions for
testing. I'll fix that when doing a proper patch set for submission.

>> +{
>
>> +}
>> +
>> +
>> +static void exit_to_user_mode_prepare(struct pt_regs *regs)
>
> idem
>
>> +{
>
>> +}
>
>> +static void syscall_exit_work(struct pt_regs *regs, unsigned long work)
>
> and more..
>
>> +{
>
>> +}
>> +