[patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

Hi,

 Here is the second version of the sys__test_and_set() syscall suite.  A
glibc patch is included this time as well.

 There are two small changes to the sys__test_and_set() implementation:

1. verify_area() is now called for the ll/sc version as well.  Otherwise
one could pass a KSEG address and gain unauthorized access.

2. The fuction now returns immediately without performing a write access
if the value stored in the memory wouldn't change.  This is to avoid the
need of a potentially costful sc operation; for consistency, this is also
done for the non-ll/sc version. 

 The glibc patch should be fairly obvious.  There is no inline version of
the _test_and_set() function for MIPS I anymore -- while previously it
saved an extra stack frame just to call sysmips(), this would be pointless
now (well, not quite as long as we fallback to sysmips(), actually, but
that is a temporary compatibility bit that will soon get removed, I hope).
Note that while sys__test_and_set() never returns an error, there might
one happen if someone tries to execute the syscall running a kernel that
does not support it.  Hence we fall back to sysmips(). 

 The official entry point is _test_and_set().  There is also the
___test_and_set() entry point defined, mostly for completeness for MIPS
II+ systems, to be sure all syscalls actually have their wrappers
exported.  Not to be used under normal circumstances, though.

 Andreas, what do you think: Should we fall back to sysmips() as in the
following patch (at a considerable performance hit -- without the fallback
the entire ___test_and_set() wrapper is seven instructions long) or just
require a specific minimum kernel version bail out at the compile time if
no __NR__test_and_set is defined?  Granted, pthreads don't run for
MIPS/Linux for a long time, so it's possible the user base is not that
large such an abrupt switch would be impossible.  Especially as sysmips() 
seems to be continuously in flux for the last few months.  I assume the
switch to the new syscall would be mandatory for glibc 2.3 in any case. 

 I'm open to constructive feedback.  An open question is whether returning
the result in v1 is clean.  I believe it is -- I haven't been convinced
that storing the result in a memory location passed as the third argument
is cleaner.  Certainly it's not faster and v1 is still dedicated to be a
result register.  It's used by sys_pipe() this way, for example. 

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

patch-mips-2.4.0-test12-20010110-tas-12
diff -up --recursive --new-file linux-mips-2.4.0-test12-20010110.macro/arch/mips/kernel/syscall.c linux-mips-2.4.0-test12-20010110/arch/mips/kernel/syscall.c
--- linux-mips-2.4.0-test12-20010110.macro/arch/mips/kernel/syscall.c	Sun Oct 29 05:26:54 2000
+++ linux-mips-2.4.0-test12-20010110/arch/mips/kernel/syscall.c	Wed May 30 13:01:00 2001
@@ -174,6 +174,90 @@ asmlinkage int sys_olduname(struct oldol
 	return error;
 }
 
+/* Note: errno is always zero and the result is in v1. */
+asmlinkage int sys__test_and_set(struct pt_regs regs)
+{
+	int *ptr, val, ret, err, tmp;
+
+	ptr = (int *)(regs.regs[4]);
+	val = (int)(regs.regs[5]);
+
+	/* Don't emulate unaligned accesses. */
+	if ((int)ptr & 3)
+		goto fault;
+
+	/* A zero here saves us three instructions. */
+	err = verify_area(VERIFY_WRITE, ptr, 0);
+	if (err)
+		goto fault;
+
+#ifdef CONFIG_CPU_HAS_LLSC
+	__asm__(".set	mips2\n\t"
+		"1:\n\t"
+		"ll	%0,%5\n\t"
+		".set	push\n\t"
+		".set	noreorder\n\t"
+		"beq	%0,%4,3f\n\t"
+		" move	%3,%4\n"
+		".set	pop\n\t"
+		"2:\n\t"
+		"sc	%3,%1\n\t"
+		"beqz	%3,1b\n\t"
+		"3:\n\t"
+		".set	mips0\n\t"
+		".section .fixup,\"ax\"\n"
+		"4:\n\t"
+		"li	%2,%7\n\t"
+		"j	3b\n\t"
+		".previous\n\t"
+		".section __ex_table,\"a\"\n\t"
+		".word	1b,4b\n\t"
+		".word	2b,4b\n\t"
+		".previous"
+		: "=&r" (ret), "=R" (*ptr), "=r" (err), "=&r" (tmp)
+		: "r" (val), "1" (*ptr), "2" (0), "i" (-EFAULT));
+#else
+	save_and_cli(tmp);
+	err = __get_user(ret, ptr);
+	if (ret != val)
+		err |= __put_user(val, ptr);	/* No fault
+						   unless unwriteable. */
+	restore_flags(tmp);
+#endif
+
+	if (err)
+		goto fault;
+
+	(int)(regs.regs[3]) = ret;
+
+	return 0;
+
+fault:
+	regs.cp0_epc -= 4;		/* Go back to SYSCALL. */
+
+	{
+		struct siginfo info;
+
+
+		if ((int)ptr & 3) {
+			info.si_signo = SIGBUS;
+			info.si_code = BUS_ADRALN;
+		} else {
+			info.si_signo = SIGSEGV;
+			if (verify_area(VERIFY_WRITE, ptr, 0))
+				info.si_code = SEGV_ACCERR;
+			else
+				info.si_code = SEGV_MAPERR;
+		}
+		info.si_errno = 0;
+		info.si_addr = (void *)regs.cp0_epc;
+		
+		force_sig_info(info.si_signo, &info, current);
+	}
+
+	return 0;
+}
+
 /*
  * Do the indirect syscall syscall.
  * Don't care about kernel locking; the actual syscall will do it.
diff -up --recursive --new-file linux-mips-2.4.0-test12-20010110.macro/arch/mips/kernel/syscalls.h linux-mips-2.4.0-test12-20010110/arch/mips/kernel/syscalls.h
--- linux-mips-2.4.0-test12-20010110.macro/arch/mips/kernel/syscalls.h	Wed Nov  8 05:26:57 2000
+++ linux-mips-2.4.0-test12-20010110/arch/mips/kernel/syscalls.h	Wed May 23 23:59:02 2001
@@ -235,3 +235,4 @@ SYS(sys_mincore, 3)
 SYS(sys_madvise, 3)
 SYS(sys_getdents64, 3)
 SYS(sys_fcntl64, 3)				/* 4220 */
+SYS(sys__test_and_set, 0)
diff -up --recursive --new-file linux-mips-2.4.0-test12-20010110.macro/include/asm-mips/unistd.h linux-mips-2.4.0-test12-20010110/include/asm-mips/unistd.h
--- linux-mips-2.4.0-test12-20010110.macro/include/asm-mips/unistd.h	Thu Oct 26 04:27:09 2000
+++ linux-mips-2.4.0-test12-20010110/include/asm-mips/unistd.h	Wed May 23 23:09:00 2001
@@ -233,11 +233,12 @@
 #define __NR_madvise			(__NR_Linux + 218)
 #define __NR_getdents64			(__NR_Linux + 219)
 #define __NR_fcntl64			(__NR_Linux + 220)
+#define __NR__test_and_set		(__NR_Linux + 221)
 
 /*
  * Offset of the last Linux flavoured syscall
  */
-#define __NR_Linux_syscalls		220
+#define __NR_Linux_syscalls		221
 
 #ifndef _LANGUAGE_ASSEMBLY
 

glibc-2.2.3-mips-tas.patch
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/Versions glibc-2.2.3/sysdeps/unix/sysv/linux/mips/Versions
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/Versions	Wed Aug  2 21:53:16 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/Versions	Wed May 30 02:20:28 2001
@@ -16,6 +16,6 @@ libc {
   }
   GLIBC_2.2 {
     # _*
-    _test_and_set;
+    _test_and_set; ___test_and_set;
   }
 }
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Fri Jul 28 13:37:25 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Wed May 30 12:05:33 2001
@@ -21,10 +21,47 @@
    defined in sys/tas.h  */
 
 #include <features.h>
