lkml.org 
[lkml]   [2021]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] LKMM: Add volatile_if()
    On Sat, Jun 05, 2021 at 10:57:39AM -0400, Alan Stern wrote:
    > On Fri, Jun 04, 2021 at 03:19:11PM -0700, Linus Torvalds wrote:
    > > Now, part of this is that I do think that in *general* we should never
    > > use this very suble load-cond-store pattern to begin with. We should
    > > strive to use more smp_load_acquire() and smp_store_release() if we
    > > care about ordering of accesses. They are typically cheap enough, and
    > > if there's much of an ordering issue, they are the right things to do.
    > >
    > > I think the whole "load-to-store ordering" subtle non-ordered case is
    > > for very very special cases, when you literally don't have a general
    > > memory ordering, you just have an ordering for *one* very particular
    > > access. Like some of the very magical code in the rw-semaphore case,
    > > or that smp_cond_load_acquire().
    > >
    > > IOW, I would expect that we have a handful of uses of this thing. And
    > > none of them have that "the conditional store is the same on both
    > > sides" pattern, afaik.
    > >
    > > And immediately when the conditional store is different, you end up
    > > having a dependency on it that orders it.
    > >
    > > But I guess I can accept the above made-up example as an "argument",
    > > even though I feel it is entirely irrelevant to the actual issues and
    > > uses we have.
    >
    > Indeed, the expansion of the currently proposed version of
    >
    > volatile_if (A) {
    > B;
    > } else {
    > C;
    > }
    >
    > is basically the same as
    >
    > if (A) {
    > barrier();
    > B;
    > } else {
    > barrier();
    > C;
    > }
    >
    > which is just about as easy to write by hand. (For some reason my
    > fingers don't like typing "volatile_"; the letters tend to get
    > scrambled.)
    >
    > So given that:
    >
    > 1. Reliance on control dependencies is uncommon in the kernel,
    > and
    >
    > 2. The loads in A could just be replaced with load_acquires
    > at a low penalty (or store-releases could go into B and C),
    >
    > it seems that we may not need volatile_if at all! The only real reason
    > for having it in the first place was to avoid the penalty of
    > load-acquire on architectures where it has a significant cost, when the
    > control dependency would provide the necessary ordering for free. Such
    > architectures are getting less and less common.

    That does sound good, but...

    Current compilers beg to differ at -O2: https://godbolt.org/z/5K55Gardn

    ------------------------------------------------------------------------
    #define READ_ONCE(x) (*(volatile typeof(x) *)&(x))
    #define WRITE_ONCE(x, val) (READ_ONCE(x) = (val))
    #define barrier() __asm__ __volatile__("": : :"memory")

    int x, y;

    int main(int argc, char *argv[])
    {
    if (READ_ONCE(x)) {
    barrier();
    WRITE_ONCE(y, 1);
    } else {
    barrier();
    WRITE_ONCE(y, 1);
    }
    return 0;
    }
    ------------------------------------------------------------------------

    Both gcc and clang generate a load followed by a store, with no branch.
    ARM gets the same results from both compilers.

    As Linus suggested, removing one (but not both!) invocations of barrier()
    does cause a branch to be emitted, so maybe that is a way forward.
    Assuming it is more than just dumb luck, anyway. :-/

    Thanx, Paul

    \
     
     \ /
      Last update: 2021-06-06 02:15    [W:4.542 / U:0.244 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site