Subject: linux 2.4.5: sysmips(MIPS_ATOMIC_SET) is broken (yep, again...)

From: "Maciej W. Rozycki" <[email-masked]>
Date: Thu, 5 Jul 2001 13:14:16 +0200 (MET DST)

Hi,

 Once again a sysmips() patch.  This time the exception fixup code is
broken -- it never returns (well, it probably returns *somewhere* via a
following fixup entry).  Also for whatever reason the R3k code is missing. 

 The following patch fixes it.  While I was at it, I modified contraints a
bit and replaced some of embedded "\t" chars with tabs (the code was
completely unreadable before -- now it should be a bit better, as much as
embedded asm can be).

 The R3k variant works fine for me.  I was unable to thest the ll/sc one,
but the semantics should be unchanged, i.e. if it worked before, so it
will now.  The patch should go into Linus' tree as well. 

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email-masked], PGP key available        +

patch-mips-2.4.5-20010627-sysmips-1
diff -up --recursive --new-file linux-mips-2.4.5-20010627.macro/arch/mips/kernel/sysmips.c linux-mips-2.4.5-20010627/arch/mips/kernel/sysmips.c
--- linux-mips-2.4.5-20010627.macro/arch/mips/kernel/sysmips.c	Mon Apr  9 04:25:42 2001
+++ linux-mips-2.4.5-20010627/arch/mips/kernel/sysmips.c	Wed Jul  4 22:07:30 2001
@@ -75,33 +75,41 @@ sys_sysmips(int cmd, int arg1, int arg2,
 	}
 
 	case MIPS_ATOMIC_SET: {
-#ifdef CONFIG_CPU_HAS_LLSC
-		unsigned int tmp;
+		unsigned int tmp1;
 
 		p = (int *) arg1;
 		errno = verify_area(VERIFY_WRITE, p, sizeof(*p));
 		if (errno)
 			return errno;
-		errno = 0;
 
-		__asm__(".set\tpush\t\t\t# sysmips(MIPS_ATOMIC, ...)\n\t"
-			".set\tmips2\n\t"
-			".set\tnoat\n\t"
-			"1:\tll\t%0, %4\n\t"
-			"move\t$1, %3\n\t"
-			"2:\tsc\t$1, %1\n\t"
-			"beqz\t$1, 1b\n\t"
-			".set\tpop\n\t"
-			".section\t.fixup,\"ax\"\n"
-			"3:\tli\t%2, 1\t\t\t# error\n\t"
+#ifdef CONFIG_CPU_HAS_LLSC
+		__asm__(".set	push	\t\t# sysmips(MIPS_ATOMIC, ...)\n\t"
+			".set	mips2\n"
+			"1:\n\t"
+			"ll	%0, %5\n\t"
+			"move	%3, %4\n"
+			"2:\n\t"
+			"sc	%3, %1\n\t"
+			"beqz	%3, 1b\n"
+			"3:\n\t"
+			".set	pop\n\t"
+			".section .fixup,\"ax\"\n"
+			"4:\n\t"
+			"li	%2, 1	\t\t# error\n\t"
+			"j	3b\n\t"
 			".previous\n\t"
-			".section\t__ex_table,\"a\"\n\t"
-			".word\t1b, 3b\n\t"
-			".word\t2b, 3b\n\t"
+			".section __ex_table,\"a\"\n\t"
+			".word	1b, 4b\n\t"
+			".word	2b, 4b\n\t"
 			".previous\n\t"
-			: "=&r" (tmp), "=o" (* (u32 *) p), "=r" (errno)
-			: "r" (arg2), "o" (* (u32 *) p), "2" (errno)
-			: "$1");
+			: "=&r" (tmp), "=R" (*p), "=r" (errno), "=&r" (tmp1)
+			: "r" (arg2), "1" (*p), "2" (0));
+#else
+		save_and_cli(tmp1);
+		errno = __get_user(tmp, p);
+		errno |= __put_user(arg2, p);
+		restore_flags(tmp1);
+#endif
 
 		if (errno)
 			return -EFAULT;
@@ -119,9 +127,6 @@ sys_sysmips(int cmd, int arg1, int arg2,
 			: /* No outputs */
 			: "r" (&cmd));
 		/* Unreached */
-#else
-	printk("sys_sysmips(MIPS_ATOMIC_SET, ...) not ready for !CONFIG_CPU_HAS_LLSC\n");
-#endif
 	}
 
 	case MIPS_FIXADE:

---

From: Florian Lohoff <[email-masked]>
Date: Thu, 5 Jul 2001 16:27:05 +0200

> will now.  The patch should go into Linus' tree as well. 

How is this patch supposed to work in the means of how does it come around
the -MAXERRNO stuff in scall32 ?

Flo
-- 
Florian Lohoff                  [email-masked]             +49-5201-669912
     Why is it called "common sense" when nobody seems to have any?

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Thu, 5 Jul 2001 17:08:36 +0200 (MET DST)

> the -MAXERRNO stuff in scall32 ?

 It works as far as sysmips(MIPS_ATOMIC_SET) can.  It does not add
anything new -- it merely fixes what is obviously broken (falling through
to MIPS_FIXADE from MIPS_ATOMIC_SET for non-ll/sc CPUs, what is being
currently done, is not the most brilliant idea, either).  For better
handling I encourage you to use the _test_and_set patch I've sent here
recently.  I'm using it exclusively for a few weeks now. 

 I do not intend to maintain sysmips(MIPS_ATOMIC_SET) beyond fixing
obviously broken stuff.  I'm voting only to keep it until we have a
replacement.  It should go away in 2.5 for sure -- if you recall the
recent discussion, the conclusion was it's not really needed if we can
satisfy glibc differently. 

 I'd like to see Linus' 2.4.x working on R3k, OTOH.  It would be very
unfortunate if the official 2.4.x didn't work on R3k, just like 2.2.x (and
patches for 2.2.x were available at that time!).

---

From: Jun Sun <[email-masked]>
Date: Thu, 05 Jul 2001 10:51:22 -0700

> satisfy glibc differently.

That was the conclusion, but did not make to the CVS tree, probably due to
Ralf's unwillingness to take a overhead for "flawed" CPUs.

In my last patch for Vr41xx, I have a patch for this.  Basically, I will send
a SIGSYS if the return value is a small negative.  This will practically
satify all the need while keep the change minimum.  The small modification to
the semantic is not too bad at all if you consider the original syscall
semantic is already badly broken.

Jun

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Thu, 5 Jul 2001 20:23:33 +0200 (MET DST)

> Ralf's unwillingness to take a overhead for "flawed" CPUs.

 What overhead?  The code is conditional at the preprocessor level.

> semantic is already badly broken.

 The default action for SIGSYS is to abort with a core dump, so it seems
fine here -- I don't object.  It allows us to use the normal return path,
instead of that crude jump hack, too.

 Not that I particularly care about sysmips(MIPS_ATOMIC_SET) anymore...

  Maciej