+#include <sgidefs.h>
+#include <unistd.h>
+#include <sysdep.h>
+#include <sys/sysmips.h>
 
 #define _EXTERN_INLINE
 #ifndef __USE_EXTERN_INLINES
 # define __USE_EXTERN_INLINES 1
 #endif
 
+#ifdef __NR__test_and_set
+#define __HAVE__TEST_AND_SET 1
+#else
+#define __HAVE__TEST_AND_SET 0
+#endif
+
 #include "sys/tas.h"
+
+int ___test_and_set (int *p, int v)
+{
+  if (__HAVE__TEST_AND_SET)
+    {
+      register int *__p asm ("$4") = p;
+      register int __v asm ("$5") = v;
+      register int __n asm ("$2") = SYS_ify (_test_and_set);
+      register int __e asm ("$7");
+      register int __r asm ("$3");
+
+      asm
+        ("syscall"
+	 : "=r" (__r), "=r" (__e)
+	 : "r" (__p), "r" (__v), "r" (__n)
+	 : "$2",
+	   "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24",
+	   "memory");
+      if (!__e)
+	return __r;
+    }
+  return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
+}
+
+#if (_MIPS_ISA < _MIPS_ISA_MIPS2)
+strong_alias (___test_and_set, _test_and_set)
+#endif
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h	Sun Jan  7 04:35:41 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h	Wed May 30 02:18:19 2001
@@ -22,11 +22,11 @@
 
 #include <features.h>
 #include <sgidefs.h>
