Package: release.debian.org
Severity: normal
User: release.debian....@packages.debian.org
Usertags: unblock

Hi,

Please unblock package bind9

This version fixes RC bug #778720 where bind9 randomly crashes on MIPS.
The MIPS atomics implementation was buggy (register constraints were
wrong and there were no memory barriers) and to fix it I just replaced
it with C11 atomics. Only the MIPS part was replaced - this update
should have no effect on other architectures.

I've tested the new version under heavy load for an hour or so on the 3
MIPS architectures and it seems to be OK. Previously it would crash
within 5 mins.

I've attached the debdiff. Since that isn't the easiest to read, I also
attach the diff of lib/isc/mips/include/isc/atomic.h (the only file
changed) between the version in testing and unstable.

There is another RC bug affecting bind9 (#860225), but that bug is not a
regression from stretch.

Thanks,
James

unblock bind9/1:9.10.3.dfsg.P4-12.2
diff -Nru bind9-9.10.3.dfsg.P4/debian/changelog 
bind9-9.10.3.dfsg.P4/debian/changelog
--- bind9-9.10.3.dfsg.P4/debian/changelog       2017-03-17 18:07:16.000000000 
+0000
+++ bind9-9.10.3.dfsg.P4/debian/changelog       2017-04-18 16:42:50.000000000 
+0100
@@ -1,3 +1,11 @@
+bind9 (1:9.10.3.dfsg.P4-12.2) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Replace 32_mips_atomic.diff with a version that uses C11 atomics. Fixes
+    hangs and crashes on MIPS. (Closes: #778720)
+
+ -- James Cowgill <jcowg...@debian.org>  Tue, 18 Apr 2017 16:42:50 +0100
+
 bind9 (1:9.10.3.dfsg.P4-12.1) unstable; urgency=medium
 
   * Non-maintainer upload.
diff -Nru bind9-9.10.3.dfsg.P4/debian/patches/32_mips_atomic.diff 
bind9-9.10.3.dfsg.P4/debian/patches/32_mips_atomic.diff
--- bind9-9.10.3.dfsg.P4/debian/patches/32_mips_atomic.diff     2017-02-19 
22:38:45.000000000 +0000
+++ bind9-9.10.3.dfsg.P4/debian/patches/32_mips_atomic.diff     2017-04-18 
16:42:50.000000000 +0100
@@ -1,22 +1,29 @@
-Author: Thiemo Seufer <t...@networkno.de>
-Date:   Thu Nov 8 15:11:48 2007 -0700
-Forwarded: yes RT#41965
-
-    mips:atomic.h: improve implementation of atomic ops, fix mips{el,64}
-
-    The appended patch extends the configure check to cover mips64 and
-    mipsel, and improves the mips atomics implementation.
-
-    See http://bugs.debian.org/406409 for more detail.
-
-    Signed-off-by: LaMont Jones <lam...@debian.org>
+Description: Replace MIPS atomics assembly with calls to C11 atomic functions
+ This fixes various hangs and crashes on MIPS.
+Author: James Cowgill <jcowg...@debian.org>
+Forwarded: no
+Bug-Debian: https://bugs.debian.org/778720
 
 --- a/lib/isc/mips/include/isc/atomic.h
 +++ b/lib/isc/mips/include/isc/atomic.h
-@@ -31,18 +31,20 @@
- isc_atomic_xadd(isc_int32_t *p, int val) {
-       isc_int32_t orig;
+@@ -19,32 +19,19 @@
+ #ifndef ISC_ATOMIC_H
+ #define ISC_ATOMIC_H 1
+ 
++#include <stdatomic.h>
++
+ #include <isc/platform.h>
+ #include <isc/types.h>
  
+-#ifdef ISC_PLATFORM_USEGCCASM
+ /*
+  * This routine atomically increments the value stored in 'p' by 'val', and
+  * returns the previous value.
+  */
+ static inline isc_int32_t
+ isc_atomic_xadd(isc_int32_t *p, int val) {
+-      isc_int32_t orig;
+-
 -      /* add is a cheat, since MIPS has no mov instruction */
 -      __asm__ volatile (
 -          "1:"
@@ -29,24 +36,13 @@
 -          : "m"(*p), "r"(val)
 -          : "memory", "$3"
 -              );
-+      __asm__ __volatile__ (
-+      "       .set    push            \n"
-+      "       .set    mips2           \n"
-+      "       .set    noreorder       \n"
-+      "       .set    noat            \n"
-+      "1:     ll      $1, %1          \n"
-+      "       addu    %0, $1, %2      \n"
-+      "       sc      %0, %1          \n"
-+      "       beqz    %0, 1b          \n"
-+      "       move    %0, $1          \n"
-+      "       .set    pop             \n"
-+      : "=&r" (orig), "+R" (*p)
-+      : "r" (val)
-+      : "memory");
  
-       return (orig);
+-      return (orig);
++      return atomic_fetch_add(p, val);
  }
-@@ -52,16 +54,7 @@
+ 
+ /*
+@@ -52,16 +39,7 @@ isc_atomic_xadd(isc_int32_t *p, int val)
   */
  static inline void
  isc_atomic_store(isc_int32_t *p, isc_int32_t val) {
@@ -60,16 +56,16 @@
 -          : "m"(*p), "r"(val)
 -          : "memory", "$3"
 -              );
-+      *p = val;
++      atomic_store(p, val);
  }
  
  /*
-@@ -72,20 +65,23 @@
+@@ -71,28 +49,8 @@ isc_atomic_store(isc_int32_t *p, isc_int
+  */
  static inline isc_int32_t
  isc_atomic_cmpxchg(isc_int32_t *p, int cmpval, int val) {
-       isc_int32_t orig;
-+      isc_int32_t tmp;
- 
+-      isc_int32_t orig;
+-
 -      __asm__ volatile(
 -          "1:"
 -          "ll $3, %1\n"
@@ -83,21 +79,15 @@
 -          : "m"(*p), "r"(cmpval), "r"(val)
 -          : "memory", "$3"
 -              );
-+      __asm__ __volatile__ (
-+      "       .set    push            \n"
-+      "       .set    mips2           \n"
-+      "       .set    noreorder       \n"
-+      "       .set    noat            \n"
-+      "1:     ll      $1, %1          \n"
-+      "       bne     $1, %3, 2f      \n"
-+      "       move    %2, %4          \n"
-+      "       sc      %2, %1          \n"
-+      "       beqz    %2, 1b          \n"
-+      "2:     move    %0, $1          \n"
-+      "       .set    pop             \n"
-+      : "=&r"(orig), "+R" (*p), "=r" (tmp)
-+      : "r"(cmpval), "r"(val)
-+      : "memory");
- 
-       return (orig);
+-
+-      return (orig);
++      atomic_compare_exchange_strong(p, &cmpval, val);
++      return cmpval;
  }
+ 
+-#else /* !ISC_PLATFORM_USEGCCASM */
+-
+-#error "unsupported compiler.  disable atomic ops by --disable-atomic"
+-
+-#endif
+ #endif /* ISC_ATOMIC_H */
--- a/lib/isc/mips/include/isc/atomic.h
+++ b/lib/isc/mips/include/isc/atomic.h
@@ -19,34 +19,19 @@
 #ifndef ISC_ATOMIC_H
 #define ISC_ATOMIC_H 1
 
+#include <stdatomic.h>
+
 #include <isc/platform.h>
 #include <isc/types.h>
 
-#ifdef ISC_PLATFORM_USEGCCASM
 /*
  * This routine atomically increments the value stored in 'p' by 'val', and
  * returns the previous value.
  */
 static inline isc_int32_t
 isc_atomic_xadd(isc_int32_t *p, int val) {
-	isc_int32_t orig;
-
-	__asm__ __volatile__ (
-	"	.set	push		\n"
-	"	.set	mips2		\n"
-	"	.set	noreorder	\n"
-	"	.set	noat		\n"
-	"1:	ll	$1, %1		\n"
-	"	addu	%0, $1, %2	\n"
-	"	sc	%0, %1		\n"
-	"	beqz	%0, 1b		\n"
-	"	move	%0, $1		\n"
-	"	.set	pop		\n"
-	: "=&r" (orig), "+R" (*p)
-	: "r" (val)
-	: "memory");
 
-	return (orig);
+	return atomic_fetch_add(p, val);
 }
 
 /*
@@ -54,7 +39,7 @@ isc_atomic_xadd(isc_int32_t *p, int val)
  */
 static inline void
 isc_atomic_store(isc_int32_t *p, isc_int32_t val) {
-	*p = val;
+	atomic_store(p, val);
 }
 
 /*
@@ -64,31 +49,8 @@ isc_atomic_store(isc_int32_t *p, isc_int
  */
 static inline isc_int32_t
 isc_atomic_cmpxchg(isc_int32_t *p, int cmpval, int val) {
-	isc_int32_t orig;
-	isc_int32_t tmp;
-
-	__asm__ __volatile__ (
-	"	.set	push		\n"
-	"	.set	mips2		\n"
-	"	.set	noreorder	\n"
-	"	.set	noat		\n"
-	"1:	ll	$1, %1		\n"
-	"	bne	$1, %3, 2f	\n"
-	"	move	%2, %4		\n"
-	"	sc	%2, %1		\n"
-	"	beqz	%2, 1b		\n"
-	"2:	move	%0, $1		\n"
-	"	.set	pop		\n"
-	: "=&r"(orig), "+R" (*p), "=r" (tmp)
-	: "r"(cmpval), "r"(val)
-	: "memory");
-
-	return (orig);
+	atomic_compare_exchange_strong(p, &cmpval, val);
+	return cmpval;
 }
 
-#else /* !ISC_PLATFORM_USEGCCASM */
-
-#error "unsupported compiler.  disable atomic ops by --disable-atomic"
-
-#endif
 #endif /* ISC_ATOMIC_H */

Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to