Skip to content

Commit 51d351d

Browse files
rostedtgregkh
rostedt
authored and
gregkh
committed
ftrace: Fix function graph with loading of modules
commit 8a56d77 upstream. Commit 8c4f3c3 "ftrace: Check module functions being traced on reload" fixed module loading and unloading with respect to function tracing, but it missed the function graph tracer. If you perform the following # cd /sys/kernel/debug/tracing # echo function_graph > current_tracer # modprobe nfsd # echo nop > current_tracer You'll get the following oops message: ------------[ cut here ]------------ WARNING: CPU: 2 PID: 2910 at /linux.git/kernel/trace/ftrace.c:1640 __ftrace_hash_rec_update.part.35+0x168/0x1b9() Modules linked in: nfsd exportfs nfs_acl lockd ipt_MASQUERADE sunrpc ip6t_REJECT nf_conntrack_ipv6 nf_defrag_ipv6 ip6table_filter ip6_tables uinput snd_hda_codec_idt CPU: 2 PID: 2910 Comm: bash Not tainted 3.13.0-rc1-test #7 Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS SDBLI944.86P 05/08/2007 0000000000000668 ffff8800787efcf8 ffffffff814fe193 ffff88007d500000 0000000000000000 ffff8800787efd38 ffffffff8103b80a 0000000000000668 ffffffff810b2b9a ffffffff81a48370 0000000000000001 ffff880037aea000 Call Trace: [<ffffffff814fe193>] dump_stack+0x4f/0x7c [<ffffffff8103b80a>] warn_slowpath_common+0x81/0x9b [<ffffffff810b2b9a>] ? __ftrace_hash_rec_update.part.35+0x168/0x1b9 [<ffffffff8103b83e>] warn_slowpath_null+0x1a/0x1c [<ffffffff810b2b9a>] __ftrace_hash_rec_update.part.35+0x168/0x1b9 [<ffffffff81502f89>] ? __mutex_lock_slowpath+0x364/0x364 [<ffffffff810b2cc2>] ftrace_shutdown+0xd7/0x12b [<ffffffff810b47f0>] unregister_ftrace_graph+0x49/0x78 [<ffffffff810c4b30>] graph_trace_reset+0xe/0x10 [<ffffffff810bf393>] tracing_set_tracer+0xa7/0x26a [<ffffffff810bf5e1>] tracing_set_trace_write+0x8b/0xbd [<ffffffff810c501c>] ? ftrace_return_to_handler+0xb2/0xde [<ffffffff811240a8>] ? __sb_end_write+0x5e/0x5e [<ffffffff81122aed>] vfs_write+0xab/0xf6 [<ffffffff8150a185>] ftrace_graph_caller+0x85/0x85 [<ffffffff81122dbd>] SyS_write+0x59/0x82 [<ffffffff8150a185>] ftrace_graph_caller+0x85/0x85 [<ffffffff8150a2d2>] system_call_fastpath+0x16/0x1b ---[ end trace 940358030751eafb ]--- The above mentioned commit didn't go far enough. Well, it covered the function tracer by adding checks in __register_ftrace_function(). The problem is that the function graph tracer circumvents that (for a slight efficiency gain when function graph trace is running with a function tracer. The gain was not worth this). The problem came with ftrace_startup() which should always be called after __register_ftrace_function(), if you want this bug to be completely fixed. Anyway, this solution moves __register_ftrace_function() inside of ftrace_startup() and removes the need to call them both. Reported-by: Dave Wysochanski <dwysocha@redhat.com> Fixes: ed926f9 ("ftrace: Use counters to enable functions to trace") Signed-off-by: Steven Rostedt <rostedt@goodmis.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 5833570 commit 51d351d

1 file changed

Lines changed: 34 additions & 34 deletions

File tree

‎kernel/trace/ftrace.c‎

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -312,9 +312,6 @@ static int remove_ftrace_list_ops(struct ftrace_ops **list,
312312

313313
static int __register_ftrace_function(struct ftrace_ops *ops)
314314
{
315-
if (ftrace_disabled)
316-
return -ENODEV;
317-
318315
if (FTRACE_WARN_ON(ops == &global_ops))
319316
return -EINVAL;
320317

@@ -348,9 +345,6 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
348345
{
349346
int ret;
350347

351-
if (ftrace_disabled)
352-
return -ENODEV;
353-
354348
if (WARN_ON(!(ops->flags & FTRACE_OPS_FL_ENABLED)))
355349
return -EBUSY;
356350

@@ -1940,10 +1934,15 @@ static void ftrace_startup_enable(int command)
19401934
static int ftrace_startup(struct ftrace_ops *ops, int command)
19411935
{
19421936
bool hash_enable = true;
1937+
int ret;
19431938

19441939
if (unlikely(ftrace_disabled))
19451940
return -ENODEV;
19461941

1942+
ret = __register_ftrace_function(ops);
1943+
if (ret)
1944+
return ret;
1945+
19471946
ftrace_start_up++;
19481947
command |= FTRACE_UPDATE_CALLS;
19491948

@@ -1965,12 +1964,17 @@ static int ftrace_startup(struct ftrace_ops *ops, int command)
19651964
return 0;
19661965
}
19671966

1968-
static void ftrace_shutdown(struct ftrace_ops *ops, int command)
1967+
static int ftrace_shutdown(struct ftrace_ops *ops, int command)
19691968
{
19701969
bool hash_disable = true;
1970+
int ret;
19711971

19721972
if (unlikely(ftrace_disabled))
1973-
return;
1973+
return -ENODEV;
1974+
1975+
ret = __unregister_ftrace_function(ops);
1976+
if (ret)
1977+
return ret;
19741978

19751979
ftrace_start_up--;
19761980
/*
@@ -2005,9 +2009,10 @@ static void ftrace_shutdown(struct ftrace_ops *ops, int command)
20052009
}
20062010

20072011
if (!command || !ftrace_enabled)
2008-
return;
2012+
return 0;
20092013

20102014
ftrace_run_update_code(command);
2015+
return 0;
20112016
}
20122017

20132018
static void ftrace_startup_sysctl(void)
@@ -2873,16 +2878,13 @@ static void __enable_ftrace_function_probe(void)
28732878
if (i == FTRACE_FUNC_HASHSIZE)
28742879
return;
28752880

2876-
ret = __register_ftrace_function(&trace_probe_ops);
2877-
if (!ret)
2878-
ret = ftrace_startup(&trace_probe_ops, 0);
2881+
ret = ftrace_startup(&trace_probe_ops, 0);
28792882

28802883
ftrace_probe_registered = 1;
28812884
}
28822885

28832886
static void __disable_ftrace_function_probe(void)
28842887
{
2885-
int ret;
28862888
int i;
28872889

28882890
if (!ftrace_probe_registered)
@@ -2895,9 +2897,7 @@ static void __disable_ftrace_function_probe(void)
28952897
}
28962898

28972899
/* no more funcs left */
2898-
ret = __unregister_ftrace_function(&trace_probe_ops);
2899-
if (!ret)
2900-
ftrace_shutdown(&trace_probe_ops, 0);
2900+
ftrace_shutdown(&trace_probe_ops, 0);
29012901

29022902
ftrace_probe_registered = 0;
29032903
}
@@ -3948,12 +3948,15 @@ device_initcall(ftrace_nodyn_init);
39483948
static inline int ftrace_init_dyn_debugfs(struct dentry *d_tracer) { return 0; }
39493949
static inline void ftrace_startup_enable(int command) { }
39503950
/* Keep as macros so we do not need to define the commands */
3951-
# define ftrace_startup(ops, command) \
3952-
({ \
3953-
(ops)->flags |= FTRACE_OPS_FL_ENABLED; \
3954-
0; \
3951+
# define ftrace_startup(ops, command) \
3952+
({ \
3953+
int ___ret = __register_ftrace_function(ops); \
3954+
if (!___ret) \
3955+
(ops)->flags |= FTRACE_OPS_FL_ENABLED; \
3956+
___ret; \
39553957
})
3956-
# define ftrace_shutdown(ops, command) do { } while (0)
3958+
# define ftrace_shutdown(ops, command) __unregister_ftrace_function(ops)
3959+
39573960
# define ftrace_startup_sysctl() do { } while (0)
39583961
# define ftrace_shutdown_sysctl() do { } while (0)
39593962

@@ -4323,15 +4326,8 @@ int register_ftrace_function(struct ftrace_ops *ops)
43234326

43244327
mutex_lock(&ftrace_lock);
43254328

4326-
if (unlikely(ftrace_disabled))
4327-
goto out_unlock;
4328-
4329-
ret = __register_ftrace_function(ops);
4330-
if (!ret)
4331-
ret = ftrace_startup(ops, 0);
4332-
4329+
ret = ftrace_startup(ops, 0);
43334330

4334-
out_unlock:
43354331
mutex_unlock(&ftrace_lock);
43364332
return ret;
43374333
}
@@ -4348,9 +4344,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
43484344
int ret;
43494345

43504346
mutex_lock(&ftrace_lock);
4351-
ret = __unregister_ftrace_function(ops);
4352-
if (!ret)
4353-
ftrace_shutdown(ops, 0);
4347+
ret = ftrace_shutdown(ops, 0);
43544348
mutex_unlock(&ftrace_lock);
43554349

43564350
return ret;
@@ -4544,6 +4538,12 @@ ftrace_suspend_notifier_call(struct notifier_block *bl, unsigned long state,
45444538
return NOTIFY_DONE;
45454539
}
45464540

4541+
/* Just a place holder for function graph */
4542+
static struct ftrace_ops fgraph_ops __read_mostly = {
4543+
.func = ftrace_stub,
4544+
.flags = FTRACE_OPS_FL_GLOBAL,
4545+
};
4546+
45474547
int register_ftrace_graph(trace_func_graph_ret_t retfunc,
45484548
trace_func_graph_ent_t entryfunc)
45494549
{
@@ -4570,7 +4570,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,
45704570
ftrace_graph_return = retfunc;
45714571
ftrace_graph_entry = entryfunc;
45724572

4573-
ret = ftrace_startup(&global_ops, FTRACE_START_FUNC_RET);
4573+
ret = ftrace_startup(&fgraph_ops, FTRACE_START_FUNC_RET);
45744574

45754575
out:
45764576
mutex_unlock(&ftrace_lock);
@@ -4587,7 +4587,7 @@ void unregister_ftrace_graph(void)
45874587
ftrace_graph_active--;
45884588
ftrace_graph_return = (trace_func_graph_ret_t)ftrace_stub;
45894589
ftrace_graph_entry = ftrace_graph_entry_stub;
4590-
ftrace_shutdown(&global_ops, FTRACE_STOP_FUNC_RET);
4590+
ftrace_shutdown(&fgraph_ops, FTRACE_STOP_FUNC_RET);
45914591
unregister_pm_notifier(&ftrace_suspend_notifier);
45924592
unregister_trace_sched_switch(ftrace_graph_probe_sched_switch, NULL);
45934593

0 commit comments

Comments
 (0)