mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Alexandre Oliva <aoliva@redhat.com>
To: linux-kernel@vger.kernel.org
Cc: akpm@osdl.org, torvalds@osdl.org, arjanv@redhat.com, davej@redhat.com
Subject: raid 5 with >= 5 members broken on x86
Date: 26 Feb 2004 15:36:10 -0300	[thread overview]
Message-ID: <orznb5leqs.fsf@free.redhat.lsd.ic.unicamp.br> (raw)

[Apologies if this is a dup (or a trip :-); I seem to be having
 trouble getting through the list spam filtering.  Too bad the failure
 is silent.]

There was a bug in the inline asm used to implement xor_p5_mmx_5.  It
had pushes and pops although it took a +g operand.  It turned out
that gcc chose to pass `lines' as an %esp+offset address.  With
changes in esp, it wouldn't work.

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.

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.

Here's the patch.  More details at
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=116679

Here's the patch.

--- include/asm-i386/xor.h.orig	2004-02-26 15:13:03.000000000 -0300
+++ include/asm-i386/xor.h	2004-02-26 15:18:20.000000000 -0300
@@ -425,10 +425,28 @@
 
 	kernel_fpu_begin();
 
-	/* need to save p4/p5 manually to not exceed gcc's 10 argument limit */
+	/* GCC up to 3.0.x had this limitation of at most 10 asm
+	   operands, and + operands counted as two (one input and one
+	   output).  We unfortunately have to inform GCC that we're
+	   modifying all of these registers.  The trick we use here is
+	   to make sure the values of p4 and p5 are dissociated from
+	   whatever other registers or stack locations they might have
+	   been shared with, and make sure GCC knows they've been
+	   modified after the asm statement.  This incurs no
+	   additional costs and AFAICT is safe.
+
+	   Should it be found to not work, we'd have to resort to
+	   passing to the asm statement a pointer to a stack save
+	   area, and use that to preserve registers that had to be
+	   marked as read only but that may have been clobbered, which
+	   would impact performance without any actual advantage.
+	   Note that we can't just push and pop the registers, because
+	   this changes esp, and lines may be passed as an
+	   esp-relative address.  Well, I suppose we could require
+	   lines to be passed in a register...  -aoliva@redhat.com */
+
+	__asm__ ("" : "+r" (p4), "+r" (p5));
 	__asm__ __volatile__ (
-	"	pushl %4\n"
-	"	pushl %5\n"        	
 	" .align 32,0x90             ;\n"
 	" 1:                         ;\n"
 	"       movq   (%1), %%mm0   ;\n"
@@ -487,12 +505,11 @@
 	"       addl $64, %5         ;\n"
 	"       decl %0              ;\n"
 	"       jnz 1b               ;\n"
-	"	popl %5\n"
-	"	popl %4\n"
 	: "+g" (lines),
 	  "+r" (p1), "+r" (p2), "+r" (p3)
 	: "r" (p4), "r" (p5)
 	: "memory");
+	__asm__ __volatile__ ("" : "+r" (p4), "+r" (p5));
 
 	kernel_fpu_end();
 }

-- 
Alexandre Oliva   Enjoy Guarana', see http://www.ic.unicamp.br/~oliva/
Happy GNU Year!                     oliva@{lsd.ic.unicamp.br, gnu.org}
Red Hat GCC Developer                 aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist                Professional serial bug killer

             reply	other threads:[~2004-02-26 21:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-02-26 18:36 Alexandre Oliva [this message]
2004-02-26 21:50 ` Linus Torvalds
2004-02-26 22:04   ` Linus Torvalds
2004-02-26 22:13   ` Alexandre Oliva
2004-02-26 22:32     ` Linus Torvalds
2004-02-26 22:40       ` Alexandre Oliva
2004-02-26 22:57       ` Alexandre Oliva
2004-02-26 23:37       ` Alexandre Oliva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=orznb5leqs.fsf@free.redhat.lsd.ic.unicamp.br \
    --to=aoliva@redhat.com \
    --cc=akpm@osdl.org \
    --cc=arjanv@redhat.com \
    --cc=davej@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®