[lkml]   [2004]   [Feb]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: raid 5 with >= 5 members broken on x86
    On Feb 26, 2004, Linus Torvalds <> wrote:

    > On Thu, 26 Feb 2004, Alexandre Oliva wrote:
    >> I suppose I could just change lines from +g to +r, like xor_pII_mmx_5,
    >> but avoiding the pushes and pops is more efficient, and making sure
    >> GCC doesn't get clever about sharing or reusing p4 and p5, it's just
    >> as safe. This approach should probably be extended to the other uses
    >> of push and pop due to limitations in the number of operands.

    > You can't do this in a separate inline asm.

    There's a reason why I added both asms, one with volatile and one
    without. I know what I'm doing. I even tried to explain it in the
    comments. Did you read them? Let me try again.

    + __asm__ ("" : "+r" (p4), "+r" (p5));

    This makes sure GCC no longer knows what's in p4 and p5. They're no
    longer shared with anything they might have been shared with before.
    So, when we read from p4 and p5, we know that, even if p4 and p5 are
    reloaded into some other register, they're not shared with anything
    else. We don't need the above to be volatile because it's ok to
    reorder it with the preparation of the arguments for the asm below, or
    with anything before. The point is only to get rid of any potential
    sharing of value that the variables might have with whatever might
    have been assigned to them before.

    __asm__ __volatile__ (
    : "+g" (lines),
    "+r" (p1), "+r" (p2), "+r" (p3)
    : "r" (p4), "r" (p5)
    : "memory");

    Ok, so we read from p4 and p5, that GCC knew nothing about. GCC
    doesn't know they changed. But that's ok, because immediately after
    this volatile asm, there's another:

    + __asm__ __volatile__ ("" : "+r" (p4), "+r" (p5));

    This one tells GCC: look, I'm clobbering these registers. They no
    longer have the values they used to. Strictly speaking, this is not
    even necessary, since the variables go out of scope at the end of the
    function. But strictly speaking, it's correct, in that it implies GCC
    will make no assumptions that the registers that held the values of p4
    and p5 at the end of the previous asm statement still do.

    So the only possibility of problem would be in case GCC used the
    registers with the modified values of p4 and p5 as addresses for
    output reloads for any of the other operands of the second asm (the
    first volatile one). But GCC can't possibly do this because it has no
    idea of what values p4 and p5 have.

    So it the assembly sequence is strictly correct, even though it
    requires some deep knowledge of the semantics of asm statements to
    conclude that.

    > There is nothing to say that gcc wouldn't do a re-load or something
    > in between, so you really need to tell the _first_ ask about it.

    The only other reload it could do is an input reload of p4 and p5,
    which, again, doesn't matter, because p4 and p5 are dead anyway.
    Should we actually be interested in their values, I very much agree
    with you it wouldn't work. But in this case, we don't need their
    values. We just want to tell GCC it doesn't know what's in those
    variables any more.

    So, it doesn't know what's in the p4 and p5 registers before the asm
    volatile, because of the first non-volatile asm, and it doesn't know
    what's in the variables afterwards, because of the last volatile asm,
    so (i) it won't attempt to reuse the values that are modified in the
    asm even though it doesn't know, and (ii) these registers won't have
    been reused with anything else from before.

    I claim it's safe and correct.

    >> Yet another possibility is to just use +r for p4 and p5; this works in
    >> GCC 3.1 and above. I wasn't sure the kernel was willing to require
    >> that, so I took the most conservative approach.

    > No, I don't think we're ready to force a bigger and slower compiler
    > on x86 for something like this.


    > One approach is to just do the loop _outside_ of the asm?

    IIUC the loop has to be aligned to work as quickly as possible.

    > Btw, the "xor_pII_mmx_5()" thing just uses "+r" for the line count,
    > so why doesn't that work for this case?

    It does. I even said so. But it's slower because of the unnecessary
    pushes and pops. The optimization I propose here could be used for
    xor_pII_mmx_5 as well.

    Alexandre Oliva Enjoy Guarana', see
    Happy GNU Year! oliva@{,}
    Red Hat GCC Developer aoliva@{,}
    Free Software Evangelist Professional serial bug killer
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 14:01    [W:0.027 / U:3.580 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site