-#include <sys/sysmips.h>
 
 __BEGIN_DECLS
 
 extern int _test_and_set (int *p, int v) __THROW;
+extern int ___test_and_set (int *p, int v) __THROW;
 
 #ifdef __USE_EXTERN_INLINES
 
@@ -59,15 +59,7 @@ _test_and_set (int *p, int v) __THROW
   return r;
 }
 
-# else /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
-
-_EXTERN_INLINE int
-_test_and_set (int *p, int v) __THROW
-{
-  return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
-}
-
-# endif /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
+# endif /* (_MIPS_ISA >= _MIPS_ISA_MIPS2) */
 
 #endif /* __USE_EXTERN_INLINES */
 

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Andreas Jaeger]

"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:

> Hi,
> 
>  Here is the second version of the sys__test_and_set() syscall suite.  A
> glibc patch is included this time as well.
> 
>  There are two small changes to the sys__test_and_set() implementation:
> 
> 1. verify_area() is now called for the ll/sc version as well.  Otherwise
> one could pass a KSEG address and gain unauthorized access.
> 
> 2. The fuction now returns immediately without performing a write access
> if the value stored in the memory wouldn't change.  This is to avoid the
> need of a potentially costful sc operation; for consistency, this is also
> done for the non-ll/sc version. 
> 
>  The glibc patch should be fairly obvious.  There is no inline version of
> the _test_and_set() function for MIPS I anymore -- while previously it
> saved an extra stack frame just to call sysmips(), this would be pointless
> now (well, not quite as long as we fallback to sysmips(), actually, but
> that is a temporary compatibility bit that will soon get removed, I hope).
> Note that while sys__test_and_set() never returns an error, there might
> one happen if someone tries to execute the syscall running a kernel that
> does not support it.  Hence we fall back to sysmips(). 
> 
>  The official entry point is _test_and_set().  There is also the
> ___test_and_set() entry point defined, mostly for completeness for MIPS
> II+ systems, to be sure all syscalls actually have their wrappers
> exported.  Not to be used under normal circumstances, though.
> 
>  Andreas, what do you think: Should we fall back to sysmips() as in the
> following patch (at a considerable performance hit -- without the fallback
> the entire ___test_and_set() wrapper is seven instructions long) or just
> require a specific minimum kernel version bail out at the compile time if
> no __NR__test_and_set is defined?  Granted, pthreads don't run for
> MIPS/Linux for a long time, so it's possible the user base is not that
> large such an abrupt switch would be impossible.  Especially as sysmips() 
> seems to be continuously in flux for the last few months.  I assume the
> switch to the new syscall would be mandatory for glibc 2.3 in any case. 

Do it the following way:
- Define in sysdeps/unix/sysv/linux/kernel-features a new macro, e.g.
  __ASSUME_TEST_AND_SET with the appropriate guards
- Do *both* implementations like this:
#include <kernel-features.h>
#if __ASSUME_TEST_AND_SET
fast code without fallback
#else
slow code that first tries kernel call and then falls back to sysmips
#endif

This way you get the fast one if you configure glibc with
--enable-kernel=2.4.6 if we assume that 2.4.6 is the first kernel with
those features. 

Check other places in glibc for details how this can be done.

Thanks,
Andreas

>  I'm open to constructive feedback.  An open question is whether returning
> the result in v1 is clean.  I believe it is -- I haven't been convinced
> that storing the result in a memory location passed as the third argument
> is cleaner.  Certainly it's not faster and v1 is still dedicated to be a
> result register.  It's used by sys_pipe() this way, for example. 
> 
>   Maciej

-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Jun Sun]

