Skip to content

Commit 7b40066

Browse files
compudjrostedt
authored andcommitted
tracepoint: Use rcu get state and cond sync for static call updates
State transitions from 1->0->1 and N->2->1 callbacks require RCU synchronization. Rather than performing the RCU synchronization every time the state change occurs, which is quite slow when many tracepoints are registered in batch, instead keep a snapshot of the RCU state on the most recent transitions which belong to a chain, and conditionally wait for a grace period on the last transition of the chain if one g.p. has not elapsed since the last snapshot. This applies to both RCU and SRCU. This brings the performance regression caused by commit 231264d ("Fix: tracepoint: static call function vs data state mismatch") back to what it was originally. Before this commit: # trace-cmd start -e all # time trace-cmd start -p nop real 0m10.593s user 0m0.017s sys 0m0.259s After this commit: # trace-cmd start -e all # time trace-cmd start -p nop real 0m0.878s user 0m0.000s sys 0m0.103s Link: https://lkml.kernel.org/r/20210805192954.30688-1-mathieu.desnoyers@efficios.com Link: https://lore.kernel.org/io-uring/4ebea8f0-58c9-e571-fd30-0ce4f6f09c70@samba.org/ Cc: stable@vger.kernel.org Cc: Ingo Molnar <mingo@redhat.com> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: "Paul E. McKenney" <paulmck@kernel.org> Cc: Stefan Metzmacher <metze@samba.org> Fixes: 231264d ("Fix: tracepoint: static call function vs data state mismatch") Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
1 parent 231264d commit 7b40066

1 file changed

Lines changed: 67 additions & 14 deletions

File tree

kernel/tracepoint.c

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,44 @@ extern tracepoint_ptr_t __stop___tracepoints_ptrs[];
2828
DEFINE_SRCU(tracepoint_srcu);
2929
EXPORT_SYMBOL_GPL(tracepoint_srcu);
3030

31+
enum tp_transition_sync {
32+
TP_TRANSITION_SYNC_1_0_1,
33+
TP_TRANSITION_SYNC_N_2_1,
34+
35+
_NR_TP_TRANSITION_SYNC,
36+
};
37+
38+
struct tp_transition_snapshot {
39+
unsigned long rcu;
40+
unsigned long srcu;
41+
bool ongoing;
42+
};
43+
44+
/* Protected by tracepoints_mutex */
45+
static struct tp_transition_snapshot tp_transition_snapshot[_NR_TP_TRANSITION_SYNC];
46+
47+
static void tp_rcu_get_state(enum tp_transition_sync sync)
48+
{
49+
struct tp_transition_snapshot *snapshot = &tp_transition_snapshot[sync];
50+
51+
/* Keep the latest get_state snapshot. */
52+
snapshot->rcu = get_state_synchronize_rcu();
53+
snapshot->srcu = start_poll_synchronize_srcu(&tracepoint_srcu);
54+
snapshot->ongoing = true;
55+
}
56+
57+
static void tp_rcu_cond_sync(enum tp_transition_sync sync)
58+
{
59+
struct tp_transition_snapshot *snapshot = &tp_transition_snapshot[sync];
60+
61+
if (!snapshot->ongoing)
62+
return;
63+
cond_synchronize_rcu(snapshot->rcu);
64+
if (!poll_state_synchronize_srcu(&tracepoint_srcu, snapshot->srcu))
65+
synchronize_srcu(&tracepoint_srcu);
66+
snapshot->ongoing = false;
67+
}
68+
3169
/* Set to 1 to enable tracepoint debug output */
3270
static const int tracepoint_debug;
3371

@@ -311,6 +349,11 @@ static int tracepoint_add_func(struct tracepoint *tp,
311349
*/
312350
switch (nr_func_state(tp_funcs)) {
313351
case TP_FUNC_1: /* 0->1 */
352+
/*
353+
* Make sure new static func never uses old data after a
354+
* 1->0->1 transition sequence.
355+
*/
356+
tp_rcu_cond_sync(TP_TRANSITION_SYNC_1_0_1);
314357
/* Set static call to first function */
315358
tracepoint_update_call(tp, tp_funcs);
316359
/* Both iterator and static call handle NULL tp->funcs */
@@ -325,10 +368,15 @@ static int tracepoint_add_func(struct tracepoint *tp,
325368
* Requires ordering between RCU assign/dereference and
326369
* static call update/call.
327370
*/
328-
rcu_assign_pointer(tp->funcs, tp_funcs);
329-
break;
371+
fallthrough;
330372
case TP_FUNC_N: /* N->N+1 (N>1) */
331373
rcu_assign_pointer(tp->funcs, tp_funcs);
374+
/*
375+
* Make sure static func never uses incorrect data after a
376+
* N->...->2->1 (N>1) transition sequence.
377+
*/
378+
if (tp_funcs[0].data != old[0].data)
379+
tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1);
332380
break;
333381
default:
334382
WARN_ON_ONCE(1);
@@ -372,31 +420,36 @@ static int tracepoint_remove_func(struct tracepoint *tp,
372420
/* Both iterator and static call handle NULL tp->funcs */
373421
rcu_assign_pointer(tp->funcs, NULL);
374422
/*
375-
* Make sure new func never uses old data after a 1->0->1
376-
* transition sequence.
377-
* Considering that transition 0->1 is the common case
378-
* and don't have rcu-sync, issue rcu-sync after
379-
* transition 1->0 to break that sequence by waiting for
380-
* readers to be quiescent.
423+
* Make sure new static func never uses old data after a
424+
* 1->0->1 transition sequence.
381425
*/
382-
tracepoint_synchronize_unregister();
426+
tp_rcu_get_state(TP_TRANSITION_SYNC_1_0_1);
383427
break;
384428
case TP_FUNC_1: /* 2->1 */
385429
rcu_assign_pointer(tp->funcs, tp_funcs);
386430
/*
387-
* On 2->1 transition, RCU sync is needed before setting
388-
* static call to first callback, because the observer
389-
* may have loaded any prior tp->funcs after the last one
390-
* associated with an rcu-sync.
431+
* Make sure static func never uses incorrect data after a
432+
* N->...->2->1 (N>2) transition sequence. If the first
433+
* element's data has changed, then force the synchronization
434+
* to prevent current readers that have loaded the old data
435+
* from calling the new function.
391436
*/
392-
tracepoint_synchronize_unregister();
437+
if (tp_funcs[0].data != old[0].data)
438+
tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1);
439+
tp_rcu_cond_sync(TP_TRANSITION_SYNC_N_2_1);
393440
/* Set static call to first function */
394441
tracepoint_update_call(tp, tp_funcs);
395442
break;
396443
case TP_FUNC_2: /* N->N-1 (N>2) */
397444
fallthrough;
398445
case TP_FUNC_N:
399446
rcu_assign_pointer(tp->funcs, tp_funcs);
447+
/*
448+
* Make sure static func never uses incorrect data after a
449+
* N->...->2->1 (N>2) transition sequence.
450+
*/
451+
if (tp_funcs[0].data != old[0].data)
452+
tp_rcu_get_state(TP_TRANSITION_SYNC_N_2_1);
400453
break;
401454
default:
402455
WARN_ON_ONCE(1);

0 commit comments

Comments
 (0)