[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#860887: marked as done (unblock: bind9/1:9.10.3.dfsg.P4-12.2)

Your message dated Fri, 21 Apr 2017 16:54:00 +0000
with message-id <a15542ba-70f7-600d-50eb-c9101a237f47@thykier.net>
and subject line Re: Bug#860887: unblock: bind9/1:9.10.3.dfsg.P4-12.2
has caused the Debian Bug report #860887,
regarding unblock: bind9/1:9.10.3.dfsg.P4-12.2
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
860887: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=860887
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
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


--- End Message ---
--- Begin Message --- 
Cyril Brulebois:
> Niels Thykier <niels@thykier.net> (2017-04-21):
>> Ack from here, CC'ing KiBi for a d-i ack.
> 
> No objections, thanks.
> 
> 
> KiBi.
> 

Unblocked, thanks.

~Niels

--- End Message ---
Reply to: