Skip to content

Commit 083e9f6

Browse files
committed
ring-buffer: Remove useless update to write_stamp in rb_try_to_discard()
When filtering is enabled, a temporary buffer is created to place the content of the trace event output so that the filter logic can decide from the trace event output if the trace event should be filtered out or not. If it is to be filtered out, the content in the temporary buffer is simply discarded, otherwise it is written into the trace buffer. But if an interrupt were to come in while a previous event was using that temporary buffer, the event written by the interrupt would actually go into the ring buffer itself to prevent corrupting the data on the temporary buffer. If the event is to be filtered out, the event in the ring buffer is discarded, or if it fails to discard because another event were to have already come in, it is turned into padding. The update to the write_stamp in the rb_try_to_discard() happens after a fix was made to force the next event after the discard to use an absolute timestamp by setting the before_stamp to zero so it does not match the write_stamp (which causes an event to use the absolute timestamp). But there's an effort in rb_try_to_discard() to put back the write_stamp to what it was before the event was added. But this is useless and wasteful because nothing is going to be using that write_stamp for calculations as it still will not match the before_stamp. Remove this useless update, and in doing so, we remove another cmpxchg64()! Also update the comments to reflect this change as well as remove some extra white space in another comment. Link: https://lore.kernel.org/linux-trace-kernel/[email protected] Cc: Masami Hiramatsu <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Joel Fernandes <[email protected]> Cc: Vincent Donnefort <[email protected]> Fixes: b2dd797 ("ring-buffer: Force absolute timestamp on discard of event") Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent dd93942 commit 083e9f6

File tree

1 file changed

+11
-36
lines changed

1 file changed

+11
-36
lines changed

kernel/trace/ring_buffer.c

Lines changed: 11 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2983,34 +2983,13 @@ static unsigned rb_calculate_event_length(unsigned length)
29832983
return length;
29842984
}
29852985

2986-
static u64 rb_time_delta(struct ring_buffer_event *event)
2987-
{
2988-
switch (event->type_len) {
2989-
case RINGBUF_TYPE_PADDING:
2990-
return 0;
2991-
2992-
case RINGBUF_TYPE_TIME_EXTEND:
2993-
return rb_event_time_stamp(event);
2994-
2995-
case RINGBUF_TYPE_TIME_STAMP:
2996-
return 0;
2997-
2998-
case RINGBUF_TYPE_DATA:
2999-
return event->time_delta;
3000-
default:
3001-
return 0;
3002-
}
3003-
}
3004-
30052986
static inline bool
30062987
rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer,
30072988
struct ring_buffer_event *event)
30082989
{
30092990
unsigned long new_index, old_index;
30102991
struct buffer_page *bpage;
30112992
unsigned long addr;
3012-
u64 write_stamp;
3013-
u64 delta;
30142993

30152994
new_index = rb_event_index(event);
30162995
old_index = new_index + rb_event_ts_length(event);
@@ -3019,14 +2998,10 @@ rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer,
30192998

30202999
bpage = READ_ONCE(cpu_buffer->tail_page);
30213000

3022-
delta = rb_time_delta(event);
3023-
3024-
if (!rb_time_read(&cpu_buffer->write_stamp, &write_stamp))
3025-
return false;
3026-
3027-
/* Make sure the write stamp is read before testing the location */
3028-
barrier();
3029-
3001+
/*
3002+
* Make sure the tail_page is still the same and
3003+
* the next write location is the end of this event
3004+
*/
30303005
if (bpage->page == (void *)addr && rb_page_write(bpage) == old_index) {
30313006
unsigned long write_mask =
30323007
local_read(&bpage->write) & ~RB_WRITE_MASK;
@@ -3037,20 +3012,20 @@ rb_try_to_discard(struct ring_buffer_per_cpu *cpu_buffer,
30373012
* to make sure that the next event adds an absolute
30383013
* value and does not rely on the saved write stamp, which
30393014
* is now going to be bogus.
3015+
*
3016+
* By setting the before_stamp to zero, the next event
3017+
* is not going to use the write_stamp and will instead
3018+
* create an absolute timestamp. This means there's no
3019+
* reason to update the wirte_stamp!
30403020
*/
30413021
rb_time_set(&cpu_buffer->before_stamp, 0);
30423022

3043-
/* Something came in, can't discard */
3044-
if (!rb_time_cmpxchg(&cpu_buffer->write_stamp,
3045-
write_stamp, write_stamp - delta))
3046-
return false;
3047-
30483023
/*
30493024
* If an event were to come in now, it would see that the
30503025
* write_stamp and the before_stamp are different, and assume
30513026
* that this event just added itself before updating
30523027
* the write stamp. The interrupting event will fix the
3053-
* write stamp for us, and use the before stamp as its delta.
3028+
* write stamp for us, and use an absolute timestamp.
30543029
*/
30553030

30563031
/*
@@ -3487,7 +3462,7 @@ static void check_buffer(struct ring_buffer_per_cpu *cpu_buffer,
34873462
return;
34883463

34893464
/*
3490-
* If this interrupted another event,
3465+
* If this interrupted another event,
34913466
*/
34923467
if (atomic_inc_return(this_cpu_ptr(&checking)) != 1)
34933468
goto out;

0 commit comments

Comments
 (0)