lkml.org 
[lkml]   [2010]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH] asm-generic: Use raw_local_irq_save/restore instead local_irq_save/restore
    Andrew Morton wrote:
    > On Wed, 26 May 2010 10:42:43 -0400 Steven Rostedt <rostedt@goodmis.org> wrote:
    >
    >> On Wed, 2010-05-26 at 10:48 +0200, monstr@monstr.eu wrote:
    >>> From: Michal Simek <monstr@monstr.eu>
    >>>
    >>> start/stop_critical_timing function for preemptirqsoff, preemptoff
    >>> and irqsoff tracers contains atomic_inc and atomic_dec operations.
    >>>
    >>> Atomic operations used local_irq_save/restore macros to ensure
    >>> atomic access but they are traced by the same function which is causing
    >>> recursion problem.
    >>>
    >>> The reason is when these tracers are turn ON then local_irq_save/restore
    >>> macros are changed in include/linux/irqflags.h to call trace_hardirqs_on/off
    >>> which call start/stop_critical_timing.
    >>>
    >>> Microblaze was affected because use generic atomic implementation.
    >>>
    >>> Signed-off-by: Michal Simek <monstr@monstr.eu>
    >> Acked-by: Steven Rostedt <rostedt@goodmis.org>
    >>
    >
    > Sighed-at-by: me.
    >
    >>> diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
    >>> index 058129e..6c190fd 100644
    >>> --- a/include/asm-generic/atomic.h
    >>> +++ b/include/asm-generic/atomic.h
    >>> @@ -57,11 +57,11 @@ static inline int atomic_add_return(int i, atomic_t *v)
    >>> unsigned long flags;
    >>> int temp;
    >>>
    >>> - local_irq_save(flags);
    >>> + raw_local_irq_save(flags);
    >>> temp = v->counter;
    >>> temp += i;
    >>> v->counter = temp;
    >>> - local_irq_restore(flags);
    >>> + raw_local_irq_restore(flags);
    >>>
    >>> return temp;
    >>> }
    >
    > If a developer looks at atomic_add_return() and asks himself "why did
    > this use raw_local_irq_save()", the only way of answering that question
    > is to go groveling through the git logs, which is a right PITA if
    > you're trying to get some coding work done.
    >
    > Guys, any time you add code which is non-obvious at the raw C level, it
    > *needs* a comment!

    Andrew: Can you please add this patch to your tree? I have it in
    microblaze next branch but should go through different tree.

    Thanks,
    Michal

    --
    Michal Simek, Ing. (M.Eng)
    w: www.monstr.eu p: +42-0-721842854
    Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
    Microblaze U-BOOT custodian
    From 06c2447c8939227b4c4d3340fba1c712f03e70ca Mon Sep 17 00:00:00 2001
    From: Michal Simek <monstr@monstr.eu>
    Date: Wed, 26 May 2010 08:57:36 +0200
    Subject: [PATCH] asm-generic: Use raw_local_irq_save/restore instead local_irq_save/restore

    start/stop_critical_timing function for preemptirqsoff, preemptoff
    and irqsoff tracers contains atomic_inc and atomic_dec operations.

    Atomic operations used local_irq_save/restore macros to ensure
    atomic access but they are traced by the same function which is causing
    recursion problem.

    The reason is when these tracers are turn ON then local_irq_save/restore
    macros are changed in include/linux/irqflags.h to call trace_hardirqs_on/off
    which call start/stop_critical_timing.

    Microblaze was affected because use generic atomic implementation.

    Signed-off-by: Michal Simek <monstr@monstr.eu>
    Acked-by: Steven Rostedt <rostedt@goodmis.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    ---
    include/asm-generic/atomic.h | 12 ++++++------
    1 files changed, 6 insertions(+), 6 deletions(-)

    diff --git a/include/asm-generic/atomic.h b/include/asm-generic/atomic.h
    index 058129e..e53347f 100644
    --- a/include/asm-generic/atomic.h
    +++ b/include/asm-generic/atomic.h
    @@ -57,11 +57,11 @@ static inline int atomic_add_return(int i, atomic_t *v)
    unsigned long flags;
    int temp;

    - local_irq_save(flags);
    + raw_local_irq_save(flags); /* Don't trace it in a irqsoff handler */
    temp = v->counter;
    temp += i;
    v->counter = temp;
    - local_irq_restore(flags);
    + raw_local_irq_restore(flags);

    return temp;
    }
    @@ -78,11 +78,11 @@ static inline int atomic_sub_return(int i, atomic_t *v)
    unsigned long flags;
    int temp;

    - local_irq_save(flags);
    + raw_local_irq_save(flags); /* Don't trace it in a irqsoff handler */
    temp = v->counter;
    temp -= i;
    v->counter = temp;
    - local_irq_restore(flags);
    + raw_local_irq_restore(flags);

    return temp;
    }
    @@ -135,9 +135,9 @@ static inline void atomic_clear_mask(unsigned long mask, unsigned long *addr)
    unsigned long flags;

    mask = ~mask;
    - local_irq_save(flags);
    + raw_local_irq_save(flags); /* Don't trace it in a irqsoff handler */
    *addr &= mask;
    - local_irq_restore(flags);
    + raw_local_irq_restore(flags);
    }

    #define atomic_xchg(ptr, v) (xchg(&(ptr)->counter, (v)))
    --
    1.5.5.6
    \
     
     \ /
      Last update: 2010-07-26 10:55    [W:0.033 / U:90.952 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site