Package: release.debian.org Severity: normal User: release.debian.org@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 <jcowgill@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 <ths@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 <lamont@debian.org>
+Description: Replace MIPS atomics assembly with calls to C11 atomic functions
+ This fixes various hangs and crashes on MIPS.
+Author: James Cowgill <jcowgill@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