lkml.org 
[lkml]   [2008]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC patch 08/18] cnt32_to_63 should use smp_rmb()
    On Fri, 07 Nov 2008 00:23:44 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

    > #define cnt32_to_63(cnt_lo) \
    > ({ \
    > - static volatile u32 __m_cnt_hi; \
    > + static u32 __m_cnt_hi; \
    > union cnt32_to_63 __x; \
    > __x.hi = __m_cnt_hi; \
    > + smp_rmb(); /* read __m_cnt_hi before mmio cnt_lo */ \
    > __x.lo = (cnt_lo); \
    > if (unlikely((s32)(__x.hi ^ __x.lo) < 0)) \
    > __m_cnt_hi = __x.hi = (__x.hi ^ 0x80000000) + (__x.hi >> 31); \

    Oh dear. We have a macro which secretly maintains
    per-instantiation-site global state? And doesn't even implement locking
    to protect that state?

    I mean, the darned thing is called from sched_clock(), which can be
    concurrently called on separate CPUs and which can be called from
    interrupt context (with an arbitrary nesting level!) while it was running
    in process context.

    Who let that thing into Linux?


    Look:

    /*
    * Caller must provide locking to protect *caller_state
    */
    u32 cnt32_to_63(u32 *caller_state, u32 cnt_lo);

    But even that looks pretty crappy.


    \
     
     \ /
      Last update: 2008-11-07 07:11    [W:2.193 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site