| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 3 Jan 2008 12:35:06 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
Linus Torvalds <torvalds@...ux-foundation.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Christoph Hellwig <hch@...radead.org>,
Gregory Haskins <ghaskins@...ell.com>,
Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
"William L. Irwin" <sparclinux@...r.kernel.org>,
Steven Rostedt <srostedt@...hat.com>
Subject: Re: [RFC PATCH 01/11] Add basic support for gcc profiler
instrumentation
* Steven Rostedt (rostedt@...dmis.org) wrote:
...
> Index: linux-compile.git/arch/x86/Kconfig
> ===================================================================
> --- linux-compile.git.orig/arch/x86/Kconfig 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/arch/x86/Kconfig 2008-01-03 01:02:33.000000000 -0500
> @@ -28,6 +28,12 @@ config GENERIC_CMOS_UPDATE
> bool
> default y
>
> +# function tracing might turn this off:
> +config REGPARM
> + bool
> + depends on !MCOUNT
> + default y
> +
> config CLOCKSOURCE_WATCHDOG
> bool
> default y
....
> Index: linux-compile.git/arch/x86/kernel/mcount-wrapper.S
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-compile.git/arch/x86/kernel/mcount-wrapper.S 2008-01-03 01:02:33.000000000 -0500
> @@ -0,0 +1,25 @@
> +/*
> + * linux/arch/x86/mcount-wrapper.S
> + *
> + * Copyright (C) 2004 Ingo Molnar
> + */
> +
> +.globl mcount
> +mcount:
> + cmpl $0, mcount_enabled
> + jz out
> +
> + push %ebp
> + mov %esp, %ebp
> + pushl %eax
> + pushl %ecx
> + pushl %edx
> +
> + call __mcount
> +
> + popl %edx
> + popl %ecx
> + popl %eax
> + popl %ebp
Writing this stack setup in assembly may be the one thing that conflicts
with REGPARM ?
> +out:
> + ret
> Index: linux-compile.git/include/linux/linkage.h
> ===================================================================
> --- linux-compile.git.orig/include/linux/linkage.h 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/include/linux/linkage.h 2008-01-03 01:02:33.000000000 -0500
> @@ -3,6 +3,8 @@
>
> #include <asm/linkage.h>
>
> +#define notrace __attribute__((no_instrument_function))
> +
> #ifdef __cplusplus
> #define CPP_ASMLINKAGE extern "C"
> #else
> Index: linux-compile.git/kernel/sysctl.c
> ===================================================================
> --- linux-compile.git.orig/kernel/sysctl.c 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/kernel/sysctl.c 2008-01-03 01:02:33.000000000 -0500
> @@ -46,6 +46,7 @@
> #include <linux/nfs_fs.h>
> #include <linux/acpi.h>
> #include <linux/reboot.h>
> +#include <linux/mcount.h>
>
> #include <asm/uaccess.h>
> #include <asm/processor.h>
> @@ -470,6 +471,16 @@ static struct ctl_table kern_table[] = {
> .mode = 0644,
> .proc_handler = &proc_dointvec,
> },
> +#ifdef CONFIG_MCOUNT
> + {
> + .ctl_name = CTL_UNNUMBERED,
> + .procname = "mcount_enabled",
> + .data = &mcount_enabled,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = &proc_dointvec,
> + },
> +#endif
> #ifdef CONFIG_KMOD
> {
> .ctl_name = KERN_MODPROBE,
> Index: linux-compile.git/lib/Kconfig.debug
> ===================================================================
> --- linux-compile.git.orig/lib/Kconfig.debug 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/lib/Kconfig.debug 2008-01-03 01:02:33.000000000 -0500
> @@ -517,4 +517,6 @@ config FAULT_INJECTION_STACKTRACE_FILTER
> help
> Provide stacktrace filter for fault-injection capabilities
>
> +source lib/mcount/Kconfig
> +
> source "samples/Kconfig"
> Index: linux-compile.git/lib/Makefile
> ===================================================================
> --- linux-compile.git.orig/lib/Makefile 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/lib/Makefile 2008-01-03 01:02:33.000000000 -0500
> @@ -66,6 +66,8 @@ obj-$(CONFIG_AUDIT_GENERIC) += audit.o
> obj-$(CONFIG_SWIOTLB) += swiotlb.o
> obj-$(CONFIG_FAULT_INJECTION) += fault-inject.o
>
> +obj-$(CONFIG_MCOUNT) += mcount/
> +
> lib-$(CONFIG_GENERIC_BUG) += bug.o
>
> hostprogs-y := gen_crc32table
> Index: linux-compile.git/lib/mcount/Kconfig
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-compile.git/lib/mcount/Kconfig 2008-01-03 01:02:33.000000000 -0500
> @@ -0,0 +1,6 @@
> +
> +# MCOUNT itself is useless, or will just be added overhead.
> +# It needs something to register a function with it.
> +config MCOUNT
> + bool
> + depends on DEBUG_KERNEL
> Index: linux-compile.git/lib/mcount/Makefile
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-compile.git/lib/mcount/Makefile 2008-01-03 01:02:33.000000000 -0500
> @@ -0,0 +1,3 @@
> +obj-$(CONFIG_MCOUNT) += libmcount.o
> +
> +libmcount-objs := mcount.o
> Index: linux-compile.git/lib/mcount/mcount.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-compile.git/lib/mcount/mcount.c 2008-01-03 01:02:33.000000000 -0500
> @@ -0,0 +1,78 @@
> +/*
> + * Infrastructure for profiling code inserted by 'gcc -pg'.
> + *
> + * Copyright (C) 2007 Arnaldo Carvalho de Melo <acme@...hat.com>
> + *
> + * Converted to be more generic:
> + * Copyright (C) 2007-2008 Steven Rostedt <srostedt@...hat.com>
> + *
> + * From code in the latency_tracer, that is:
> + *
> + * Copyright (C) 2004-2006 Ingo Molnar
> + * Copyright (C) 2004 William Lee Irwin III
> + */
> +
> +#include <linux/module.h>
> +#include <linux/mcount.h>
> +
> +/*
> + * Since we have nothing protecting between the test of
> + * mcount_trace_function and the call to it, we can't
> + * set it to NULL without risking a race that will have
> + * the kernel call the NULL pointer. Instead, we just
> + * set the function pointer to a dummy function.
> + */
> +notrace void dummy_mcount_tracer(unsigned long ip,
> + unsigned long parent_ip)
> +{
> + /* do nothing */
> +}
> +
> +mcount_func_t mcount_trace_function = dummy_mcount_tracer;
> +int mcount_enabled;
> +
> +/** __mcount - hook for profiling
> + *
> + * This routine is called from the arch specific mcount routine, that in turn is
> + * called from code inserted by gcc -pg.
> + */
> +notrace void __mcount(void)
> +{
> + if (mcount_trace_function != dummy_mcount_tracer)
> + mcount_trace_function(CALLER_ADDR1, CALLER_ADDR2);
> +}
I don't see what the mcount_trace_function test gives us here : we
already tested mcount_enabled.
> +EXPORT_SYMBOL_GPL(mcount);
> +/*
> + * The above EXPORT_SYMBOL is for the gcc call of mcount and not the
> + * function __mcount that it is underneath. I put the export there
> + * to fool checkpatch.pl. It wants that export to be with the
> + * function, but that function happens to be in assembly.
> + */
> +
> +/**
> + * register_mcount_function - register a function for profiling
> + * @func - the function for profiling.
> + *
> + * Register a function to be called by all functions in the
> + * kernel.
> + *
> + * Note: @func and all the functions it calls must be labeled
> + * with "notrace", otherwise it will go into a
> + * recursive loop.
> + */
> +int register_mcount_function(mcount_func_t func)
> +{
> + mcount_trace_function = func;
> + return 0;
> +}
> +
> +/**
> + * clear_mcount_function - reset the mcount function
> + *
> + * This NULLs the mcount function and in essence stops
> + * tracing. There may be lag
> + */
> +void clear_mcount_function(void)
> +{
> + mcount_trace_function = dummy_mcount_tracer;
> +}
> Index: linux-compile.git/include/linux/mcount.h
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-compile.git/include/linux/mcount.h 2008-01-03 01:02:33.000000000 -0500
> @@ -0,0 +1,21 @@
> +#ifndef _LINUX_MCOUNT_H
> +#define _LINUX_MCOUNT_H
> +
> +#ifdef CONFIG_MCOUNT
> +extern int mcount_enabled;
> +
> +#include <linux/linkage.h>
> +
> +#define CALLER_ADDR0 ((unsigned long)__builtin_return_address(0))
> +#define CALLER_ADDR1 ((unsigned long)__builtin_return_address(1))
> +#define CALLER_ADDR2 ((unsigned long)__builtin_return_address(2))
> +
> +typedef void (*mcount_func_t)(unsigned long ip, unsigned long parent_ip);
> +
> +extern void mcount(void);
> +
> +int register_mcount_function(mcount_func_t func);
> +void clear_mcount_function(void);
> +
> +#endif /* CONFIG_MCOUNT */
> +#endif /* _LINUX_MCOUNT_H */
> Index: linux-compile.git/arch/x86/kernel/entry_64.S
> ===================================================================
> --- linux-compile.git.orig/arch/x86/kernel/entry_64.S 2008-01-03 01:02:28.000000000 -0500
> +++ linux-compile.git/arch/x86/kernel/entry_64.S 2008-01-03 01:02:33.000000000 -0500
> @@ -53,6 +53,52 @@
>
> .code64
>
> +#ifdef CONFIG_MCOUNT
> +
> +ENTRY(mcount)
> + cmpl $0, mcount_enabled
> + jz out
> +
> + push %rbp
> +
> + lea dummy_mcount_tracer, %rbp
> + cmpq %rbp, mcount_trace_function
Ok, so we normally jump over the function call (with mcount_enabled being 0)
but we can call it in rare cases when it is being set concurrently (even
though the mcount_trace_function is there, concurrency could still allow
the call).
Therefore we have one data cache hit when disabled (mcount_enabled), and
must do a supplementary comparison before the call when enabled. I
wonder why the cmpq %rbp, mcount_trace_function test is there at all ?
> + jz out_rbp
> +
> + mov %rsp,%rbp
> +
> + push %r11
> + push %r10
> + push %r9
> + push %r8
> + push %rdi
> + push %rsi
> + push %rdx
> + push %rcx
> + push %rax
> +
> + mov 0x0(%rbp),%rax
> + mov 0x8(%rbp),%rdi
> + mov 0x8(%rax),%rsi
> +
> + call *mcount_trace_function
> +
> + pop %rax
> + pop %rcx
> + pop %rdx
> + pop %rsi
> + pop %rdi
> + pop %r8
> + pop %r9
> + pop %r10
> + pop %r11
> +
> +out_rbp:
> + pop %rbp
> +out:
> + ret
> +#endif
> +
> #ifndef CONFIG_PREEMPT
> #define retint_kernel retint_restore_args
> #endif
>
> --
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists