[PATCH 1/2] ftrace: move the CALLER_ADDRx macros into its own header

[PATCH 1/2] ftrace: move the CALLER_ADDRx macros into its own header

[Sebastian Andrzej Siewior]

This patch moves the CALLER_ADDRx macros into their own header so it is
not required to include ftrace.h just for them.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/ftrace.h        | 25 +------------------------
 include/linux/ftrace_caller.h | 28 ++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 24 deletions(-)
 create mode 100644 include/linux/ftrace_caller.h

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 81de7123959d..83193c717cb9 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -16,8 +16,7 @@
 #include <linux/types.h>
 #include <linux/init.h>
 #include <linux/fs.h>
-
-#include <asm/ftrace.h>
+#include <linux/ftrace_caller.h>
 
 /*
  * If the arch supports passing the variable contents of
@@ -689,28 +688,6 @@ static inline void __ftrace_enabled_restore(int enabled)
 #endif
 }
 
-/* All archs should have this, but we define it for consistency */
-#ifndef ftrace_return_address0
-# define ftrace_return_address0 __builtin_return_address(0)
-#endif
-
-/* Archs may use other ways for ADDR1 and beyond */
-#ifndef ftrace_return_address
-# ifdef CONFIG_FRAME_POINTER
-#  define ftrace_return_address(n) __builtin_return_address(n)
-# else
-#  define ftrace_return_address(n) 0UL
-# endif
-#endif
-
-#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
-#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
-#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
-#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
-#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
-#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
-#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))
-
 #ifdef CONFIG_IRQSOFF_TRACER
   extern void time_hardirqs_on(unsigned long a0, unsigned long a1);
   extern void time_hardirqs_off(unsigned long a0, unsigned long a1);
diff --git a/include/linux/ftrace_caller.h b/include/linux/ftrace_caller.h
new file mode 100644
index 000000000000..887a7c220cd6
--- /dev/null
+++ b/include/linux/ftrace_caller.h
@@ -0,0 +1,28 @@
+#ifndef _LINUX_FTRACE_CALLER_H
+#define _LINUX_FTRACE_CALLER_H
+
+#include <asm/ftrace.h>
+
+/* All archs should have this, but we define it for consistency */
+#ifndef ftrace_return_address0
+# define ftrace_return_address0		__builtin_return_address(0)
+#endif
+
+/* Archs may use other ways for ADDR1 and beyond */
+#ifndef ftrace_return_address
+# ifdef CONFIG_FRAME_POINTER
+#  define ftrace_return_address(n)	__builtin_return_address(n)
+# else
+#  define ftrace_return_address(n)	0UL
+# endif
+#endif
+
+#define CALLER_ADDR0 ((unsigned long)ftrace_return_address0)
+#define CALLER_ADDR1 ((unsigned long)ftrace_return_address(1))
+#define CALLER_ADDR2 ((unsigned long)ftrace_return_address(2))
+#define CALLER_ADDR3 ((unsigned long)ftrace_return_address(3))
+#define CALLER_ADDR4 ((unsigned long)ftrace_return_address(4))
+#define CALLER_ADDR5 ((unsigned long)ftrace_return_address(5))
+#define CALLER_ADDR6 ((unsigned long)ftrace_return_address(6))
+
+#endif
-- 
2.7.0

[PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[Sebastian Andrzej Siewior]

The preempt_disable() invokes preempt_count_add() which saves the caller
in ->preempt_disable_ip. It uses CALLER_ADDR1 which does not look for
its caller but for the parent of the caller. Which means we get the correct
caller for something like spin_lock() unless the architectures inlines
those invocations. It is always wrong for preempt_disable() or
local_bh_disable().

This patch makes the function get_parent_ip() which tries
CALLER_ADDR0,1,2 if the former is a locking function.
This seems to record the preempt_disable() caller properly for
preempt_disable() itself as well as for get_cpu_var() or
local_bh_disable().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/linux/sched.h | 13 ++++++++++++-
 kernel/sched/core.c   | 14 ++------------
 kernel/softirq.c      |  4 ++--
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index a10494a94cc3..cd21fd41ba2a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -59,6 +59,7 @@ struct sched_param {
 #include <linux/gfp.h>
 #include <linux/magic.h>
 #include <linux/cgroup-defs.h>
+#include <linux/ftrace_caller.h>
 
 #include <asm/processor.h>
 
@@ -182,7 +183,17 @@ extern void update_cpu_load_nohz(int active);
 static inline void update_cpu_load_nohz(int active) { }
 #endif
 
-extern unsigned long get_parent_ip(unsigned long addr);
+static inline unsigned long get_parent_ip(void)
+{
+	unsigned long addr = CALLER_ADDR0;
+
+	if (!in_lock_functions(addr))
+		return addr;
+	addr = CALLER_ADDR1;
+	if (!in_lock_functions(addr))
+		return addr;
+	return CALLER_ADDR2;
+}
 
 extern void dump_cpu_task(int cpu);
 
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9503d590e5ef..12c2527f5957 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3010,16 +3010,6 @@ u64 scheduler_tick_max_deferment(void)
 }
 #endif
 
-notrace unsigned long get_parent_ip(unsigned long addr)
-{
-	if (in_lock_functions(addr)) {
-		addr = CALLER_ADDR2;
-		if (in_lock_functions(addr))
-			addr = CALLER_ADDR3;
-	}
-	return addr;
-}
-
 #if defined(CONFIG_PREEMPT) && (defined(CONFIG_DEBUG_PREEMPT) || \
 				defined(CONFIG_PREEMPT_TRACER))
 
@@ -3041,7 +3031,7 @@ void preempt_count_add(int val)
 				PREEMPT_MASK - 10);
 #endif
 	if (preempt_count() == val) {
-		unsigned long ip = get_parent_ip(CALLER_ADDR1);
+		unsigned long ip = get_parent_ip();
 #ifdef CONFIG_DEBUG_PREEMPT
 		current->preempt_disable_ip = ip;
 #endif
@@ -3068,7 +3058,7 @@ void preempt_count_sub(int val)
 #endif
 
 	if (preempt_count() == val)
-		trace_preempt_on(CALLER_ADDR0, get_parent_ip(CALLER_ADDR1));
+		trace_preempt_on(CALLER_ADDR0, get_parent_ip());
 	__preempt_count_sub(val);
 }
 EXPORT_SYMBOL(preempt_count_sub);
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 479e4436f787..ec71033a87a2 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -116,9 +116,9 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 
 	if (preempt_count() == cnt) {
 #ifdef CONFIG_DEBUG_PREEMPT
-		current->preempt_disable_ip = get_parent_ip(CALLER_ADDR1);
+		current->preempt_disable_ip = get_parent_ip();
 #endif
-		trace_preempt_off(CALLER_ADDR0, get_parent_ip(CALLER_ADDR1));
+		trace_preempt_off(CALLER_ADDR0, get_parent_ip());
 	}
 }
 EXPORT_SYMBOL(__local_bh_disable_ip);
-- 
2.7.0

Re: [PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[Sebastian Andrzej Siewior]

Before:
 preempt_test(1) kernel_init_freeable+0x1bd/0x239
 preempt_test(2) preempt_test+0x75/0x15c
 preempt_test(3) preempt_test+0xaa/0x15c
 preempt_test(4) kernel_init_freeable+0x1bd/0x239
 preempt_test(5) kernel_init_freeable+0x1bd/0x239

After:
 preempt_test(1) preempt_test+0x2f/0x15c
 preempt_test(2) preempt_test+0x75/0x15c
 preempt_test(3) preempt_test+0xaa/0x15c
 preempt_test(4) preempt_test+0xd7/0x15c
 preempt_test(5) preempt_test+0x121/0x15c

diff --git a/init/main.c b/init/main.c
index 9e64d7097f1a..da4a4b10964a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -974,6 +974,35 @@ static int __ref kernel_init(void *unused)
 	      "See Linux Documentation/init.txt for guidance.");
 }
 
