lkml.org 
[lkml]   [2006]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Export current_is_keventd() for libphy


    On Wed, 6 Dec 2006, Linus Torvalds wrote:
    >
    > Sadly, gcc doesn't do it in this case. Still, I'd rather keep the source
    > clean, and hope that the compiler improves eventually, than to make the
    > code uglier.

    Actually, it's our own damn fault.

    We've long had our arguments "const volatile" to test_bit(), which
    basically means that gcc can't do the optimization. Damn.

    And they need to be "volatile" not because we _want_ the thing to be
    volaile, but because these things are occationally used on volatile
    objects (so if the function doesn't take a volatile pointer, it would
    warn).

    That's why so many of these helper functions use "const volatile"
    pointers, which on the face of it would seem strange if you don't realize
    that it's more about a C type issue than about the _actual_ type being
    either "const" or "volatile".

    Sad. I guess we could remove the "const volatile" from the _cast_, but the
    thing is, if the underlying object we're actually looking at really _is_
    volatile, we shouldn't do that. GAAH.

    Really sad. I doubt any of the callers actually want the "volatile" access
    at all. Things like <linux/page-flags.h> definitely _don't_ want it.

    I suspect we should just face up to the fact that

    (a) "volatile" on kernel data is basically always a bug, and you should
    use locking. "volatile" doesn't help anything at all with memory
    ordering and friends, so it's insane to think it "solves" anything on
    its own.
    (b) on "iomem" pointers it does make sense, but those need special
    accessor functions _anyway_, so things like test_bit() wouldn't work
    on them.
    (c) if you spin on a value changing, you should use "cpu_relax()" or
    "barrier()" anyway, which will force gcc to re-load any values from
    memory over the loop.

    and remove the "volatile" from all the bitop accessor functions.

    Or at least from "test_bit()". It's not like it's synchronous _anyway_
    (there's no memory barriers etc).

    Hmm? Comments? linux-arch added to Cc, just in case people care (this
    particular thing is in <asm-*/bitops.h>, so it _is_ architecture-
    specific, but we should probably all agree on it first)

    Linus
    -
    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: 2009-11-18 23:46    [W:2.143 / U:0.140 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site