Skip to content

Commit e6ea44e

Browse files
author
Steven Rostedt
committed
ftrace: consolidate mutexes
Impact: clean up Now that ftrace_lock is a mutex, there is no reason to have three different mutexes protecting similar data. All the mutex paths are not in hot paths, so having a mutex to cover more data is not a problem. This patch removes the ftrace_sysctl_lock and ftrace_start_lock and uses the ftrace_lock to protect the locations that were protected by these locks. By doing so, this change also removes some of the lock nesting that was taking place. There are still more mutexes in ftrace.c that can probably be consolidated, but they can be dealt with later. We need to be careful about the way the locks are nested, and by consolidating, we can cause a recursive deadlock. Signed-off-by: Steven Rostedt <srostedt@redhat.com>
1 parent 52baf11 commit e6ea44e

File tree

1 file changed

+21
-47
lines changed

1 file changed

+21
-47
lines changed

‎kernel/trace/ftrace.c

Lines changed: 21 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,6 @@ int function_trace_stop;
6262
static int ftrace_disabled __read_mostly;
6363

6464
static DEFINE_MUTEX(ftrace_lock);
65-
static DEFINE_MUTEX(ftrace_sysctl_lock);
66-
static DEFINE_MUTEX(ftrace_start_lock);
6765

6866
static struct ftrace_ops ftrace_list_end __read_mostly =
6967
{
@@ -134,8 +132,6 @@ static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip)
134132

135133
static int __register_ftrace_function(struct ftrace_ops *ops)
136134
{
137-
mutex_lock(&ftrace_lock);
138-
139135
ops->next = ftrace_list;
140136
/*
141137
* We are entering ops into the ftrace_list but another
@@ -171,17 +167,12 @@ static int __register_ftrace_function(struct ftrace_ops *ops)
171167
#endif
172168
}
173169

174-
mutex_unlock(&ftrace_lock);
175-
176170
return 0;
177171
}
178172

179173
static int __unregister_ftrace_function(struct ftrace_ops *ops)
180174
{
181175
struct ftrace_ops **p;
182-
int ret = 0;
183-
184-
mutex_lock(&ftrace_lock);
185176

186177
/*
187178
* If we are removing the last function, then simply point
@@ -190,17 +181,15 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
190181
if (ftrace_list == ops && ops->next == &ftrace_list_end) {
191182
ftrace_trace_function = ftrace_stub;
192183
ftrace_list = &ftrace_list_end;
193-
goto out;
184+
return 0;
194185
}
195186

196187
for (p = &ftrace_list; *p != &ftrace_list_end; p = &(*p)->next)
197188
if (*p == ops)
198189
break;
199190

200-
if (*p != ops) {
201-
ret = -1;
202-
goto out;
203-
}
191+
if (*p != ops)
192+
return -1;
204193

205194
*p = (*p)->next;
206195

@@ -221,10 +210,7 @@ static int __unregister_ftrace_function(struct ftrace_ops *ops)
221210
}
222211
}
223212

224-
out:
225-
mutex_unlock(&ftrace_lock);
226-
227-
return ret;
213+
return 0;
228214
}
229215

230216
static void ftrace_update_pid_func(void)
@@ -622,21 +608,17 @@ static void ftrace_startup(int command)
622608
if (unlikely(ftrace_disabled))
623609
return;
624610

625-
mutex_lock(&ftrace_start_lock);
626611
ftrace_start_up++;
627612
command |= FTRACE_ENABLE_CALLS;
628613

629614
ftrace_startup_enable(command);
630-
631-
mutex_unlock(&ftrace_start_lock);
632615
}
633616

634617
static void ftrace_shutdown(int command)
635618
{
636619
if (unlikely(ftrace_disabled))
637620
return;
638621

639-
mutex_lock(&ftrace_start_lock);
640622
ftrace_start_up--;
641623
if (!ftrace_start_up)
642624
command |= FTRACE_DISABLE_CALLS;
@@ -647,11 +629,9 @@ static void ftrace_shutdown(int command)
647629
}
648630

649631
if (!command || !ftrace_enabled)
650-
goto out;
632+
return;
651633

652634
ftrace_run_update_code(command);
653-
out:
654-
mutex_unlock(&ftrace_start_lock);
655635
}
656636

657637
static void ftrace_startup_sysctl(void)
@@ -661,15 +641,13 @@ static void ftrace_startup_sysctl(void)
661641
if (unlikely(ftrace_disabled))
662642
return;
663643

664-
mutex_lock(&ftrace_start_lock);
665644
/* Force update next time */
666645
saved_ftrace_func = NULL;
667646
/* ftrace_start_up is true if we want ftrace running */
668647
if (ftrace_start_up)
669648
command |= FTRACE_ENABLE_CALLS;
670649

671650
ftrace_run_update_code(command);
672-
mutex_unlock(&ftrace_start_lock);
673651
}
674652

675653
static void ftrace_shutdown_sysctl(void)
@@ -679,13 +657,11 @@ static void ftrace_shutdown_sysctl(void)
679657
if (unlikely(ftrace_disabled))
680658
return;
681659

682-
mutex_lock(&ftrace_start_lock);
683660
/* ftrace_start_up is true if ftrace is running */
684661
if (ftrace_start_up)
685662
command |= FTRACE_DISABLE_CALLS;
686663

687664
ftrace_run_update_code(command);
688-
mutex_unlock(&ftrace_start_lock);
689665
}
690666