+static DEFINE_PER_CPU(unsigned long, pcpu_rtest_var);
+
+static noinline void preempt_test(void)
+{
+	spinlock_t sl;
+
+	spin_lock_init(&sl);
+
+	preempt_disable();
+	pr_err("%s(1) %pF\n", __func__, current->preempt_disable_ip);
+	preempt_enable();
+
+	spin_lock(&sl);
+	pr_err("%s(2) %pF\n", __func__, current->preempt_disable_ip);
+	spin_unlock(&sl);
+
+	spin_lock_bh(&sl);
+	pr_err("%s(3) %pF\n", __func__, current->preempt_disable_ip);
+	spin_unlock_bh(&sl);
+
+	get_cpu_var(pcpu_rtest_var);
+	pr_err("%s(4) %pF\n", __func__, current->preempt_disable_ip);
+	put_cpu_var(pcpu_rtest_var);
+
+	local_bh_disable();
+	pr_err("%s(5) %pF\n", __func__, current->preempt_disable_ip);
+	local_bh_enable();
+}
+
 static noinline void __init kernel_init_freeable(void)
 {
 	/*
@@ -1006,6 +1035,7 @@ static noinline void __init kernel_init_freeable(void)
 	page_alloc_init_late();
 
 	do_basic_setup();
+	preempt_test();
 
 	/* Open the /dev/console on the rootfs, this should never fail */
 	if (sys_open((const char __user *) "/dev/console", O_RDWR, 0) < 0)

Sebastian

Re: [PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4242 bytes --]

Hi Sebastian,

[auto build test ERROR on tip/sched/core]
[also build test ERROR on v4.5-rc3 next-20160211]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Andrzej-Siewior/ftrace-move-the-CALLER_ADDRx-macros-into-its-own-header/20160212-072259
config: s390-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=s390 

All errors (new ones prefixed by >>):

   In file included from include/linux/ftrace_caller.h:4:0,
                    from include/linux/sched.h:62,
                    from include/linux/kvm_host.h:15,
                    from arch/s390/kernel/asm-offsets.c:10:
   arch/s390/include/asm/ftrace.h: In function 'ftrace_generate_call_insn':
>> arch/s390/include/asm/ftrace.h:77:2: error: implicit declaration of function 'is_module_addr' [-Werror=implicit-function-declaration]
     target = is_module_addr((void *) ip) ? ftrace_plt : FTRACE_ADDR;
     ^
   In file included from include/linux/mm.h:67:0,
                    from include/linux/kvm_host.h:17,
                    from arch/s390/kernel/asm-offsets.c:10:
   arch/s390/include/asm/pgtable.h: At top level:
>> arch/s390/include/asm/pgtable.h:120:19: error: static declaration of 'is_module_addr' follows non-static declaration
    static inline int is_module_addr(void *addr)
                      ^
   In file included from include/linux/ftrace_caller.h:4:0,
                    from include/linux/sched.h:62,
                    from include/linux/kvm_host.h:15,
                    from arch/s390/kernel/asm-offsets.c:10:
   arch/s390/include/asm/ftrace.h:77:11: note: previous implicit declaration of 'is_module_addr' was here
     target = is_module_addr((void *) ip) ? ftrace_plt : FTRACE_ADDR;
              ^
   cc1: some warnings being treated as errors
   make[2]: *** [arch/s390/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +/is_module_addr +77 arch/s390/include/asm/ftrace.h

e6d60b36 Heiko Carstens 2015-01-09  61  		return 1;
e6d60b36 Heiko Carstens 2015-01-09  62  #else
c933146a Heiko Carstens 2014-10-15  63  	if (insn->disp == MCOUNT_INSN_SIZE / 2)
c933146a Heiko Carstens 2014-10-15  64  		return 1;
c933146a Heiko Carstens 2014-10-15  65  #endif
e6d60b36 Heiko Carstens 2015-01-09  66  #endif
c933146a Heiko Carstens 2014-10-15  67  	return 0;
c933146a Heiko Carstens 2014-10-15  68  }
c933146a Heiko Carstens 2014-10-15  69  
c933146a Heiko Carstens 2014-10-15  70  static inline void ftrace_generate_call_insn(struct ftrace_insn *insn,
c933146a Heiko Carstens 2014-10-15  71  					     unsigned long ip)
c933146a Heiko Carstens 2014-10-15  72  {
c933146a Heiko Carstens 2014-10-15  73  #ifdef CONFIG_FUNCTION_TRACER
c933146a Heiko Carstens 2014-10-15  74  	unsigned long target;
10dec7db Heiko Carstens 2014-08-15  75  
c933146a Heiko Carstens 2014-10-15  76  	/* brasl r0,ftrace_caller */
c933146a Heiko Carstens 2014-10-15 @77  	target = is_module_addr((void *) ip) ? ftrace_plt : FTRACE_ADDR;
c933146a Heiko Carstens 2014-10-15  78  	insn->opc = 0xc005;
c933146a Heiko Carstens 2014-10-15  79  	insn->disp = (target - ip) / 2;
c933146a Heiko Carstens 2014-10-15  80  #endif
c933146a Heiko Carstens 2014-10-15  81  }
c933146a Heiko Carstens 2014-10-15  82  
c933146a Heiko Carstens 2014-10-15  83  #endif /* __ASSEMBLY__ */
5d360a75 Heiko Carstens 2008-12-25  84  #endif /* _ASM_S390_FTRACE_H */

:::::: The code at line 77 was first introduced by commit
:::::: c933146a5e41e42ea3eb4f34fa02e201da3f068e s390/ftrace,kprobes: allow to patch first instruction

:::::: TO: Heiko Carstens <heiko.carstens@de.ibm.com>
:::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 40125 bytes --]

