lkml.org 
[lkml]   [2007]   [Aug]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] i386: Fix a couple busy loops in mach_wakecpu.h:wait_for_init_deassert()
    Date
    On Friday 24 August 2007 13:12, Kenn Humborg wrote:
    > > On Thursday 16 August 2007 01:39, Satyam Sharma wrote:
    > > > static inline void wait_for_init_deassert(atomic_t *deassert)
    > > > {
    > > > - while (!atomic_read(deassert));
    > > > + while (!atomic_read(deassert))
    > > > + cpu_relax();
    > > > return;
    > > > }
    > >
    > > For less-than-briliant people like me, it's totally non-obvious that
    > > cpu_relax() is needed for correctness here, not just to make P4 happy.
    > >
    > > IOW: "atomic_read" name quite unambiguously means "I will read
    > > this variable from main memory". Which is not true and creates
    > > potential for confusion and bugs.
    >
    > To me, "atomic_read" means a read which is synchronized with other
    > changes to the variable (using the atomic_XXX functions) in such
    > a way that I will always only see the "before" or "after"
    > state of the variable - never an intermediate state while a
    > modification is happening. It doesn't imply that I have to
    > see the "after" state immediately after another thread modifies
    > it.

    So you are ok with compiler propagating n1 to n2 here:

    n1 += atomic_read(x);
    other_variable++;
    n2 += atomic_read(x);

    without accessing x second time. What's the point? Any sane coder
    will say that explicitly anyway:

    tmp = atomic_read(x);
    n1 += tmp;
    other_variable++;
    n2 += tmp;

    if only for the sake of code readability. Because first code
    is definitely hinting that it reads RAM twice, and it's actively *bad*
    for code readability when in fact it's not the case!

    Locking, compiler and CPU barriers are complicated enough already,
    please don't make them even harder to understand.
    --
    vda
    -
    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/

    \
     
     \ /
      Last update: 2007-08-24 16:29    [W:0.025 / U:0.104 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site