Messages in this thread | | | Date | Wed, 25 May 2016 09:36:04 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] tracing: Choose static tp_printk buffer by explicit nesting count |
| |
On Wed, 25 May 2016 15:16:40 +0200 Peter Zijlstra <peterz@infradead.org> wrote:
> > static char *get_trace_buf(void) > > { > > + struct trace_buffer_struct *buffer = this_cpu_ptr(trace_percpu_buffer); > > > > + if (!buffer || buffer->nesting >= 4) > > return NULL; > > This is buggy fwiw; you need to unconditionally increment > buffer->nesting to match the unconditional decrement. > > Otherwise 5 'increments' and 5 decrements will land you at -1.
As I said, I did a quick look and haven't reviewed it.
Peter, thanks for looking at it.
> > > > > + return &buffer->buffer[buffer->nesting++][0]; > > +} > > + > > +static void put_trace_buf(void) > > +{ > > + this_cpu_dec(trace_percpu_buffer->nesting); > > } > > So I don't know about tracing; but for perf this construct would not > work 'properly'. > > The per context counter -- which is lost in this scheme -- guards > against in-context recursion. > > Only if we nest from another context do we allow generation of a new > event.
The ring buffer itself has a context check, where if you try to record another event nested in the same context, it will simply return NULL.
But this buffer is only used for trace_printk() to manipulate a printf format. It writes into here first, and then copies it into the tracing ring buffer. If it happens at a nested event within the same context, then trace_buffer_lock_reserve() will return NULL and the event wont be recorded.
-- Steve
| |