Re: [PATCH] MIPS: unsigned result is always greater than 0

All of lore.kernel.org
 help / color / mirror / Atom feed

From: Ralf Baechle <ralf@linux-mips.org>
To: "David VomLehn (dvomlehn)" <dvomlehn@cisco.com>
Cc: Roel Kluin <roel.kluin@gmail.com>, linux-mips@linux-mips.org
Subject: Re: [PATCH] MIPS: unsigned result is always greater than 0
Date: Mon, 12 Jan 2009 01:26:54 +0000	[thread overview]
Message-ID: <20090112012654.GB27976@linux-mips.org> (raw)
In-Reply-To: <FF038EB85946AA46B18DFEE6E6F8A2896606BB@xmb-rtp-218.amer.cisco.com>

On Tue, Jan 06, 2009 at 04:46:19PM -0500, David VomLehn (dvomlehn) wrote:

> I agree that the code as it exists is wrong, but, as I see it, the
> problem is that the type of result should be changed from unsigned long
> to int. This fixes the comparison so it works correctly. In addition,
> such a change means that result would be the same type as the counter
> element of atomic_t, avoiding possible surprises should longs be larger
> than ints.

Originally that code was intended only for R2000-class processors.  But
LL/SC don't work correctly on XKPHYS addresses on some R5000-class
processors, so we avoid these instructions on the affected processors
running 64-bit kernels and use the fallback method of disabling interrupts
anyway, so long indeed might be longer than int.  What in the end saves
us is that atomic_sub_if_positive's only caller atomic_dec_if_positive
is only being used by the RM9000 watchdog driver and the RM9000 doesn't
suffer from the mentioned CPU bug.

Still no reason not to fix it.  I changed the intermediate variables used in
all functions that operate on atomic_t to int rsp. long for the atomic64_*
functions for consistency - which as a nice side effect also shaves of
almost 2kB from the kernel due to the elemination of gcc-generated
non-sense sign extension code.

  Ralf

MIPS: atomic_*(): Change type of intermediate variables.

This shaves of 1912 bytes of an IP27 defconfig kernel and avoids
unexpected overflow behaviour in atomic_sub_if_positive.  Apply the same
changes to the atomic64_* functions for consistency.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/arch/mips/include/asm/atomic.h b/arch/mips/include/asm/atomic.h
index c996c3b..1b332e1 100644
--- a/arch/mips/include/asm/atomic.h
+++ b/arch/mips/include/asm/atomic.h
@@ -50,7 +50,7 @@
 static __inline__ void atomic_add(int i, atomic_t * v)
 {
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -62,7 +62,7 @@ static __inline__ void atomic_add(int i, atomic_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -95,7 +95,7 @@ static __inline__ void atomic_add(int i, atomic_t * v)
 static __inline__ void atomic_sub(int i, atomic_t * v)
 {
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -107,7 +107,7 @@ static __inline__ void atomic_sub(int i, atomic_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -135,12 +135,12 @@ static __inline__ void atomic_sub(int i, atomic_t * v)
  */
 static __inline__ int atomic_add_return(int i, atomic_t * v)
 {
-	unsigned long result;
+	int result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -154,7 +154,7 @@ static __inline__ int atomic_add_return(int i, atomic_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -187,12 +187,12 @@ static __inline__ int atomic_add_return(int i, atomic_t * v)
 
 static __inline__ int atomic_sub_return(int i, atomic_t * v)
 {
-	unsigned long result;
+	int result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -206,7 +206,7 @@ static __inline__ int atomic_sub_return(int i, atomic_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -247,12 +247,12 @@ static __inline__ int atomic_sub_return(int i, atomic_t * v)
  */
 static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
 {
-	unsigned long result;
+	int result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -270,7 +270,7 @@ static __inline__ int atomic_sub_if_positive(int i, atomic_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		int temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -429,7 +429,7 @@ static __inline__ int atomic_add_unless(atomic_t *v, int a, int u)
 static __inline__ void atomic64_add(long i, atomic64_t * v)
 {
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -441,7 +441,7 @@ static __inline__ void atomic64_add(long i, atomic64_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -474,7 +474,7 @@ static __inline__ void atomic64_add(long i, atomic64_t * v)
 static __inline__ void atomic64_sub(long i, atomic64_t * v)
 {
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -486,7 +486,7 @@ static __inline__ void atomic64_sub(long i, atomic64_t * v)
 		: "=&r" (temp), "=m" (v->counter)
 		: "Ir" (i), "m" (v->counter));
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -514,12 +514,12 @@ static __inline__ void atomic64_sub(long i, atomic64_t * v)
  */
 static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 {
-	unsigned long result;
+	long result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -533,7 +533,7 @@ static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -566,12 +566,12 @@ static __inline__ long atomic64_add_return(long i, atomic64_t * v)
 
 static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
 {
-	unsigned long result;
+	long result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -585,7 +585,7 @@ static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -626,12 +626,12 @@ static __inline__ long atomic64_sub_return(long i, atomic64_t * v)
  */
 static __inline__ long atomic64_sub_if_positive(long i, atomic64_t * v)
 {
-	unsigned long result;
+	long result;
 
 	smp_llsc_mb();
 
 	if (cpu_has_llsc && R10000_LLSC_WAR) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"
@@ -649,7 +649,7 @@ static __inline__ long atomic64_sub_if_positive(long i, atomic64_t * v)
 		: "Ir" (i), "m" (v->counter)
 		: "memory");
 	} else if (cpu_has_llsc) {
-		unsigned long temp;
+		long temp;
 
 		__asm__ __volatile__(
 		"	.set	mips3					\n"

     prev parent reply	other threads:[~2009-01-12  1:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-02 15:09 [PATCH] MIPS: unsigned result is always greater than 0 Roel Kluin
2009-01-06 21:46 ` David VomLehn (dvomlehn)
2009-01-06 21:46   ` David VomLehn (dvomlehn)
2009-01-12  1:26   ` Ralf Baechle [this message]

find likely ancestor, descendant, or conflicting patches for this message:
( dfblob:c996c3b dfblob:1b332e1 )
 OR (
bs:"Re: [PATCH] MIPS: unsigned result is always greater than 0" )
	(help)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090112012654.GB27976@linux-mips.org \
    --to=ralf@linux-mips.org \
    --cc=dvomlehn@cisco.com \
    --cc=linux-mips@linux-mips.org \
    --cc=roel.kluin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Be sure your reply has a Subject: header at the top and a blank line before the message body.

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.