From: Greg KH <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
alan@lxorguk.ukuu.org.uk, Jiri Olsa <jolsa@redhat.com>,
Steven Rostedt <rostedt@goodmis.org>
Subject: [14/65] ftrace: Fix unregister ftrace_ops accounting
Date: Wed, 01 Feb 2012 12:55:54 -0800 [thread overview]
Message-ID: <20120201205738.721739735@clark.kroah.org> (raw)
In-Reply-To: <20120201210236.GA25966@kroah.com>
3.0-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jiri Olsa <jolsa@redhat.com>
commit 30fb6aa74011dcf595f306ca2727254d708b786e upstream.
Multiple users of the function tracer can register their functions
with the ftrace_ops structure. The accounting within ftrace will
update the counter on each function record that is being traced.
When the ftrace_ops filtering adds or removes functions, the
function records will be updated accordingly if the ftrace_ops is
still registered.
When a ftrace_ops is removed, the counter of the function records,
that the ftrace_ops traces, are decremented. When they reach zero
the functions that they represent are modified to stop calling the
mcount code.
When changes are made, the code is updated via stop_machine() with
a command passed to the function to tell it what to do. There is an
ENABLE and DISABLE command that tells the called function to enable
or disable the functions. But the ENABLE is really a misnomer as it
should just update the records, as records that have been enabled
and now have a count of zero should be disabled.
The DISABLE command is used to disable all functions regardless of
their counter values. This is the big off switch and is not the
complement of the ENABLE command.
To make matters worse, when a ftrace_ops is unregistered and there
is another ftrace_ops registered, neither the DISABLE nor the
ENABLE command are set when calling into the stop_machine() function
and the records will not be updated to match their counter. A command
is passed to that function that will update the mcount code to call
the registered callback directly if it is the only one left. This
means that the ftrace_ops that is still registered will have its callback
called by all functions that have been set for it as well as the ftrace_ops
that was just unregistered.
Here's a way to trigger this bug. Compile the kernel with
CONFIG_FUNCTION_PROFILER set and with CONFIG_FUNCTION_GRAPH not set:
CONFIG_FUNCTION_PROFILER=y
# CONFIG_FUNCTION_GRAPH is not set
This will force the function profiler to use the function tracer instead
of the function graph tracer.
# cd /sys/kernel/debug/tracing
# echo schedule > set_ftrace_filter
# echo function > current_tracer
# cat set_ftrace_filter
schedule
# cat trace
# tracer: nop
#
# entries-in-buffer/entries-written: 692/68108025 #P:4
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
kworker/0:2-909 [000] .... 531.235574: schedule <-worker_thread
<idle>-0 [001] .N.. 531.235575: schedule <-cpu_idle
kworker/0:2-909 [000] .... 531.235597: schedule <-worker_thread
sshd-2563 [001] .... 531.235647: schedule <-schedule_hrtimeout_range_clock
# echo 1 > function_profile_enabled
# echo 0 > function_porfile_enabled
# cat set_ftrace_filter
schedule
# cat trace
# tracer: function
#
# entries-in-buffer/entries-written: 159701/118821262 #P:4
#
# _-----=> irqs-off
# / _----=> need-resched
# | / _---=> hardirq/softirq
# || / _--=> preempt-depth
# ||| / delay
# TASK-PID CPU# |||| TIMESTAMP FUNCTION
# | | | |||| | |
<idle>-0 [002] ...1 604.870655: local_touch_nmi <-cpu_idle
<idle>-0 [002] d..1 604.870655: enter_idle <-cpu_idle
<idle>-0 [002] d..1 604.870656: atomic_notifier_call_chain <-enter_idle
<idle>-0 [002] d..1 604.870656: __atomic_notifier_call_chain <-atomic_notifier_call_chain
The same problem could have happened with the trace_probe_ops,
but they are modified with the set_frace_filter file which does the
update at closure of the file.
The simple solution is to change ENABLE to UPDATE and call it every
time an ftrace_ops is unregistered.
Link: http://lkml.kernel.org/r/1323105776-26961-3-git-send-email-jolsa@redhat.com
Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
kernel/trace/ftrace.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -952,7 +952,7 @@ struct ftrace_func_probe {
};
enum {
- FTRACE_ENABLE_CALLS = (1 << 0),
+ FTRACE_UPDATE_CALLS = (1 << 0),
FTRACE_DISABLE_CALLS = (1 << 1),
FTRACE_UPDATE_TRACE_FUNC = (1 << 2),
FTRACE_START_FUNC_RET = (1 << 3),
@@ -1521,7 +1521,7 @@ int ftrace_text_reserved(void *start, vo
static int
-__ftrace_replace_code(struct dyn_ftrace *rec, int enable)
+__ftrace_replace_code(struct dyn_ftrace *rec, int update)
{
unsigned long ftrace_addr;
unsigned long flag = 0UL;
@@ -1529,17 +1529,17 @@ __ftrace_replace_code(struct dyn_ftrace
ftrace_addr = (unsigned long)FTRACE_ADDR;
/*
- * If we are enabling tracing:
+ * If we are updating calls:
*
* If the record has a ref count, then we need to enable it
* because someone is using it.
*
* Otherwise we make sure its disabled.
*
- * If we are disabling tracing, then disable all records that
+ * If we are disabling calls, then disable all records that
* are enabled.
*/
- if (enable && (rec->flags & ~FTRACE_FL_MASK))
+ if (update && (rec->flags & ~FTRACE_FL_MASK))
flag = FTRACE_FL_ENABLED;
/* If the state of this record hasn't changed, then do nothing */
@@ -1555,7 +1555,7 @@ __ftrace_replace_code(struct dyn_ftrace
return ftrace_make_nop(NULL, rec, ftrace_addr);
}
-static void ftrace_replace_code(int enable)
+static void ftrace_replace_code(int update)
{
struct dyn_ftrace *rec;
struct ftrace_page *pg;
@@ -1569,7 +1569,7 @@ static void ftrace_replace_code(int enab
if (rec->flags & FTRACE_FL_FREE)
continue;
- failed = __ftrace_replace_code(rec, enable);
+ failed = __ftrace_replace_code(rec, update);
if (failed) {
ftrace_bug(failed, rec->ip);
/* Stop processing */
@@ -1619,7 +1619,7 @@ static int __ftrace_modify_code(void *da
{
int *command = data;
- if (*command & FTRACE_ENABLE_CALLS)
+ if (*command & FTRACE_UPDATE_CALLS)
ftrace_replace_code(1);
else if (*command & FTRACE_DISABLE_CALLS)
ftrace_replace_code(0);
@@ -1675,7 +1675,7 @@ static int ftrace_startup(struct ftrace_
return -ENODEV;
ftrace_start_up++;
- command |= FTRACE_ENABLE_CALLS;
+ command |= FTRACE_UPDATE_CALLS;
/* ops marked global share the filter hashes */
if (ops->flags & FTRACE_OPS_FL_GLOBAL) {
@@ -1727,8 +1727,7 @@ static void ftrace_shutdown(struct ftrac
if (ops != &global_ops || !global_start_up)
ops->flags &= ~FTRACE_OPS_FL_ENABLED;
- if (!ftrace_start_up)
- command |= FTRACE_DISABLE_CALLS;
+ command |= FTRACE_UPDATE_CALLS;
if (saved_ftrace_func != ftrace_trace_function) {
saved_ftrace_func = ftrace_trace_function;
@@ -1750,7 +1749,7 @@ static void ftrace_startup_sysctl(void)
saved_ftrace_func = NULL;
/* ftrace_start_up is true if we want ftrace running */
if (ftrace_start_up)
- ftrace_run_update_code(FTRACE_ENABLE_CALLS);
+ ftrace_run_update_code(FTRACE_UPDATE_CALLS);
}
static void ftrace_shutdown_sysctl(void)
@@ -2903,7 +2902,7 @@ ftrace_set_regex(struct ftrace_ops *ops,
ret = ftrace_hash_move(ops, enable, orig_hash, hash);
if (!ret && ops->flags & FTRACE_OPS_FL_ENABLED
&& ftrace_enabled)
- ftrace_run_update_code(FTRACE_ENABLE_CALLS);
+ ftrace_run_update_code(FTRACE_UPDATE_CALLS);
mutex_unlock(&ftrace_lock);
@@ -3091,7 +3090,7 @@ ftrace_regex_release(struct inode *inode
orig_hash, iter->hash);
if (!ret && (iter->ops->flags & FTRACE_OPS_FL_ENABLED)
&& ftrace_enabled)
- ftrace_run_update_code(FTRACE_ENABLE_CALLS);
+ ftrace_run_update_code(FTRACE_UPDATE_CALLS);
mutex_unlock(&ftrace_lock);
}
next prev parent reply other threads:[~2012-02-01 21:47 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-01 21:02 [00/65] 3.0.19-stable review Greg KH
2012-02-01 20:55 ` [01/65] ALSA: hda - Fix silent outputs from docking-station jacks of Dell laptops Greg KH
2012-02-01 20:55 ` [02/65] eCryptfs: Sanitize write counts of /dev/ecryptfs Greg KH
2012-02-01 20:55 ` [03/65] ecryptfs: Improve metadata read failure logging Greg KH
2012-02-01 20:55 ` [04/65] eCryptfs: Make truncate path killable Greg KH
2012-02-01 20:55 ` [05/65] eCryptfs: Check inode changes in setattr Greg KH
2012-02-01 20:55 ` [06/65] eCryptfs: Fix oops when printing debug info in extent crypto functions Greg KH
2012-02-01 20:55 ` [07/65] drm/radeon/kms: Add an MSI quirk for Dell RS690 Greg KH
2012-02-01 20:55 ` [08/65] drm: Fix authentication kernel crash Greg KH
2012-02-01 20:55 ` [09/65] xfs: Fix missing xfs_iunlock() on error recovery path in xfs_readlink() Greg KH
2012-02-01 20:55 ` [10/65] crypto: sha512 - make it work, undo percpu message schedule Greg KH
2012-02-01 20:55 ` [11/65] crypto: sha512 - reduce stack usage to safe number Greg KH
2012-02-01 20:55 ` [12/65] ftrace: Balance records when updating the hash Greg KH
2012-02-01 20:55 ` [13/65] ftrace: Update filter when tracing enabled in set_ftrace_filter() Greg KH
2012-02-01 20:55 ` Greg KH [this message]
2012-02-01 20:55 ` [15/65] ah: Dont return NET_XMIT_DROP on input Greg KH
2012-02-01 20:55 ` [16/65] xfs: fix endian conversion issue in discard code Greg KH
2012-02-01 20:55 ` [17/65] x86/uv: Fix uv_gpa_to_soc_phys_ram() shift Greg KH
2012-02-01 20:55 ` [18/65] x86/microcode_amd: Add support for CPU family specific container files Greg KH
2012-02-01 20:55 ` [19/65] ALSA: hda - Fix silent output on ASUS A6Rp Greg KH
2012-02-01 20:56 ` [20/65] ALSA: hda - Fix silent output on Haier W18 laptop Greg KH
2012-02-01 20:56 ` [21/65] drm/i915/sdvo: always set positive sync polarity Greg KH
2012-02-01 20:56 ` [22/65] cap_syslog: dont use WARN_ONCE for CAP_SYS_ADMIN deprecation warning Greg KH
2012-02-01 20:56 ` [23/65] mach-ux500: enable ARM errata 764369 Greg KH
2012-02-01 20:56 ` [24/65] ARM: 7296/1: proc-v7.S: remove HARVARD_CACHE preprocessor guards Greg KH
2012-02-01 20:56 ` [25/65] USB: option: Add LG docomo L-02C Greg KH
2012-02-01 20:56 ` [26/65] USB: ftdi_sio: fix TIOCSSERIAL baud_base handling Greg KH
2012-02-01 20:56 ` [27/65] USB: ftdi_sio: fix initial baud rate Greg KH
2012-02-01 20:56 ` [28/65] USB: ftdi_sio: add PID for TI XDS100v2 / BeagleBone A3 Greg KH
2012-02-01 20:56 ` [29/65] USB: serial: ftdi additional IDs Greg KH
2012-02-01 20:56 ` [30/65] USB: ftdi_sio: Add more identifiers Greg KH
2012-02-01 20:56 ` [31/65] USB: cdc-wdm: updating desc->length must be protected by spin_lock Greg KH
2012-02-01 20:56 ` [32/65] USB: cdc-wdm: use two mutexes to allow simultaneous read and write Greg KH
2012-02-01 20:56 ` [33/65] qcaux: add more Pantech UML190 and UML290 ports Greg KH
2012-02-01 20:56 ` [34/65] usb: io_ti: Make edge_remove_sysfs_attrs the port_remove method Greg KH
2012-02-01 20:56 ` [35/65] TTY: fix UV serial console regression Greg KH
2012-02-01 20:56 ` [36/65] serial: amba-pl011: lock console writes against interrupts Greg KH
2012-02-01 20:56 ` [37/65] jsm: Fixed EEH recovery error Greg KH
2012-02-01 20:56 ` [38/65] vmwgfx: Fix assignment in vmw_framebuffer_create_handle Greg KH
2012-02-01 20:56 ` [39/65] USB: usbsevseg: fix max length Greg KH
2012-02-01 20:56 ` [40/65] drivers/usb/host/ehci-fsl.c: add missing iounmap Greg KH
2012-02-01 20:56 ` [41/65] xhci: Fix USB 3.0 device restart on resume Greg KH
2012-02-01 20:56 ` [42/65] xHCI: Cleanup isoc transfer ring when TD length mismatch found Greg KH
2012-02-01 20:56 ` [43/65] hwmon: (f71805f) Fix clamping of temperature limits Greg KH
2012-02-01 20:56 ` [44/65] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Greg KH
2012-02-01 20:56 ` [45/65] hwmon: (sht15) fix bad error code Greg KH
2012-02-01 20:56 ` [46/65] USB: cdc-wdm: call wake_up_all to allow driver to shutdown on device removal Greg KH
2012-02-01 20:56 ` [47/65] USB: cdc-wdm: better allocate a buffer that is at least as big as we tell the USB core Greg KH
2012-02-01 20:56 ` [48/65] USB: cdc-wdm: Avoid hanging on interface with no USB_CDC_DMM_TYPE Greg KH
2012-02-01 20:56 ` [49/65] netns: fix net_alloc_generic() Greg KH
2012-02-01 20:56 ` [50/65] netns: Fail conspicously if someone uses net_generic at an inappropriate time Greg KH
2012-02-01 20:56 ` [51/65] net caif: Register properly as a pernet subsystem Greg KH
2012-02-01 20:56 ` [52/65] bonding: fix enslaving in alb mode when link down Greg KH
2012-02-01 20:56 ` [53/65] l2tp: l2tp_ip - fix possible oops on packet receive Greg KH
2012-02-01 20:56 ` [54/65] net: bpf_jit: fix divide by 0 generation Greg KH
2012-02-01 20:56 ` [55/65] rds: Make rds_sock_lock BH rather than IRQ safe Greg KH
2012-02-01 20:56 ` [56/65] tcp: fix tcp_trim_head() to adjust segment count with skb MSS Greg KH
2012-02-01 20:56 ` [57/65] tcp: md5: using remote adress for md5 lookup in rst packet Greg KH
2012-02-01 20:56 ` [58/65] USB: serial: CP210x: Added USB-ID for the Link Instruments MSO-19 Greg KH
2012-02-01 20:56 ` [59/65] USB: cp210x: call generic open last in open Greg KH
2012-02-01 20:56 ` [60/65] USB: cp210x: fix CP2104 baudrate usage Greg KH
2012-02-01 20:56 ` [61/65] USB: cp210x: do not map baud rates to B0 Greg KH
2012-02-01 20:56 ` [62/65] USB: cp210x: fix up set_termios variables Greg KH
2012-02-01 20:56 ` [63/65] USB: cp210x: clean up, refactor and document speed handling Greg KH
2012-02-01 20:56 ` [64/65] USB: cp210x: initialise baud rate at open Greg KH
2012-02-01 20:56 ` [65/65] USB: cp210x: allow more baud rates above 1Mbaud Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120201205738.721739735@clark.kroah.org \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®