Andreas Jaeger wrote:
> 
> "Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:
> 
> > Hi,
> >
> >  Here is the second version of the sys__test_and_set() syscall suite.  A
> > glibc patch is included this time as well.
> >
> >  There are two small changes to the sys__test_and_set() implementation:
> >
> > 1. verify_area() is now called for the ll/sc version as well.  Otherwise
> > one could pass a KSEG address and gain unauthorized access.
> >
> > 2. The fuction now returns immediately without performing a write access
> > if the value stored in the memory wouldn't change.  This is to avoid the
> > need of a potentially costful sc operation; for consistency, this is also
> > done for the non-ll/sc version.
> >
> >  The glibc patch should be fairly obvious.  There is no inline version of
> > the _test_and_set() function for MIPS I anymore -- while previously it
> > saved an extra stack frame just to call sysmips(), this would be pointless
> > now (well, not quite as long as we fallback to sysmips(), actually, but
> > that is a temporary compatibility bit that will soon get removed, I hope).
> > Note that while sys__test_and_set() never returns an error, there might
> > one happen if someone tries to execute the syscall running a kernel that
> > does not support it.  Hence we fall back to sysmips().
> >
> >  The official entry point is _test_and_set().  There is also the
> > ___test_and_set() entry point defined, mostly for completeness for MIPS
> > II+ systems, to be sure all syscalls actually have their wrappers
> > exported.  Not to be used under normal circumstances, though.
> >
> >  Andreas, what do you think: Should we fall back to sysmips() as in the
> > following patch (at a considerable performance hit -- without the fallback
> > the entire ___test_and_set() wrapper is seven instructions long) or just
> > require a specific minimum kernel version bail out at the compile time if
> > no __NR__test_and_set is defined?  Granted, pthreads don't run for
> > MIPS/Linux for a long time, so it's possible the user base is not that
> > large such an abrupt switch would be impossible.  Especially as sysmips()
> > seems to be continuously in flux for the last few months.  I assume the
> > switch to the new syscall would be mandatory for glibc 2.3 in any case.
> 
> Do it the following way:
> - Define in sysdeps/unix/sysv/linux/kernel-features a new macro, e.g.
>   __ASSUME_TEST_AND_SET with the appropriate guards
> - Do *both* implementations like this:
> #include <kernel-features.h>
> #if __ASSUME_TEST_AND_SET
> fast code without fallback
> #else
> slow code that first tries kernel call and then falls back to sysmips
> #endif
> 
> This way you get the fast one if you configure glibc with
> --enable-kernel=2.4.6 if we assume that 2.4.6 is the first kernel with
> those features.
> 
> Check other places in glibc for details how this can be done.
> 

This might be an overkill - the slow code on a newer kernel costs about 1 or 2
cycle longer per call.


> >  I'm open to constructive feedback.  An open question is whether returning
> > the result in v1 is clean.  I believe it is -- I haven't been convinced
> > that storing the result in a memory location passed as the third argument
> > is cleaner.  Certainly it's not faster and v1 is still dedicated to be a
> > result register.  It's used by sys_pipe() this way, for example.

On a second thought I feel stronger using $v1 is not a good idea - what if the
return path of syscall suddenly needs to modify $v1?  Also we have generic
syscall routine in glibc, what if someday that routine starts to check $v1 as
well?

I understand the chances of these "what if" are low (and perhaps sys_pipe() is
already this way), but I am still convinced we should the right thing. 
(Whoever invented MIPS_ATOMIC_SET might have been thinking it should never
need to return an error code!)

I don't see any "dirtiness" in using the third argument.  The slowdown in
performance should be negligible under the context of the whole system call.

A syscall is invented to be here and stay.  I personally feel more comfortable
to play a little safer rather than a littel faster.

Jun

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

On Thu, 31 May 2001, Jun Sun wrote:

> > This way you get the fast one if you configure glibc with
> > --enable-kernel=2.4.6 if we assume that 2.4.6 is the first kernel with
> > those features.
> >
> > Check other places in glibc for details how this can be done.
> 
> This might be an overkill - the slow code on a newer kernel costs about 1 or 2
> cycle longer per call.

 Do you realize how often the call is invoked!?  There is a number of
memory stores involved in the slow implementation -- with write-trough
cache it really sucks, even more than memory accesses in general. 

> On a second thought I feel stronger using $v1 is not a good idea - what if the
> return path of syscall suddenly needs to modify $v1?  Also we have generic
> syscall routine in glibc, what if someday that routine starts to check $v1 as
> well?

 We are safe here.  V1 is a result register so its semantics must be
