linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michal Nazarewicz <mina86@mina86.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@kernel.org>
Cc: John Stultz <john.stultz@linaro.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Steven Rostedt \(Red Hat\)" <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH] kernel.h: make abs() work with 64-bit types
Date: Wed, 16 Sep 2015 14:57:59 +0200	[thread overview]
Message-ID: <xa1tzj0mmsh4.fsf@mina86.com> (raw)
In-Reply-To: <CA+55aFzjZqt-8HhvbvLb7s3vR_G3Hjmf=buKBBKTnmKm+KVK-Q@mail.gmail.com>

For 64-bit arguments, abs macro casts it to an int which leads to lost
precision and may cause incorrect results.  To deal with 64-bit types
abs64 macro has been introduced but still there are places where abs
macro is used incorrectly.

To deal with the problem, expand abs macro such that it operates on s64
type when dealing with 64-bit types while still returning long when
dealing with smaller types.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 include/linux/kernel.h | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

On Tue, Sep 15 2015, Linus Torvalds wrote:
> So I think the "auto-expand to 's64' using __builtin_choose_expr()" is
> the preferable model, and get rid of abs64() entirely. It has very few
> uses.

Compile tested (with ‘make allmodconfig && make bzImage modules’ on
x86_64) only.

The 32-bit case could be further ‘simplified’ with:

		typeof(__builtin_choose_expr(sizeof(x) == sizeof(long), \
		                             1L, 1)) __x = (x);         \
		(long)(__x < 0 ? -__x : __x);  

but I don’t suppose the few saved lines (and hacker-cred ;) ) are worth
added confusion or risk of angry developer attacking me with a blunt
instrument after they had to debug the code for previous fortnight.

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 5582410..f985d16 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -200,28 +200,31 @@ extern int _cond_resched(void);
 
 #define might_sleep_if(cond) do { if (cond) might_sleep(); } while (0)
 
-/*
- * abs() handles unsigned and signed longs, ints, shorts and chars.  For all
- * input types abs() returns a signed long.
- * abs() should not be used for 64-bit types (s64, u64, long long) - use abs64()
- * for those.
+/**
+ * abs - return absolute value of an argument
+ * @x: the value.  If it is unsigned type, it is converted to signed type first
+ *   (s64, long or int depending on its size).
+ *
+ * Return: an absolute value of x.  If x is 64-bit, macro's return type is s64,
+ *   otherwise it is signed long.
  */
-#define abs(x) ({						\
-		long ret;					\
-		if (sizeof(x) == sizeof(long)) {		\
-			long __x = (x);				\
-			ret = (__x < 0) ? -__x : __x;		\
-		} else {					\
-			int __x = (x);				\
-			ret = (__x < 0) ? -__x : __x;		\
-		}						\
-		ret;						\
-	})
-
-#define abs64(x) ({				\
-		s64 __x = (x);			\
-		(__x < 0) ? -__x : __x;		\
-	})
+#define abs(x) __builtin_choose_expr(sizeof(x) == sizeof(s64), ({	\
+		s64 __x = (x);						\
+		(__x < 0) ? -__x : __x;					\
+	}), ({								\
+		long ret;						\
+		if (sizeof(x) == sizeof(long)) {			\
+			long __x = (x);					\
+			ret = (__x < 0) ? -__x : __x;			\
+		} else {						\
+			int __x = (x);					\
+			ret = (__x < 0) ? -__x : __x;			\
+		}							\
+		ret;							\
+	}))
+
+/* Deprecated, use abs instead. */
+#define abs64(x) abs((s64)(x))
 
 /**
  * reciprocal_scale - "scale" a value into range [0, ep_ro)
-- 
2.6.0.rc0.131.gf624c3d


  reply	other threads:[~2015-09-16 12:58 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-15  1:05 [RFC][PATCH 0/5] Fixes for abs() usage on 64bit values John Stultz
2015-09-15  1:05 ` [RFC][PATCH 1/5] clocksource: Fix abs() usage w/ " John Stultz
2015-10-02 20:57   ` [tip:timers/urgent] " tip-bot for John Stultz
2015-09-15  1:05 ` [RFC][PATCH 2/5] time: Fix abs() usage with 64-bit values John Stultz
2015-09-15  1:05 ` [RFC][PATCH 3/5] ext4: Fix abs() usage in ext4_mb_check_group_pa John Stultz
2015-09-15  1:05 ` [RFC][PATCH 4/5] percpu: Fix abs() usage in percpu_counter_compare() John Stultz
2015-09-15  1:05 ` [RFC][PATCH 5/5] abs(): Provide build error on passing 64bit value to abs() John Stultz
2015-09-15  5:22   ` Ingo Molnar
2015-09-15 23:52     ` Linus Torvalds
2015-09-16 12:57       ` Michal Nazarewicz [this message]
2015-09-18  3:12         ` [PATCH] kernel.h: make abs() work with 64-bit types John Stultz
2015-09-15  1:49 ` [RFC][PATCH 0/5] Fixes for abs() usage on 64bit values Tejun Heo
2015-09-15  3:27   ` John Stultz
2015-09-15  3:46     ` Tejun Heo
2015-09-15 12:09       ` Jeff Epler
2015-09-15 21:21       ` Andrew Morton
2015-09-15 22:54         ` Michal Nazarewicz
2015-09-15  5:20     ` Ingo Molnar
2015-09-15 23:43       ` Linus Torvalds

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=xa1tzj0mmsh4.fsf@mina86.com \
    --to=mina86@mina86.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).