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

From: Andreas Jaeger <[email-masked]>
Date: 01 Jun 2001 13:44:51 +0200

"Maciej W. Rozycki" <[email-masked]> 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 [email-masked]
   private [email-masked]
    http://www.suse.de/~aj

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Fri, 1 Jun 2001 13:32:29 +0200 (MET DST)

Maciej

-- 
+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email-masked], 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>
 
 #define _EXTERN_INLINE
 
 #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>
 
 __BEGIN_DECLS

 #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 */

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Fri, 1 Jun 2001 14:18:04 +0200 (MET DST)

> 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? 

> 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

---

From: Andreas Jaeger <[email-masked]>
Date: 01 Jun 2001 15:48:36 +0200

> looks I do not always guess glibc rules right and not everything is
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.

> out at the compilation time in this case? 

We check that for the kernel headers in configure.

>  It's a syscall wrapper.  We want to export syscall wrappers, don't

No, not everything - we already export _test_and_set and that should
be enough.

Andreas

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Fri, 1 Jun 2001 16:21:26 +0200 (MET DST)

> solutions.  Sorry, I'm limited in time currently, otherwise I would

 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.

> No, not everything - we already export _test_and_set and that should

 OK, then.

---

From: "Maciej W. Rozycki" <[email-masked]>
Date: Tue, 5 Jun 2001 14:58:52 +0200 (MET DST)

Hi,

 This version should be better.

  Maciej

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 <[email-masked]>, 2000.
 
@@ -21,6 +21,12 @@
    defined in sys/tas.h  */
 
 #include <features.h>
+#include <sgidefs.h>
+#include <unistd.h>
+#include <sysdep.h>
 
 #define _EXTERN_INLINE
@@ -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 <[email-masked]>, 2000.
 
@@ -22,7 +22,6 @@
 
 #include <features.h>
 #include <sgidefs.h>
 
 __BEGIN_DECLS
 
@@ -59,15 +58,7 @@ _test_and_set (int *p, int v) __THROW
   return r;
 }