[PATCH 0/2] ftrace updates for 2.6.28 - Kernel

This is a discussion on [PATCH 0/2] ftrace updates for 2.6.28 - Kernel ; Stephen and Ingo, The following patches are needed for 2.6.28. The first one solves a bug where the resizing of the buffer array was not protected against modifications by ftrace. The second patch solves an issue tha Thomas Gleixner was ...

+ Reply to Thread
Results 1 to 6 of 6

Thread: [PATCH 0/2] ftrace updates for 2.6.28

  1. [PATCH 0/2] ftrace updates for 2.6.28


    Stephen and Ingo,

    The following patches are needed for 2.6.28. The first one solves a bug
    where the resizing of the buffer array was not protected against
    modifications by ftrace.

    The second patch solves an issue tha Thomas Gleixner was seeing where
    the tracer would detect the infinite recursion too easily by
    tracing the timer code. This patch prevents the loop from happening
    and still does not lose any trace entries.

    The changes caused by these patches are low in risk, but I would
    still like to put them through linux-next before passing them
    off to Linus.

    Note, these changes are based against Linus's mainline and not tip.
    Also note that the branch in this tree is "devel" and not "tip/devel".

    The following patches are in:

    git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git

    branch: devel


    Steven Rostedt (2):
    ftrace: disable tracing on resize
    ring-buffer: prevent infinite looping on time stamping

    ----
    kernel/trace/ring_buffer.c | 2 +-
    kernel/trace/trace.c | 17 ++++++++++++++++-
    2 files changed, 17 insertions(+), 2 deletions(-)
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 1/2] ftrace: disable tracing on resize

    Impact: fix for bug on resize

    This patch addresses the bug found here:

    http://bugzilla.kernel.org/show_bug.cgi?id=11996

    When ftrace converted to the new unified trace buffer, the resizing of
    the buffer was not protected as much as it was originally. If tracing
    is performed while the resize occurs, then the buffer can be corrupted.

    This patch disables all ftrace buffer modifications before a resize
    takes place.

    Signed-off-by: Steven Rostedt
    ---
    kernel/trace/trace.c | 17 ++++++++++++++++-
    1 files changed, 16 insertions(+), 1 deletions(-)

    diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
    index 9f3b478..abfa810 100644
    --- a/kernel/trace/trace.c
    +++ b/kernel/trace/trace.c
    @@ -2676,7 +2676,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    {
    unsigned long val;
    char buf[64];
    - int ret;
    + int ret, cpu;
    struct trace_array *tr = filp->private_data;

    if (cnt >= sizeof(buf))
    @@ -2704,6 +2704,14 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    goto out;
    }

    + /* disable all cpu buffers */
    + for_each_tracing_cpu(cpu) {
    + if (global_trace.data[cpu])
    + atomic_inc(&global_trace.data[cpu]->disabled);
    + if (max_tr.data[cpu])
    + atomic_inc(&max_tr.data[cpu]->disabled);
    + }
    +
    if (val != global_trace.entries) {
    ret = ring_buffer_resize(global_trace.buffer, val);
    if (ret < 0) {
    @@ -2735,6 +2743,13 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    if (tracing_disabled)
    cnt = -ENOMEM;
    out:
    + for_each_tracing_cpu(cpu) {
    + if (global_trace.data[cpu])
    + atomic_dec(&global_trace.data[cpu]->disabled);
    + if (max_tr.data[cpu])
    + atomic_dec(&max_tr.data[cpu]->disabled);
    + }
    +
    max_tr.entries = global_trace.entries;
    mutex_unlock(&trace_types_lock);

    --
    1.5.6.5

    --
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH 2/2] ring-buffer: prevent infinite looping on time stamping

    Impact: removal of unnecessary looping

    The lockless part of the ring buffer allows for reentry into the code
    from interrupts. A timestamp is taken, a test is preformed and if it
    detects that an interrupt occurred that did tracing, it tries again.

    The problem arises if the timestamp code itself causes a trace.
    The detection will detect this and loop again. The difference between
    this and an interrupt doing tracing, is that this will fail every time,
    and cause an infinite loop.

    Currently, we test if the loop happens 1000 times, and if so, it will
    produce a warning and disable the ring buffer.

    The problem with this approach is that it makes it difficult to perform
    some types of tracing (tracing the timestamp code itself).

    Each trace entry has a delta timestamp from the previous entry.
    If a trace entry is reserved but and interrupt occurs and traces before
    the previous entry is commited, the delta timestamp for that entry will
    be zero. This actually makes sense in terms of tracing, because the
    interrupt entry happened before the preempted entry was commited, so
    one may consider the two happening at the same time. The order is
    still preserved in the buffer.

    With this idea, instead of trying to get a new timestamp if an interrupt
    made it in between the timestamp and the test, the entry could simply
    make the delta zero and continue. This will prevent interrupts or
    tracers in the timer code from causing the above loop.

    Signed-off-by: Steven Rostedt
    ---
    kernel/trace/ring_buffer.c | 2 +-
    1 files changed, 1 insertions(+), 1 deletions(-)

    diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
    index 3f33806..2f76193 100644
    --- a/kernel/trace/ring_buffer.c
    +++ b/kernel/trace/ring_buffer.c
    @@ -1060,7 +1060,7 @@ rb_reserve_next_event(struct ring_buffer_per_cpu *cpu_buffer,

    /* Did the write stamp get updated already? */
    if (unlikely(ts < cpu_buffer->write_stamp))
    - goto again;
    + delta = 0;

    if (test_time_stamp(delta)) {

    --
    1.5.6.5

    --
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 1/2] ftrace: disable tracing on resize


    Pekka,

    I forgot to add you to the Cc list. Could you test this patch to see if it
    fixes the bug you reported. It fixed a bug I was able to reproduce, but I
    was never able to reproduce the exact same bug you have seen.

    -- Steve


    On Mon, 10 Nov 2008, Steven Rostedt wrote:

    > Impact: fix for bug on resize
    >
    > This patch addresses the bug found here:
    >
    > http://bugzilla.kernel.org/show_bug.cgi?id=11996
    >
    > When ftrace converted to the new unified trace buffer, the resizing of
    > the buffer was not protected as much as it was originally. If tracing
    > is performed while the resize occurs, then the buffer can be corrupted.
    >
    > This patch disables all ftrace buffer modifications before a resize
    > takes place.
    >
    > Signed-off-by: Steven Rostedt
    > ---
    > kernel/trace/trace.c | 17 ++++++++++++++++-
    > 1 files changed, 16 insertions(+), 1 deletions(-)
    >
    > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
    > index 9f3b478..abfa810 100644
    > --- a/kernel/trace/trace.c
    > +++ b/kernel/trace/trace.c
    > @@ -2676,7 +2676,7 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    > {
    > unsigned long val;
    > char buf[64];
    > - int ret;
    > + int ret, cpu;
    > struct trace_array *tr = filp->private_data;
    >
    > if (cnt >= sizeof(buf))
    > @@ -2704,6 +2704,14 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    > goto out;
    > }
    >
    > + /* disable all cpu buffers */
    > + for_each_tracing_cpu(cpu) {
    > + if (global_trace.data[cpu])
    > + atomic_inc(&global_trace.data[cpu]->disabled);
    > + if (max_tr.data[cpu])
    > + atomic_inc(&max_tr.data[cpu]->disabled);
    > + }
    > +
    > if (val != global_trace.entries) {
    > ret = ring_buffer_resize(global_trace.buffer, val);
    > if (ret < 0) {
    > @@ -2735,6 +2743,13 @@ tracing_entries_write(struct file *filp, const char __user *ubuf,
    > if (tracing_disabled)
    > cnt = -ENOMEM;
    > out:
    > + for_each_tracing_cpu(cpu) {
    > + if (global_trace.data[cpu])
    > + atomic_dec(&global_trace.data[cpu]->disabled);
    > + if (max_tr.data[cpu])
    > + atomic_dec(&max_tr.data[cpu]->disabled);
    > + }
    > +
    > max_tr.entries = global_trace.entries;
    > mutex_unlock(&trace_types_lock);
    >
    > --
    > 1.5.6.5
    >
    > --
    >
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 0/2] ftrace updates for 2.6.28


    On Mon, 10 Nov 2008, Steven Rostedt wrote:
    >
    > The following patches are in:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
    >
    > branch: devel
    >
    >
    > Steven Rostedt (2):
    > ftrace: disable tracing on resize
    > ring-buffer: prevent infinite looping on time stamping
    >


    Ingo,

    I just pushed these two patches to tip/devel after solving a conflict with
    the first one.

    -- Steve

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 0/2] ftrace updates for 2.6.28


    * Steven Rostedt wrote:

    > The following patches are in:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
    >
    > branch: devel
    >
    > Steven Rostedt (2):
    > ftrace: disable tracing on resize
    > ring-buffer: prevent infinite looping on time stamping
    >
    > ----
    > kernel/trace/ring_buffer.c | 2 +-
    > kernel/trace/trace.c | 17 ++++++++++++++++-
    > 2 files changed, 17 insertions(+), 2 deletions(-)


    pulled into tip/tracing/urgent, thanks Steve!

    > The following patches are needed for 2.6.28. The first one solves a
    > bug where the resizing of the buffer array was not protected against
    > modifications by ftrace.
    >
    > The second patch solves an issue tha Thomas Gleixner was seeing
    > where the tracer would detect the infinite recursion too easily by
    > tracing the timer code. This patch prevents the loop from happening
    > and still does not lose any trace entries.
    >
    > The changes caused by these patches are low in risk, but I would
    > still like to put them through linux-next before passing them off to
    > Linus.
    >
    > Note, these changes are based against Linus's mainline and not tip.
    > Also note that the branch in this tree is "devel" and not
    > "tip/devel".


    that's OK. I resolved the (trivial) conflict with pending changes in
    tip/tracing/ftrace and pushed out a new ftrace integration branch
    towards linux-next.

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread