lkml.org 
[lkml]   [2007]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/24] make atomic_read() behave consistently across all architectures


    On Fri, 17 Aug 2007, Paul Mackerras wrote:

    > Herbert Xu writes:
    >
    > > On Fri, Aug 17, 2007 at 03:09:57PM +1000, Paul Mackerras wrote:
    > > > Herbert Xu writes:
    > > >
    > > > > Can you find an actual atomic_read code snippet there that is
    > > > > broken without the volatile modifier?
    > > >
    > > > There are some in arch-specific code, for example line 1073 of
    > > > arch/mips/kernel/smtc.c. On mips, cpu_relax() is just barrier(), so
    > > > the empty loop body is ok provided that atomic_read actually does the
    > > > load each time around the loop.
    > >
    > > A barrier() is all you need to force the compiler to reread
    > > the value.
    > >
    > > The people advocating volatile in this thread are talking
    > > about code that doesn't use barrier()/cpu_relax().
    >
    > Did you look at it? Here it is:
    >
    > /* Someone else is initializing in parallel - let 'em finish */
    > while (atomic_read(&idle_hook_initialized) < 1000)
    > ;


    Honestly, this thread is suffering from HUGE communication gaps.

    What Herbert (obviously) meant there was that "this loop could've
    been okay _without_ using volatile-semantics-atomic_read() also, if
    only it used cpu_relax()".

    That does work, because cpu_relax() is _at least_ barrier() on all
    archs (on some it also emits some arch-dependent "pause" kind of
    instruction).

    Now, saying that "MIPS does not have such an instruction so I won't
    use cpu_relax() for arch-dependent-busy-while-loops in arch/mips/"
    sounds like a wrong argument, because: tomorrow, such arch's _may_
    introduce such an instruction, so naturally, at that time we'd
    change cpu_relax() appropriately (in reality, we would actually
    *re-define* cpu_relax() and ensure that the correct version gets
    pulled in depending on whether the callsite code is legacy or only
    for the newer such CPUs of said arch, whatever), but loops such as
    this would remain un-changed, because they never used cpu_relax()!

    OTOH an argument that said the following would've made a stronger case:

    "I don't want to use cpu_relax() because that's a full memory
    clobber barrier() and I have loop-invariants / other variables
    around in that code that I *don't* want the compiler to forget
    just because it used cpu_relax(), and hence I will not use
    cpu_relax() but instead make my atomic_read() itself have
    "volatility" semantics. Not just that, but I will introduce a
    cpu_relax_no_barrier() on MIPS, that would be a no-op #define
    for now, but which may not be so forever, and continue to use
    that in such busy loops."

    In general, please read the thread-summary I've tried to do at:
    http://lkml.org/lkml/2007/8/17/25
    Feel free to continue / comment / correct stuff from there, there's
    too much confusion and circular-arguments happening on this thread
    otherwise.

    [ I might've made an incorrect statement there about
    "volatile" w.r.t. cache on non-x86 archs, I think. ]


    Satyam
    -
    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-17 10:19    [W:4.090 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site