Re: [PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[Steven Rostedt]

On Fri, 12 Feb 2016 08:27:01 +0800
kbuild test robot <lkp@intel.com> wrote:

> Hi Sebastian,
> 
> [auto build test ERROR on tip/sched/core]
> [also build test ERROR on v4.5-rc3 next-20160211]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
> 

Gotta love Wu's autobot!

> url:    https://github.com/0day-ci/linux/commits/Sebastian-Andrzej-Siewior/ftrace-move-the-CALLER_ADDRx-macros-into-its-own-header/20160212-072259
> config: s390-allyesconfig (attached as .config)
> reproduce:
>         wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=s390 
> 
> All errors (new ones prefixed by >>):
> 
>    In file included from include/linux/ftrace_caller.h:4:0,
>                     from include/linux/sched.h:62,
>                     from include/linux/kvm_host.h:15,
>                     from arch/s390/kernel/asm-offsets.c:10:
>    arch/s390/include/asm/ftrace.h: In function 'ftrace_generate_call_insn':
> >> arch/s390/include/asm/ftrace.h:77:2: error: implicit declaration of function 'is_module_addr' [-Werror=implicit-function-declaration]  
>      target = is_module_addr((void *) ip) ? ftrace_plt : FTRACE_ADDR;
>      ^
>    In file included from include/linux/mm.h:67:0,
>                     from include/linux/kvm_host.h:17,
>                     from arch/s390/kernel/asm-offsets.c:10:
>    arch/s390/include/asm/pgtable.h: At top level:
> >> arch/s390/include/asm/pgtable.h:120:19: error: static declaration of 'is_module_addr' follows non-static declaration  
>     static inline int is_module_addr(void *addr)

Looks like you need to add this? (maybe)

-- Steve

diff --git a/arch/s390/include/asm/ftrace.h b/arch/s390/include/asm/ftrace.h
index 836c56290499..16f24f92609d 100644
--- a/arch/s390/include/asm/ftrace.h
+++ b/arch/s390/include/asm/ftrace.h
@@ -12,6 +12,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/module.h>
+
 #define ftrace_return_address(n) __builtin_return_address(n)
 
 void _mcount(void);

Re: [PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[Sebastian Andrzej Siewior]

On 02/12/2016 01:45 AM, Steven Rostedt wrote:
> Looks like you need to add this? (maybe)

no. It will then try to include other files and complain even more.
I've sent v2 and drop this split and looks simpler now.

> -- Steve

Sebastian

Re: [PATCH 2/2] kernel: sched: fix preempt_disable_ip recodring for preempt_disable()

[Steven Rostedt]

On Fri, 12 Feb 2016 16:21:53 +0100
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> On 02/12/2016 01:45 AM, Steven Rostedt wrote:
> > Looks like you need to add this? (maybe)  
> 
> no. It will then try to include other files and complain even more.
> I've sent v2 and drop this split and looks simpler now.
> 

Hence why I said "maybe" ;-)

-- Steve