preserved if at all possible.  Then there is the pipe() syscall that
already makes use of v1 as a result register for a long time, so we are
really, really safe. 

 There are t* registers available for the syscall handler if an additional
register is needed.  The coding conventions are there for a reason, aren't
they? 

> I understand the chances of these "what if" are low (and perhaps sys_pipe() is
> already this way), but I am still convinced we should the right thing. 
> (Whoever invented MIPS_ATOMIC_SET might have been thinking it should never
> need to return an error code!)

 Nope -- the problem lies elsewhere.  It is sysmips() that was invented to
return an error code and MIPS_ATOMIC_SET violating it from the day #1.  It
was plain broken since the beginning.  I suppose it ws just an ad hoc hack
written once someone realized the operation is necessary.  Just like the
whole sysmips() syscall. 

> I don't see any "dirtiness" in using the third argument.  The slowdown in
> performance should be negligible under the context of the whole system call.

 I do believe good performance is the main goal here.  The syscall is as
clean as possible -- it would still be possible to make it yet faster if
dirty hacks were added.

> A syscall is invented to be here and stay.  I personally feel more comfortable
> to play a little safer rather than a littel faster.

 Cycles sum up, unfortunately.  A strace of `ls -la' on my /usr/lib
directory shows about 4500 syscall invocations of which about 4000 are
invocations of _test_and_set()!

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

On 31 May 2001, Andreas Jaeger wrote:

> Do it the following way:
> - Define in sysdeps/unix/sysv/linux/kernel-features a new macro, e.g.
>   __ASSUME_TEST_AND_SET with the appropriate guards
> - Do *both* implementations like this:
> #include <kernel-features.h>
> #if __ASSUME_TEST_AND_SET
> fast code without fallback
> #else
> slow code that first tries kernel call and then falls back to sysmips
> #endif
> This way you get the fast one if you configure glibc with
> --enable-kernel=2.4.6 if we assume that 2.4.6 is the first kernel with
> those features. 

 Thanks for the tip.  It's reasonable, indeed.  Now the point is to get
Linux changes (once introduced) back to Linus' tree.  It would be bad to
to tie a kernel version with a feature that would be present in the CVS at
oss. 

> Check other places in glibc for details how this can be done.

 OK, how about this patch then (the kernel version has to be set once
known)?

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

glibc-2.2.3-mips-tas.patch
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/kernel-features.h glibc-2.2.3/sysdeps/unix/sysv/linux/kernel-features.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/kernel-features.h	Wed Apr 25 21:51:14 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/kernel-features.h	Thu May 31 15:54:59 2001
@@ -182,3 +182,8 @@
 # define __ASSUME_FCNTL64		1
 # define __ASSUME_GETDENTS64_SYSCALL	1
 #endif
+
+/* The _test_and_set syscall is available on MIPS since 2.?.?.  */
+#if 0 && defined __mips__
+# define __ASSUME__TEST_AND_SET		1
+#endif
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/Versions glibc-2.2.3/sysdeps/unix/sysv/linux/mips/Versions
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/Versions	Wed Aug  2 21:53:16 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/Versions	Wed May 30 02:20:28 2001
@@ -16,6 +16,6 @@ libc {
   }
   GLIBC_2.2 {
     # _*
-    _test_and_set;
+    _test_and_set; ___test_and_set;
   }
 }
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Fri Jul 28 13:37:25 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Thu May 31 23:21:50 2001
@@ -21,6 +21,12 @@
    defined in sys/tas.h  */
 
 #include <features.h>
+#include <sgidefs.h>
+#include <unistd.h>
+#include <sysdep.h>
+#include <sys/sysmips.h>
+
+#include "kernel-features.h"
 
 #define _EXTERN_INLINE
 #ifndef __USE_EXTERN_INLINES
@@ -28,3 +34,46 @@
 #endif
 
 #include "sys/tas.h"
+
+#ifdef __NR__test_and_set
+# ifdef __ASSUME__TEST_AND_SET
+#  define __have_no__test_and_set 0
+# else
+static int __have_no__test_and_set;
+# endif
+#endif
+
+int ___test_and_set (int *p, int v)
+{
+#ifdef __NR__test_and_set
+  if (!__builtin_expect(__have_no__test_and_set, 0))
+    {
+      register int *__p asm ("$4") = p;
+      register int __v asm ("$5") = v;
+      register int __n asm ("$2") = SYS_ify (_test_and_set);
+      register int __e asm ("$7");
+      register int __r asm ("$3");
+
+      asm
+        ("syscall"
+	 : "=r" (__r), "=r" (__e)
+	 : "r" (__p), "r" (__v), "r" (__n)
+	 : "$2",
+	   "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24",
+	   "memory");
+# ifndef __ASSUME__TEST_AND_SET
+      if (__builtin_expect(__e, 0))
+	__have_no__test_and_set = 1;
+      else
+# endif
+	return __r;
+    }
+#endif
+#if !defined __NR__test_and_set || !defined __ASSUME__TEST_AND_SET
+    return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
+#endif
+}
+
+#if (_MIPS_ISA < _MIPS_ISA_MIPS2)
+strong_alias (___test_and_set, _test_and_set)
+#endif
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h	Sun Jan  7 04:35:41 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h	Wed May 30 02:18:19 2001
@@ -22,11 +22,11 @@
 
 #include <features.h>
 #include <sgidefs.h>
-#include <sys/sysmips.h>
 
 __BEGIN_DECLS
 
 extern int _test_and_set (int *p, int v) __THROW;
+extern int ___test_and_set (int *p, int v) __THROW;
 
 #ifdef __USE_EXTERN_INLINES
 
@@ -59,15 +59,7 @@ _test_and_set (int *p, int v) __THROW
   return r;
 }
 
-# else /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
-
-_EXTERN_INLINE int
-_test_and_set (int *p, int v) __THROW
-{
-  return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
-}
-
-# endif /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
+# endif /* (_MIPS_ISA >= _MIPS_ISA_MIPS2) */
 
 #endif /* __USE_EXTERN_INLINES */
 

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Andreas Jaeger]

"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:

> On 31 May 2001, Andreas Jaeger wrote:
> 
>> Do it the following way:
>> - Define in sysdeps/unix/sysv/linux/kernel-features a new macro, e.g.
>>   __ASSUME_TEST_AND_SET with the appropriate guards
>> - Do *both* implementations like this:
>> #include <kernel-features.h>
>> #if __ASSUME_TEST_AND_SET
>> fast code without fallback
>> #else
>> slow code that first tries kernel call and then falls back to sysmips
>> #endif
>> This way you get the fast one if you configure glibc with
>> --enable-kernel=2.4.6 if we assume that 2.4.6 is the first kernel with
>> those features. 
> 
>  Thanks for the tip.  It's reasonable, indeed.  Now the point is to get
> Linux changes (once introduced) back to Linus' tree.  It would be bad to
> to tie a kernel version with a feature that would be present in the CVS at
> oss. 
> 
>> Check other places in glibc for details how this can be done.
> 
>  OK, how about this patch then (the kernel version has to be set once
> known)?
> 
>   Maciej

diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Fri Jul 28 13:37:25 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Thu May 31 23:21:50 2001
@@ -21,6 +21,12 @@
    defined in sys/tas.h  */
 
 #include <features.h>
+#include <sgidefs.h>
+#include <unistd.h>
+#include <sysdep.h>
+#include <sys/sysmips.h>
+
+#include "kernel-features.h"
 
 #define _EXTERN_INLINE
 #ifndef __USE_EXTERN_INLINES
@@ -28,3 +34,46 @@
 #endif
 
 #include "sys/tas.h"
+
+#ifdef __NR__test_and_set
+# ifdef __ASSUME__TEST_AND_SET
+#  define __have_no__test_and_set 0

Don't add this, compare how we do it in similar cases.
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h	Sun Jan  7 04:35:41 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h	Wed May 30 02:18:19 2001
@@ -22,11 +22,11 @@
 
 #include <features.h>
 #include <sgidefs.h>
-#include <sys/sysmips.h>
 
 __BEGIN_DECLS
 
 extern int _test_and_set (int *p, int v) __THROW;
+extern int ___test_and_set (int *p, int v) __THROW;

Why do you export this here?

Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

On 1 Jun 2001, Andreas Jaeger wrote:

>  #include "sys/tas.h"
> +
> +#ifdef __NR__test_and_set
> +# ifdef __ASSUME__TEST_AND_SET
> +#  define __have_no__test_and_set 0
> 
> Don't add this, compare how we do it in similar cases.

 Hmm, I looked at sysdeps/unix/sysv/linux/getcwd.c.  It does it in a
similar way.  What's wrong with this approach?  I'm just asking -- it
looks I do not always guess glibc rules right and not everything is
documented.

 Actually I tried to avoid macros if at all possible but gcc refuses to
eliminate code even if that's something like:

static const int var = 1;
<...>
if (var)
<...>

It still generates the code to check the value of var, sigh...

 Also I feel a bit uneasy about placing the "#ifdef
__ASSUME__TEST_AND_SET" condition outside -- __NR__test_and_set might be
undefined due to outdated kernel headers even if someone specified the
--enable-kernel option.  Is it considered justified within glibc to bail
out at the compilation time in this case? 

>  extern int _test_and_set (int *p, int v) __THROW;
> +extern int ___test_and_set (int *p, int v) __THROW;
> 
> Why do you export this here?

 It's a syscall wrapper.  We want to export syscall wrappers, don't we? 
And if we export a symbol, we should also declare it -- programs declaring
library symbols themselves are broken and doomed to fail sooner or later
-- have you seen what happens on glibc systems to old programs which
declare <string.h> functions due to the lack of appropriate declarations
in system headers at one time?

 If we don't want to export the wrapper, then fine -- I'll remove both the
symbol and the declaration. 

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Andreas Jaeger]

"Maciej W. Rozycki" <macro@ds2.pg.gda.pl> writes:

> On 1 Jun 2001, Andreas Jaeger wrote:
> 
>>  #include "sys/tas.h"
>> +
>> +#ifdef __NR__test_and_set
>> +# ifdef __ASSUME__TEST_AND_SET
>> +#  define __have_no__test_and_set 0
>> 
>> Don't add this, compare how we do it in similar cases.
> 
>  Hmm, I looked at sysdeps/unix/sysv/linux/getcwd.c.  It does it in a
> similar way.  What's wrong with this approach?  I'm just asking -- it
> looks I do not always guess glibc rules right and not everything is
> documented.
We normally do not define anything to 0 - unless there's no other
way.  And looking briefly over your code there should be other
solutions.  Sorry, I'm limited in time currently, otherwise I would
rewrite it myself.

Look at i386/lockf64.c for a cleaner example.

>  Actually I tried to avoid macros if at all possible but gcc refuses to
> eliminate code even if that's something like:
> 
> static const int var = 1;
> <...>
> if (var)
> <...>
> 
> It still generates the code to check the value of var, sigh...
> 
>  Also I feel a bit uneasy about placing the "#ifdef
> __ASSUME__TEST_AND_SET" condition outside -- __NR__test_and_set might be
> undefined due to outdated kernel headers even if someone specified the
> --enable-kernel option.  Is it considered justified within glibc to bail
> out at the compilation time in this case? 

We check that for the kernel headers in configure.

>>  extern int _test_and_set (int *p, int v) __THROW;
>> +extern int ___test_and_set (int *p, int v) __THROW;
>> 
>> Why do you export this here?
> 
>  It's a syscall wrapper.  We want to export syscall wrappers, don't
>  we? 

No, not everything - we already export _test_and_set and that should
be enough.
> And if we export a symbol, we should also declare it -- programs declaring
> library symbols themselves are broken and doomed to fail sooner or later
> -- have you seen what happens on glibc systems to old programs which
> declare <string.h> functions due to the lack of appropriate declarations
> in system headers at one time?
> 
>  If we don't want to export the wrapper, then fine -- I'll remove both the
> symbol and the declaration. 


Andreas
-- 
 Andreas Jaeger
  SuSE Labs aj@suse.de
   private aj@arthur.inka.de
    http://www.suse.de/~aj

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

On 1 Jun 2001, Andreas Jaeger wrote:

> We normally do not define anything to 0 - unless there's no other
> way.  And looking briefly over your code there should be other
> solutions.  Sorry, I'm limited in time currently, otherwise I would
> rewrite it myself.

 OK, I'll check how to write it better and still get good optimization
results.  Please don't bother writing it yourself -- we don't have any
kernel code yet, so there is no real need to get involved so much.

> Look at i386/lockf64.c for a cleaner example.

 Hmm, glibc rules certainly look different from Linux's ones -- I tried to
avoid interspersing real code with preprocessor conditionals.  Since you
state it's OK, I should have no problem with coding accrdingly.

> >  It's a syscall wrapper.  We want to export syscall wrappers, don't
> >  we? 
> 
> No, not everything - we already export _test_and_set and that should
> be enough.

 OK, then.

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

Re: [patch] RFC: A sys__test_and_set() implementation, 2nd iteration

[Maciej W. Rozycki]

Hi,

 This version should be better.

  Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: macro@ds2.pg.gda.pl, PGP key available        +

glibc-2.2.3-mips-tas.patch
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/kernel-features.h glibc-2.2.3/sysdeps/unix/sysv/linux/kernel-features.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/kernel-features.h	Wed Apr 25 21:51:14 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/kernel-features.h	Thu May 31 15:54:59 2001
@@ -182,3 +182,8 @@
 # define __ASSUME_FCNTL64		1
 # define __ASSUME_GETDENTS64_SYSCALL	1
 #endif
+
+/* The _test_and_set syscall is available on MIPS since 2.?.?.  */
+#if 0 && defined __mips__
+# define __ASSUME__TEST_AND_SET		1
+#endif
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Fri Jul 28 13:37:25 2000
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/_test_and_set.c	Sat Jun  2 13:04:35 2001
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000 Free Software Foundation, Inc.
+/* Copyright (C) 2000, 2001 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Maciej W. Rozycki <macro@ds2.pg.gda.pl>, 2000.
 
@@ -21,6 +21,12 @@
    defined in sys/tas.h  */
 
 #include <features.h>
+#include <sgidefs.h>
+#include <unistd.h>
+#include <sysdep.h>
+#include <sys/sysmips.h>
+
+#include "kernel-features.h"
 
 #define _EXTERN_INLINE
 #ifndef __USE_EXTERN_INLINES
@@ -28,3 +34,45 @@
 #endif
 
 #include "sys/tas.h"
+
+#if (_MIPS_ISA < _MIPS_ISA_MIPS2)
+
+#if defined __NR__test_and_set && __ASSUME__TEST_AND_SET == 0
+static int __have_no__test_and_set;
+#endif
+
+int _test_and_set (int *p, int v)
+{
+#ifdef __NR__test_and_set
+# if __ASSUME__TEST_AND_SET == 0
+  if (!__builtin_expect(__have_no__test_and_set, 0))
+# endif
+    {
+      register int *__p asm ("$4") = p;
+      register int __v asm ("$5") = v;
+      register int __n asm ("$2") = SYS_ify (_test_and_set);
+      register int __e asm ("$7");
+      register int __r asm ("$3");
+
+      asm ("syscall"
+	   : "=r" (__r), "=r" (__e)
+	   : "r" (__p), "r" (__v), "r" (__n)
+	   : "$2",
+	     "$8", "$9", "$10", "$11", "$12", "$13", "$14", "$15", "$24",
+	     "memory");
+# if __ASSUME__TEST_AND_SET > 0
+      return __r;
+# else
+      if (!__builtin_expect(__e, 0))
+	return __r;
+
+      __have_no__test_and_set = 1;
+# endif
+    }
+#endif /* __NR__test_and_set */
+#if !defined __NR__test_and_set || __ASSUME__TEST_AND_SET == 0
+    return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
+#endif
+}
+
+#endif /* _MIPS_ISA < _MIPS_ISA_MIPS2 */
diff -up --recursive --new-file glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h
--- glibc-2.2.3.macro/sysdeps/unix/sysv/linux/mips/sys/tas.h	Sun Jan  7 04:35:41 2001
+++ glibc-2.2.3/sysdeps/unix/sysv/linux/mips/sys/tas.h	Sat Jun  2 13:44:12 2001
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000 Free Software Foundation, Inc.
+/* Copyright (C) 2000, 2001 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Maciej W. Rozycki <macro@ds2.pg.gda.pl>, 2000.
 
@@ -22,7 +22,6 @@
 
 #include <features.h>
 #include <sgidefs.h>
-#include <sys/sysmips.h>
 
 __BEGIN_DECLS
 
@@ -59,15 +58,7 @@ _test_and_set (int *p, int v) __THROW
   return r;
 }
 
-# else /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
-
-_EXTERN_INLINE int
-_test_and_set (int *p, int v) __THROW
-{
-  return sysmips (MIPS_ATOMIC_SET, (int) p, v, 0);
-}
-
-# endif /* !(_MIPS_ISA >= _MIPS_ISA_MIPS2) */
+# endif /* (_MIPS_ISA >= _MIPS_ISA_MIPS2) */
 
 #endif /* __USE_EXTERN_INLINES */