691667
static cycle_t ftrace_update_time;
@@ -1502,12 +1478,10 @@ ftrace_regex_release(struct inode *inode, struct file *file, int enable)
15021478
ftrace_match_records(iter->buffer, iter->buffer_idx, enable);
15031479
}
15041480

1505-
mutex_lock(&ftrace_sysctl_lock);
1506-
mutex_lock(&ftrace_start_lock);
1481+
mutex_lock(&ftrace_lock);
15071482
if (ftrace_start_up && ftrace_enabled)
15081483
ftrace_run_update_code(FTRACE_ENABLE_CALLS);
1509-
mutex_unlock(&ftrace_start_lock);
1510-
mutex_unlock(&ftrace_sysctl_lock);
1484+
mutex_unlock(&ftrace_lock);
15111485

15121486
kfree(iter);
15131487
mutex_unlock(&ftrace_regex_lock);
@@ -1824,7 +1798,7 @@ static int ftrace_convert_nops(struct module *mod,
18241798
unsigned long addr;
18251799
unsigned long flags;
18261800

1827-
mutex_lock(&ftrace_start_lock);
1801+
mutex_lock(&ftrace_lock);
18281802
p = start;
18291803
while (p < end) {
18301804
addr = ftrace_call_adjust(*p++);
@@ -1843,7 +1817,7 @@ static int ftrace_convert_nops(struct module *mod,
18431817
local_irq_save(flags);
18441818
ftrace_update_code(mod);
18451819
local_irq_restore(flags);
1846-
mutex_unlock(&ftrace_start_lock);
1820+
mutex_unlock(&ftrace_lock);
18471821

18481822
return 0;
18491823
}
@@ -2016,7 +1990,7 @@ ftrace_pid_write(struct file *filp, const char __user *ubuf,
20161990
if (ret < 0)
20171991
return ret;
20181992

2019-
mutex_lock(&ftrace_start_lock);
1993+
mutex_lock(&ftrace_lock);
20201994
if (val < 0) {
20211995
/* disable pid tracing */
20221996
if (!ftrace_pid_trace)
@@ -2055,7 +2029,7 @@ ftrace_pid_write(struct file *filp, const char __user *ubuf,
20552029
ftrace_startup_enable(0);
20562030

20572031
out:
2058-
mutex_unlock(&ftrace_start_lock);
2032+
mutex_unlock(&ftrace_lock);
20592033

20602034
return cnt;
20612035
}
@@ -2118,12 +2092,12 @@ int register_ftrace_function(struct ftrace_ops *ops)
21182092
if (unlikely(ftrace_disabled))
21192093
return -1;
21202094

2121-
mutex_lock(&ftrace_sysctl_lock);
2095+
mutex_lock(&ftrace_lock);
21222096

21232097
ret = __register_ftrace_function(ops);
21242098
ftrace_startup(0);
21252099

2126-
mutex_unlock(&ftrace_sysctl_lock);
2100+
mutex_unlock(&ftrace_lock);
21272101
return ret;
21282102
}
21292103

@@ -2137,10 +2111,10 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
21372111
{
21382112
int ret;
21392113

2140-
mutex_lock(&ftrace_sysctl_lock);
2114+
mutex_lock(&ftrace_lock);
21412115
ret = __unregister_ftrace_function(ops);
21422116
ftrace_shutdown(0);
2143-
mutex_unlock(&ftrace_sysctl_lock);
2117+
mutex_unlock(&ftrace_lock);
21442118

21452119
return ret;
21462120
}
@@ -2155,7 +2129,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
21552129
if (unlikely(ftrace_disabled))
21562130
return -ENODEV;
21572131

2158-
mutex_lock(&ftrace_sysctl_lock);
2132+
mutex_lock(&ftrace_lock);
21592133

21602134
ret = proc_dointvec(table, write, file, buffer, lenp, ppos);
21612135

@@ -2184,7 +2158,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
21842158
}
21852159

21862160
out:
2187-
mutex_unlock(&ftrace_sysctl_lock);
2161+
mutex_unlock(&ftrace_lock);
21882162
return ret;
21892163
}
21902164

@@ -2296,7 +2270,7 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,
22962270
{
22972271
int ret = 0;
22982272

2299-
mutex_lock(&ftrace_sysctl_lock);
2273+
mutex_lock(&ftrace_lock);
23002274

23012275
ftrace_suspend_notifier.notifier_call = ftrace_suspend_notifier_call;
23022276
register_pm_notifier(&ftrace_suspend_notifier);
@@ -2314,21 +2288,21 @@ int register_ftrace_graph(trace_func_graph_ret_t retfunc,
23142288
ftrace_startup(FTRACE_START_FUNC_RET);
23152289

23162290
out:
2317-
mutex_unlock(&ftrace_sysctl_lock);
2291+
mutex_unlock(&ftrace_lock);
23182292
return ret;
23192293
}
23202294

23212295
void unregister_ftrace_graph(void)
23222296
{
2323-
mutex_lock(&ftrace_sysctl_lock);
2297+
mutex_lock(&ftrace_lock);
23242298

23252299
atomic_dec(&ftrace_graph_active);
23262300
ftrace_graph_return = (trace_func_graph_ret_t)ftrace_stub;
23272301
ftrace_graph_entry = ftrace_graph_entry_stub;
23282302
ftrace_shutdown(FTRACE_STOP_FUNC_RET);
23292303
unregister_pm_notifier(&ftrace_suspend_notifier);
23302304

2331-
mutex_unlock(&ftrace_sysctl_lock);
2305+
mutex_unlock(&ftrace_lock);
23322306
}
23332307

23342308
/* Allocate a return stack for newly created task */

0 commit comments

Comments
 (0)