mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 00/18] lib: bitmap: Various improvements
@ 2014-07-03 22:42 Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 01/18] lib: bitmap: Make nbits parameter of bitmap_empty unsigned Rasmus Villemoes
                   ` (18 more replies)
  0 siblings, 19 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

Many functions in lib/bitmap.c start with an expression such as lim =
bits/BITS_PER_LONG. Since bits has type (signed) int, and since gcc
cannot know that it is in fact non-negative, it generates worse code
than it could. These patches, mostly consisting of changing various
parameters to unsigned, gives a slight overall code reduction:

add/remove: 1/1 grow/shrink: 8/16 up/down: 251/-414 (-163)
function                                     old     new   delta
tick_device_uses_broadcast                   335     425     +90
__irq_alloc_descs                            498     554     +56
__bitmap_andnot                               73     115     +42
__bitmap_and                                  70     101     +31
bitmap_weight                                  -      11     +11
copy_hugetlb_page_range                      752     762     +10
follow_hugetlb_page                          846     854      +8
hugetlb_init                                1415    1417      +2
hugetlb_nrpages_setup                        130     131      +1
hugetlb_add_hstate                           377     376      -1
bitmap_allocate_region                        82      80      -2
select_task_rq_fair                         2202    2191     -11
hweight_long                                  66      55     -11
__reg_op                                     230     219     -11
dm_stats_message                            2849    2833     -16
bitmap_parselist                              92      74     -18
__bitmap_weight                              115      97     -18
__bitmap_subset                              153     129     -24
__bitmap_full                                128     104     -24
__bitmap_empty                               120      96     -24
bitmap_set                                   179     149     -30
bitmap_clear                                 185     155     -30
__bitmap_equal                               136     105     -31
__bitmap_intersects                          148     108     -40
__bitmap_complement                          109      67     -42
tick_device_setup_broadcast_func.isra         81       -     -81

[The increases in __bitmap_and{,not} are due to bug fixes 17/18,18/18.
No idea why bitmap_weight suddenly appears.] While 163 bytes treewide
is insignificant, I believe the bitmap functions are often called with
locks held, so saving even a few cycles might be worth it.

While making these changes, I found a few other things that might be
worth including. 16,17,18 are actual bug fixes. The rest shouldn't
change the behaviour of any of the functions, provided no-one passed
negative nbits values. If something should come up, it should be
fairly bisectable.

A few issues I thought about, but didn't know what to do with:

* Many of the functions misbehave if nbits is compile-time 0; the
  out-of-line functions generally handle 0 correctly. bitmap_fill() is
  particularly bad, whether the 0 is known at compile time or not. It
  would probably be nice to add detection of at least compile-time 0
  and handle that appropriately.

* I didn't change __bitmap_shift_{left,right} to use unsigned because
  I want to fully understand why the algorithm works before making
  that change. However, AFAICT, they behave correctly for all
  (positive) shift amounts. This is not the case for the
  small_const_nbits versions. If for example nbits = n =
  BITS_PER_LONG, the shift operators turn into no-ops (at least on
  x86), so one get *dst = *src, whereas one would expect to get
  *dst=0. That difference in behaviour is somewhat annoying.

Rasmus Villemoes (18):
  lib: bitmap: Make nbits parameter of bitmap_empty unsigned
  lib: bitmap: Make nbits parameter of bitmap_full unsigned
  lib: bitmap: Make nbits parameter of bitmap_equal unsigned
  lib: bitmap: Make nbits parameter of bitmap_complement unsigned
  lib: bitmap: Remove unnecessary mask from bitmap_complement
  lib: bitmap: Make nbits parameter of bitmap_{and,or,xor,andnot}
    unsigned
  lib: bitmap: Make nbits parameter of bitmap_intersects unsigned
  lib: bitmap: Make nbits parameter of bitmap_subset unsigned
  lib: bitmap: Make nbits parameter of bitmap_weight unsigned
  lib: bitmap: Make the start index of bitmap_set unsigned
  lib: bitmap: Make the start index of bitmap_clear unsigned
  lib: bitmap: Simplify bitmap_parselist
  lib: bitmap: Fix typo in kerneldoc for bitmap_pos_to_ord
  lib: bitmap: Change parameter of bitmap_*_region to unsigned
  lib: bitmap: Micro-optimize bitmap_allocate_region
  lib: bitmap: Add missing mask in bitmap_shift_right
  lib: bitmap: Add missing mask in bitmap_and
  lib: bitmap: Add missing mask in bitmap_andnot

 include/linux/bitmap.h |  62 +++++++++++++--------------
 lib/bitmap.c           | 111 +++++++++++++++++++++++++------------------------
 2 files changed, 87 insertions(+), 86 deletions(-)

-- 
1.9.2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 01/18] lib: bitmap: Make nbits parameter of bitmap_empty unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 02/18] lib: bitmap: Make nbits parameter of bitmap_full unsigned Rasmus Villemoes
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 4 ++--
 lib/bitmap.c           | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 7ad6345..3d3fd6b 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -88,7 +88,7 @@
  * lib/bitmap.c provides these functions:
  */
 
-extern int __bitmap_empty(const unsigned long *bitmap, int bits);
+extern int __bitmap_empty(const unsigned long *bitmap, unsigned int nbits);
 extern int __bitmap_full(const unsigned long *bitmap, int bits);
 extern int __bitmap_equal(const unsigned long *bitmap1,
                 	const unsigned long *bitmap2, int bits);
@@ -257,7 +257,7 @@ static inline int bitmap_subset(const unsigned long *src1,
 		return __bitmap_subset(src1, src2, nbits);
 }
 
-static inline int bitmap_empty(const unsigned long *src, int nbits)
+static inline int bitmap_empty(const unsigned long *src, unsigned nbits)
 {
 	if (small_const_nbits(nbits))
 		return ! (*src & BITMAP_LAST_WORD_MASK(nbits));
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 06f7e4f..3789110 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -40,9 +40,9 @@
  * for the best explanations of this ordering.
  */
 
-int __bitmap_empty(const unsigned long *bitmap, int bits)
+int __bitmap_empty(const unsigned long *bitmap, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		if (bitmap[k])
 			return 0;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 02/18] lib: bitmap: Make nbits parameter of bitmap_full unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 01/18] lib: bitmap: Make nbits parameter of bitmap_empty unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 03/18] lib: bitmap: Make nbits parameter of bitmap_equal unsigned Rasmus Villemoes
                   ` (16 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 4 ++--
 lib/bitmap.c           | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 3d3fd6b..bc7e520 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -89,7 +89,7 @@
  */
 
 extern int __bitmap_empty(const unsigned long *bitmap, unsigned int nbits);
-extern int __bitmap_full(const unsigned long *bitmap, int bits);
+extern int __bitmap_full(const unsigned long *bitmap, unsigned int nbits);
 extern int __bitmap_equal(const unsigned long *bitmap1,
                 	const unsigned long *bitmap2, int bits);
 extern void __bitmap_complement(unsigned long *dst, const unsigned long *src,
@@ -265,7 +265,7 @@ static inline int bitmap_empty(const unsigned long *src, unsigned nbits)
 		return __bitmap_empty(src, nbits);
 }
 
-static inline int bitmap_full(const unsigned long *src, int nbits)
+static inline int bitmap_full(const unsigned long *src, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return ! (~(*src) & BITMAP_LAST_WORD_MASK(nbits));
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 3789110..9859f38 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -55,9 +55,9 @@ int __bitmap_empty(const unsigned long *bitmap, unsigned int bits)
 }
 EXPORT_SYMBOL(__bitmap_empty);
 
-int __bitmap_full(const unsigned long *bitmap, int bits)
+int __bitmap_full(const unsigned long *bitmap, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		if (~bitmap[k])
 			return 0;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 03/18] lib: bitmap: Make nbits parameter of bitmap_equal unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 01/18] lib: bitmap: Make nbits parameter of bitmap_empty unsigned Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 02/18] lib: bitmap: Make nbits parameter of bitmap_full unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 04/18] lib: bitmap: Make nbits parameter of bitmap_complement unsigned Rasmus Villemoes
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 2 +-
 lib/bitmap.c           | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index bc7e520..1e0f46c 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -91,7 +91,7 @@
 extern int __bitmap_empty(const unsigned long *bitmap, unsigned int nbits);
 extern int __bitmap_full(const unsigned long *bitmap, unsigned int nbits);
 extern int __bitmap_equal(const unsigned long *bitmap1,
-                	const unsigned long *bitmap2, int bits);
+			  const unsigned long *bitmap2, unsigned int nbits);
 extern void __bitmap_complement(unsigned long *dst, const unsigned long *src,
 			int bits);
 extern void __bitmap_shift_right(unsigned long *dst,
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 9859f38..d6bb955 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -71,9 +71,9 @@ int __bitmap_full(const unsigned long *bitmap, unsigned int bits)
 EXPORT_SYMBOL(__bitmap_full);
 
 int __bitmap_equal(const unsigned long *bitmap1,
-		const unsigned long *bitmap2, int bits)
+		const unsigned long *bitmap2, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		if (bitmap1[k] != bitmap2[k])
 			return 0;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 04/18] lib: bitmap: Make nbits parameter of bitmap_complement unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (2 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 03/18] lib: bitmap: Make nbits parameter of bitmap_equal unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 05/18] lib: bitmap: Remove unnecessary mask from bitmap_complement Rasmus Villemoes
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 6 +++---
 lib/bitmap.c           | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 1e0f46c..21fb52f 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -93,7 +93,7 @@ extern int __bitmap_full(const unsigned long *bitmap, unsigned int nbits);
 extern int __bitmap_equal(const unsigned long *bitmap1,
 			  const unsigned long *bitmap2, unsigned int nbits);
 extern void __bitmap_complement(unsigned long *dst, const unsigned long *src,
-			int bits);
+			unsigned int nbits);
 extern void __bitmap_shift_right(unsigned long *dst,
                         const unsigned long *src, int shift, int bits);
 extern void __bitmap_shift_left(unsigned long *dst,
@@ -222,7 +222,7 @@ static inline int bitmap_andnot(unsigned long *dst, const unsigned long *src1,
 }
 
 static inline void bitmap_complement(unsigned long *dst, const unsigned long *src,
-			int nbits)
+			unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		*dst = ~(*src) & BITMAP_LAST_WORD_MASK(nbits);
@@ -231,7 +231,7 @@ static inline void bitmap_complement(unsigned long *dst, const unsigned long *sr
 }
 
 static inline int bitmap_equal(const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return ! ((*src1 ^ *src2) & BITMAP_LAST_WORD_MASK(nbits));
diff --git a/lib/bitmap.c b/lib/bitmap.c
index d6bb955..0f2f845 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -86,9 +86,9 @@ int __bitmap_equal(const unsigned long *bitmap1,
 }
 EXPORT_SYMBOL(__bitmap_equal);
 
-void __bitmap_complement(unsigned long *dst, const unsigned long *src, int bits)
+void __bitmap_complement(unsigned long *dst, const unsigned long *src, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		dst[k] = ~src[k];
 
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 05/18] lib: bitmap: Remove unnecessary mask from bitmap_complement
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (3 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 04/18] lib: bitmap: Make nbits parameter of bitmap_complement unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 06/18] lib: bitmap: Make nbits parameter of bitmap_{and,or,xor,andnot} unsigned Rasmus Villemoes
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

Since the extra bits are "don't care", there is no reason to mask the
last word to the used bits when complementing. This shaves off yet a
few bytes.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 2 +-
 lib/bitmap.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 21fb52f..f42d72d 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -225,7 +225,7 @@ static inline void bitmap_complement(unsigned long *dst, const unsigned long *sr
 			unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
-		*dst = ~(*src) & BITMAP_LAST_WORD_MASK(nbits);
+		*dst = ~(*src);
 	else
 		__bitmap_complement(dst, src, nbits);
 }
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 0f2f845..4387e3c 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -93,7 +93,7 @@ void __bitmap_complement(unsigned long *dst, const unsigned long *src, unsigned
 		dst[k] = ~src[k];
 
 	if (bits % BITS_PER_LONG)
-		dst[k] = ~src[k] & BITMAP_LAST_WORD_MASK(bits);
+		dst[k] = ~src[k];
 }
 EXPORT_SYMBOL(__bitmap_complement);
 
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 06/18] lib: bitmap: Make nbits parameter of bitmap_{and,or,xor,andnot} unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (4 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 05/18] lib: bitmap: Remove unnecessary mask from bitmap_complement Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 07/18] lib: bitmap: Make nbits parameter of bitmap_intersects unsigned Rasmus Villemoes
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

This change is only for consistency with the changes to the other
bitmap_* functions; it doesn't change the size of the generated code:
inside BITS_TO_LONGS there is a sizeof(long), which causes bits to be
interpreted as unsigned anyway.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 16 ++++++++--------
 lib/bitmap.c           | 24 ++++++++++++------------
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index f42d72d..7048782 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -99,13 +99,13 @@ extern void __bitmap_shift_right(unsigned long *dst,
 extern void __bitmap_shift_left(unsigned long *dst,
                         const unsigned long *src, int shift, int bits);
 extern int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_intersects(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, int bits);
 extern int __bitmap_subset(const unsigned long *bitmap1,
@@ -188,7 +188,7 @@ static inline void bitmap_copy(unsigned long *dst, const unsigned long *src,
 }
 
 static inline int bitmap_and(unsigned long *dst, const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return (*dst = *src1 & *src2) != 0;
@@ -196,7 +196,7 @@ static inline int bitmap_and(unsigned long *dst, const unsigned long *src1,
 }
 
 static inline void bitmap_or(unsigned long *dst, const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		*dst = *src1 | *src2;
@@ -205,7 +205,7 @@ static inline void bitmap_or(unsigned long *dst, const unsigned long *src1,
 }
 
 static inline void bitmap_xor(unsigned long *dst, const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		*dst = *src1 ^ *src2;
@@ -214,7 +214,7 @@ static inline void bitmap_xor(unsigned long *dst, const unsigned long *src1,
 }
 
 static inline int bitmap_andnot(unsigned long *dst, const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return (*dst = *src1 & ~(*src2)) != 0;
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 4387e3c..0320737 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -182,10 +182,10 @@ void __bitmap_shift_left(unsigned long *dst,
 EXPORT_SYMBOL(__bitmap_shift_left);
 
 int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+				const unsigned long *bitmap2, unsigned int bits)
 {
-	int k;
-	int nr = BITS_TO_LONGS(bits);
+	unsigned int k;
+	unsigned int nr = BITS_TO_LONGS(bits);
 	unsigned long result = 0;
 
 	for (k = 0; k < nr; k++)
@@ -195,10 +195,10 @@ int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1,
 EXPORT_SYMBOL(__bitmap_and);
 
 void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+				const unsigned long *bitmap2, unsigned int bits)
 {
-	int k;
-	int nr = BITS_TO_LONGS(bits);
+	unsigned int k;
+	unsigned int nr = BITS_TO_LONGS(bits);
 
 	for (k = 0; k < nr; k++)
 		dst[k] = bitmap1[k] | bitmap2[k];
@@ -206,10 +206,10 @@ void __bitmap_or(unsigned long *dst, const unsigned long *bitmap1,
 EXPORT_SYMBOL(__bitmap_or);
 
 void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+				const unsigned long *bitmap2, unsigned int bits)
 {
-	int k;
-	int nr = BITS_TO_LONGS(bits);
+	unsigned int k;
+	unsigned int nr = BITS_TO_LONGS(bits);
 
 	for (k = 0; k < nr; k++)
 		dst[k] = bitmap1[k] ^ bitmap2[k];
@@ -217,10 +217,10 @@ void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1,
 EXPORT_SYMBOL(__bitmap_xor);
 
 int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+				const unsigned long *bitmap2, unsigned int bits)
 {
-	int k;
-	int nr = BITS_TO_LONGS(bits);
+	unsigned int k;
+	unsigned int nr = BITS_TO_LONGS(bits);
 	unsigned long result = 0;
 
 	for (k = 0; k < nr; k++)
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 07/18] lib: bitmap: Make nbits parameter of bitmap_intersects unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (5 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 06/18] lib: bitmap: Make nbits parameter of bitmap_{and,or,xor,andnot} unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 08/18] lib: bitmap: Make nbits parameter of bitmap_subset unsigned Rasmus Villemoes
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 4 ++--
 lib/bitmap.c           | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 7048782..2f3f3a4 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -107,7 +107,7 @@ extern void __bitmap_xor(unsigned long *dst, const unsigned long *bitmap1,
 extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_intersects(const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_subset(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, int bits);
 extern int __bitmap_weight(const unsigned long *bitmap, int bits);
@@ -240,7 +240,7 @@ static inline int bitmap_equal(const unsigned long *src1,
 }
 
 static inline int bitmap_intersects(const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return ((*src1 & *src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0;
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 0320737..e85daa9 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -230,9 +230,9 @@ int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 EXPORT_SYMBOL(__bitmap_andnot);
 
 int __bitmap_intersects(const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+			const unsigned long *bitmap2, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		if (bitmap1[k] & bitmap2[k])
 			return 1;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 08/18] lib: bitmap: Make nbits parameter of bitmap_subset unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (6 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 07/18] lib: bitmap: Make nbits parameter of bitmap_intersects unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 09/18] lib: bitmap: Make nbits parameter of bitmap_weight unsigned Rasmus Villemoes
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a
negative bit-count, this shouldn't affect the semantics.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 4 ++--
 lib/bitmap.c           | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 2f3f3a4..87e88f7 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -109,7 +109,7 @@ extern int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 extern int __bitmap_intersects(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_subset(const unsigned long *bitmap1,
-			const unsigned long *bitmap2, int bits);
+			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_weight(const unsigned long *bitmap, int bits);
 
 extern void bitmap_set(unsigned long *map, int i, int len);
@@ -249,7 +249,7 @@ static inline int bitmap_intersects(const unsigned long *src1,
 }
 
 static inline int bitmap_subset(const unsigned long *src1,
-			const unsigned long *src2, int nbits)
+			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return ! ((*src1 & ~(*src2)) & BITMAP_LAST_WORD_MASK(nbits));
diff --git a/lib/bitmap.c b/lib/bitmap.c
index e85daa9..c9bff53 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -245,9 +245,9 @@ int __bitmap_intersects(const unsigned long *bitmap1,
 EXPORT_SYMBOL(__bitmap_intersects);
 
 int __bitmap_subset(const unsigned long *bitmap1,
-				const unsigned long *bitmap2, int bits)
+		    const unsigned long *bitmap2, unsigned int bits)
 {
-	int k, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
 	for (k = 0; k < lim; ++k)
 		if (bitmap1[k] & ~bitmap2[k])
 			return 0;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 09/18] lib: bitmap: Make nbits parameter of bitmap_weight unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (7 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 08/18] lib: bitmap: Make nbits parameter of bitmap_subset unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 10/18] lib: bitmap: Make the start index of bitmap_set unsigned Rasmus Villemoes
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "nbits" is non-negative. Since no-one passes a negative
bit-count, this shouldn't affect the semantics.

I didn't change the return type, since that might change the semantics
of some expression containing a call to bitmap_weight(). Certainly an
int is capable of holding the result.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 4 ++--
 lib/bitmap.c           | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 87e88f7..64b0ebe 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -110,7 +110,7 @@ extern int __bitmap_intersects(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_subset(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, unsigned int nbits);
-extern int __bitmap_weight(const unsigned long *bitmap, int bits);
+extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits);
 
 extern void bitmap_set(unsigned long *map, int i, int len);
 extern void bitmap_clear(unsigned long *map, int start, int nr);
@@ -273,7 +273,7 @@ static inline int bitmap_full(const unsigned long *src, unsigned int nbits)
 		return __bitmap_full(src, nbits);
 }
 
-static inline int bitmap_weight(const unsigned long *src, int nbits)
+static inline int bitmap_weight(const unsigned long *src, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
 		return hweight_long(*src & BITMAP_LAST_WORD_MASK(nbits));
diff --git a/lib/bitmap.c b/lib/bitmap.c
index c9bff53..f69435c 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -259,9 +259,10 @@ int __bitmap_subset(const unsigned long *bitmap1,
 }
 EXPORT_SYMBOL(__bitmap_subset);
 
-int __bitmap_weight(const unsigned long *bitmap, int bits)
+int __bitmap_weight(const unsigned long *bitmap, unsigned int bits)
 {
-	int k, w = 0, lim = bits/BITS_PER_LONG;
+	unsigned int k, lim = bits/BITS_PER_LONG;
+	int w = 0;
 
 	for (k = 0; k < lim; k++)
 		w += hweight_long(bitmap[k]);
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 10/18] lib: bitmap: Make the start index of bitmap_set unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (8 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 09/18] lib: bitmap: Make nbits parameter of bitmap_weight unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 11/18] lib: bitmap: Make the start index of bitmap_clear unsigned Rasmus Villemoes
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "start" is non-negative.

Also, use the names "start" and "len" for the two parameters in both
header file and implementation, instead of the previous mix.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h |  2 +-
 lib/bitmap.c           | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 64b0ebe..ad2c67d 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -112,7 +112,7 @@ extern int __bitmap_subset(const unsigned long *bitmap1,
 			const unsigned long *bitmap2, unsigned int nbits);
 extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits);
 
-extern void bitmap_set(unsigned long *map, int i, int len);
+extern void bitmap_set(unsigned long *map, unsigned int start, int len);
 extern void bitmap_clear(unsigned long *map, int start, int nr);
 extern unsigned long bitmap_find_next_zero_area(unsigned long *map,
 					 unsigned long size,
diff --git a/lib/bitmap.c b/lib/bitmap.c
index f69435c..2a3a92f 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -274,21 +274,21 @@ int __bitmap_weight(const unsigned long *bitmap, unsigned int bits)
 }
 EXPORT_SYMBOL(__bitmap_weight);
 
-void bitmap_set(unsigned long *map, int start, int nr)
+void bitmap_set(unsigned long *map, unsigned int start, int len)
 {
 	unsigned long *p = map + BIT_WORD(start);
-	const int size = start + nr;
+	const unsigned int size = start + len;
 	int bits_to_set = BITS_PER_LONG - (start % BITS_PER_LONG);
 	unsigned long mask_to_set = BITMAP_FIRST_WORD_MASK(start);
 
-	while (nr - bits_to_set >= 0) {
+	while (len - bits_to_set >= 0) {
 		*p |= mask_to_set;
-		nr -= bits_to_set;
+		len -= bits_to_set;
 		bits_to_set = BITS_PER_LONG;
 		mask_to_set = ~0UL;
 		p++;
 	}
-	if (nr) {
+	if (len) {
 		mask_to_set &= BITMAP_LAST_WORD_MASK(size);
 		*p |= mask_to_set;
 	}
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 11/18] lib: bitmap: Make the start index of bitmap_clear unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (9 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 10/18] lib: bitmap: Make the start index of bitmap_set unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 12/18] lib: bitmap: Simplify bitmap_parselist Rasmus Villemoes
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

The compiler can generate slightly smaller and simpler code when it
knows that "start" is non-negative.

Also, use the names "start" and "len" for the two parameters for
consistency with bitmap_set.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h |  2 +-
 lib/bitmap.c           | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index ad2c67d..83c1c7d 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -113,7 +113,7 @@ extern int __bitmap_subset(const unsigned long *bitmap1,
 extern int __bitmap_weight(const unsigned long *bitmap, unsigned int nbits);
 
 extern void bitmap_set(unsigned long *map, unsigned int start, int len);
-extern void bitmap_clear(unsigned long *map, int start, int nr);
+extern void bitmap_clear(unsigned long *map, unsigned int start, int len);
 extern unsigned long bitmap_find_next_zero_area(unsigned long *map,
 					 unsigned long size,
 					 unsigned long start,
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 2a3a92f..5d25403 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -295,21 +295,21 @@ void bitmap_set(unsigned long *map, unsigned int start, int len)
 }
 EXPORT_SYMBOL(bitmap_set);
 
-void bitmap_clear(unsigned long *map, int start, int nr)
+void bitmap_clear(unsigned long *map, unsigned int start, int len)
 {
 	unsigned long *p = map + BIT_WORD(start);
-	const int size = start + nr;
+	const unsigned int size = start + len;
 	int bits_to_clear = BITS_PER_LONG - (start % BITS_PER_LONG);
 	unsigned long mask_to_clear = BITMAP_FIRST_WORD_MASK(start);
 
-	while (nr - bits_to_clear >= 0) {
+	while (len - bits_to_clear >= 0) {
 		*p &= ~mask_to_clear;
-		nr -= bits_to_clear;
+		len -= bits_to_clear;
 		bits_to_clear = BITS_PER_LONG;
 		mask_to_clear = ~0UL;
 		p++;
 	}
-	if (nr) {
+	if (len) {
 		mask_to_clear &= BITMAP_LAST_WORD_MASK(size);
 		*p &= ~mask_to_clear;
 	}
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 12/18] lib: bitmap: Simplify bitmap_parselist
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (10 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 11/18] lib: bitmap: Make the start index of bitmap_clear unsigned Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:42 ` [PATCH 13/18] lib: bitmap: Fix typo in kerneldoc for bitmap_pos_to_ord Rasmus Villemoes
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

We want len to be the index of the first '\n', or the length of the
string if there is no newline. This is a good example of the
usefulness of strchrnul(). Use that instead, thus eliminating a branch
and a call to strlen().

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/bitmap.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index 5d25403..d4b3a6d 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -665,13 +665,8 @@ static int __bitmap_parselist(const char *buf, unsigned int buflen,
 
 int bitmap_parselist(const char *bp, unsigned long *maskp, int nmaskbits)
 {
-	char *nl  = strchr(bp, '\n');
-	int len;
-
-	if (nl)
-		len = nl - bp;
-	else
-		len = strlen(bp);
+	char *nl  = strchrnul(bp, '\n');
+	int len = nl - bp;
 
 	return __bitmap_parselist(bp, len, 0, maskp, nmaskbits);
 }
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 13/18] lib: bitmap: Fix typo in kerneldoc for bitmap_pos_to_ord
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (11 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 12/18] lib: bitmap: Simplify bitmap_parselist Rasmus Villemoes
@ 2014-07-03 22:42 ` Rasmus Villemoes
  2014-07-03 22:43 ` [PATCH 14/18] lib: bitmap: Change parameter of bitmap_*_region to unsigned Rasmus Villemoes
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:42 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

A few lines above, it was stated that positions for non-set bits are
mapped to -1, which is obviously also what the code does.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index d4b3a6d..2714df9 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -712,7 +712,7 @@ EXPORT_SYMBOL(bitmap_parselist_user);
  *
  * If for example, just bits 4 through 7 are set in @buf, then @pos
  * values 4 through 7 will get mapped to 0 through 3, respectively,
- * and other @pos values will get mapped to 0.  When @pos value 7
+ * and other @pos values will get mapped to -1.  When @pos value 7
  * gets mapped to (returns) @ord value 3 in this example, that means
  * that bit 7 is the 3rd (starting with 0th) set bit in @buf.
  *
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 14/18] lib: bitmap: Change parameter of bitmap_*_region to unsigned
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (12 preceding siblings ...)
  2014-07-03 22:42 ` [PATCH 13/18] lib: bitmap: Fix typo in kerneldoc for bitmap_pos_to_ord Rasmus Villemoes
@ 2014-07-03 22:43 ` Rasmus Villemoes
  2014-07-03 22:43 ` [PATCH 15/18] lib: bitmap: Micro-optimize bitmap_allocate_region Rasmus Villemoes
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

Changing the pos parameter of __reg_op to unsigned allows the compiler
to generate slightly smaller and simpler code. Also update its callers
bitmap_*_region to receive and pass unsigned int. The return types of
bitmap_find_free_region and bitmap_allocate_region are still int to
allow a negative error code to be returned. An int is certainly
capable of representing any realistic return value.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h |  6 +++---
 lib/bitmap.c           | 12 ++++++------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 83c1c7d..2100378 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -140,9 +140,9 @@ extern void bitmap_onto(unsigned long *dst, const unsigned long *orig,
 		const unsigned long *relmap, int bits);
 extern void bitmap_fold(unsigned long *dst, const unsigned long *orig,
 		int sz, int bits);
-extern int bitmap_find_free_region(unsigned long *bitmap, int bits, int order);
-extern void bitmap_release_region(unsigned long *bitmap, int pos, int order);
-extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order);
+extern int bitmap_find_free_region(unsigned long *bitmap, unsigned int bits, int order);
+extern void bitmap_release_region(unsigned long *bitmap, unsigned int pos, int order);
+extern int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order);
 extern void bitmap_copy_le(void *dst, const unsigned long *src, int nbits);
 extern int bitmap_ord_to_pos(const unsigned long *bitmap, int n, int bits);
 
diff --git a/lib/bitmap.c b/lib/bitmap.c
index 2714df9..c2f3807 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -1042,7 +1042,7 @@ enum {
 	REG_OP_RELEASE,		/* clear all bits in region */
 };
 
-static int __reg_op(unsigned long *bitmap, int pos, int order, int reg_op)
+static int __reg_op(unsigned long *bitmap, unsigned int pos, int order, int reg_op)
 {
 	int nbits_reg;		/* number of bits in region */
 	int index;		/* index first long of region in bitmap */
@@ -1108,11 +1108,11 @@ done:
  * Return the bit offset in bitmap of the allocated region,
  * or -errno on failure.
  */
-int bitmap_find_free_region(unsigned long *bitmap, int bits, int order)
+int bitmap_find_free_region(unsigned long *bitmap, unsigned int bits, int order)
 {
-	int pos, end;		/* scans bitmap by regions of size order */
+	unsigned int pos, end;		/* scans bitmap by regions of size order */
 
-	for (pos = 0 ; (end = pos + (1 << order)) <= bits; pos = end) {
+	for (pos = 0 ; (end = pos + (1U << order)) <= bits; pos = end) {
 		if (!__reg_op(bitmap, pos, order, REG_OP_ISFREE))
 			continue;
 		__reg_op(bitmap, pos, order, REG_OP_ALLOC);
@@ -1133,7 +1133,7 @@ EXPORT_SYMBOL(bitmap_find_free_region);
  *
  * No return value.
  */
-void bitmap_release_region(unsigned long *bitmap, int pos, int order)
+void bitmap_release_region(unsigned long *bitmap, unsigned int pos, int order)
 {
 	__reg_op(bitmap, pos, order, REG_OP_RELEASE);
 }
@@ -1150,7 +1150,7 @@ EXPORT_SYMBOL(bitmap_release_region);
  * Return 0 on success, or %-EBUSY if specified region wasn't
  * free (not all bits were zero).
  */
-int bitmap_allocate_region(unsigned long *bitmap, int pos, int order)
+int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order)
 {
 	if (!__reg_op(bitmap, pos, order, REG_OP_ISFREE))
 		return -EBUSY;
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 15/18] lib: bitmap: Micro-optimize bitmap_allocate_region
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (13 preceding siblings ...)
  2014-07-03 22:43 ` [PATCH 14/18] lib: bitmap: Change parameter of bitmap_*_region to unsigned Rasmus Villemoes
@ 2014-07-03 22:43 ` Rasmus Villemoes
  2014-07-03 22:43 ` [PATCH 16/18] lib: bitmap: Add missing mask in bitmap_shift_right Rasmus Villemoes
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

__reg_op(..., REG_OP_ALLOC) always returns 0, so we might as well use
that and save an instruction.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 lib/bitmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/bitmap.c b/lib/bitmap.c
index c2f3807..faaf720 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -1154,8 +1154,7 @@ int bitmap_allocate_region(unsigned long *bitmap, unsigned int pos, int order)
 {
 	if (!__reg_op(bitmap, pos, order, REG_OP_ISFREE))
 		return -EBUSY;
-	__reg_op(bitmap, pos, order, REG_OP_ALLOC);
-	return 0;
+	return __reg_op(bitmap, pos, order, REG_OP_ALLOC);
 }
 EXPORT_SYMBOL(bitmap_allocate_region);
 
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 16/18] lib: bitmap: Add missing mask in bitmap_shift_right
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (14 preceding siblings ...)
  2014-07-03 22:43 ` [PATCH 15/18] lib: bitmap: Micro-optimize bitmap_allocate_region Rasmus Villemoes
@ 2014-07-03 22:43 ` Rasmus Villemoes
  2014-07-03 22:43 ` [PATCH 17/18] lib: bitmap: Add missing mask in bitmap_and Rasmus Villemoes
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

There is no guarantee that *src does not contain garbage bits outside
the lower nbits, so we need to mask it before the shift-and-assign.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 2100378..75df61d 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -284,7 +284,7 @@ static inline void bitmap_shift_right(unsigned long *dst,
 			const unsigned long *src, int n, int nbits)
 {
 	if (small_const_nbits(nbits))
-		*dst = *src >> n;
+		*dst = (*src & BITMAP_LAST_WORD_MASK(nbits)) >> n;
 	else
 		__bitmap_shift_right(dst, src, n, nbits);
 }
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 17/18] lib: bitmap: Add missing mask in bitmap_and
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (15 preceding siblings ...)
  2014-07-03 22:43 ` [PATCH 16/18] lib: bitmap: Add missing mask in bitmap_shift_right Rasmus Villemoes
@ 2014-07-03 22:43 ` Rasmus Villemoes
  2014-07-03 22:43 ` [PATCH 18/18] lib: bitmap: Add missing mask in bitmap_andnot Rasmus Villemoes
  2014-07-07 23:20 ` [PATCH 00/18] lib: bitmap: Various improvements Andrew Morton
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

Apparently, bitmap_and is supposed to return whether the new bitmap is
empty. But it didn't take potential garbage bits in the last word into
account.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 2 +-
 lib/bitmap.c           | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 75df61d..3399a9e 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -191,7 +191,7 @@ static inline int bitmap_and(unsigned long *dst, const unsigned long *src1,
 			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
-		return (*dst = *src1 & *src2) != 0;
+		return (*dst = *src1 & *src2 & BITMAP_LAST_WORD_MASK(nbits)) != 0;
 	return __bitmap_and(dst, src1, src2, nbits);
 }
 
diff --git a/lib/bitmap.c b/lib/bitmap.c
index faaf720..ce2ec80 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -185,11 +185,14 @@ int __bitmap_and(unsigned long *dst, const unsigned long *bitmap1,
 				const unsigned long *bitmap2, unsigned int bits)
 {
 	unsigned int k;
-	unsigned int nr = BITS_TO_LONGS(bits);
+	unsigned int lim = bits/BITS_PER_LONG;
 	unsigned long result = 0;
 
-	for (k = 0; k < nr; k++)
+	for (k = 0; k < lim; k++)
 		result |= (dst[k] = bitmap1[k] & bitmap2[k]);
+	if (bits % BITS_PER_LONG)
+		result |= (dst[k] = bitmap1[k] & bitmap2[k] &
+			   BITMAP_LAST_WORD_MASK(bits));
 	return result != 0;
 }
 EXPORT_SYMBOL(__bitmap_and);
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 18/18] lib: bitmap: Add missing mask in bitmap_andnot
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (16 preceding siblings ...)
  2014-07-03 22:43 ` [PATCH 17/18] lib: bitmap: Add missing mask in bitmap_and Rasmus Villemoes
@ 2014-07-03 22:43 ` Rasmus Villemoes
  2014-07-07 23:20 ` [PATCH 00/18] lib: bitmap: Various improvements Andrew Morton
  18 siblings, 0 replies; 21+ messages in thread
From: Rasmus Villemoes @ 2014-07-03 22:43 UTC (permalink / raw)
  To: linux-kernel; +Cc: Rasmus Villemoes

Apparently, bitmap_andnot is supposed to return whether the new bitmap
is empty. But it didn't take potential garbage bits in the last word
into account.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/bitmap.h | 2 +-
 lib/bitmap.c           | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 3399a9e..e1c8d08 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -217,7 +217,7 @@ static inline int bitmap_andnot(unsigned long *dst, const unsigned long *src1,
 			const unsigned long *src2, unsigned int nbits)
 {
 	if (small_const_nbits(nbits))
-		return (*dst = *src1 & ~(*src2)) != 0;
+		return (*dst = *src1 & ~(*src2) & BITMAP_LAST_WORD_MASK(nbits)) != 0;
 	return __bitmap_andnot(dst, src1, src2, nbits);
 }
 
diff --git a/lib/bitmap.c b/lib/bitmap.c
index ce2ec80..1e031f2 100644
--- a/lib/bitmap.c
+++ b/lib/bitmap.c
@@ -223,11 +223,14 @@ int __bitmap_andnot(unsigned long *dst, const unsigned long *bitmap1,
 				const unsigned long *bitmap2, unsigned int bits)
 {
 	unsigned int k;
-	unsigned int nr = BITS_TO_LONGS(bits);
+	unsigned int lim = bits/BITS_PER_LONG;
 	unsigned long result = 0;
 
-	for (k = 0; k < nr; k++)
+	for (k = 0; k < lim; k++)
 		result |= (dst[k] = bitmap1[k] & ~bitmap2[k]);
+	if (bits % BITS_PER_LONG)
+		result |= (dst[k] = bitmap1[k] & ~bitmap2[k] &
+			   BITMAP_LAST_WORD_MASK(bits));
 	return result != 0;
 }
 EXPORT_SYMBOL(__bitmap_andnot);
-- 
1.9.2


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/18] lib: bitmap: Various improvements
  2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
                   ` (17 preceding siblings ...)
  2014-07-03 22:43 ` [PATCH 18/18] lib: bitmap: Add missing mask in bitmap_andnot Rasmus Villemoes
@ 2014-07-07 23:20 ` Andrew Morton
  2014-07-07 23:45   ` Joe Perches
  18 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2014-07-07 23:20 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: linux-kernel

On Fri,  4 Jul 2014 00:42:46 +0200 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> Many functions in lib/bitmap.c start with an expression such as lim =
> bits/BITS_PER_LONG. Since bits has type (signed) int, and since gcc
> cannot know that it is in fact non-negative, it generates worse code
> than it could. These patches, mostly consisting of changing various
> parameters to unsigned, gives a slight overall code reduction:

Yes, we have a bad habit of using signed types for things where
negative values are absurd.

The patches look OK to me.

> A few issues I thought about, but didn't know what to do with:
> 
> * Many of the functions misbehave if nbits is compile-time 0; the
>   out-of-line functions generally handle 0 correctly. bitmap_fill() is
>   particularly bad, whether the 0 is known at compile time or not. It
>   would probably be nice to add detection of at least compile-time 0
>   and handle that appropriately.

The best option here would be a compile-time check.  Presumably
BUILD_BUG_ON().  That will catch the errant use and will add no runtime
overhead.

> * I didn't change __bitmap_shift_{left,right} to use unsigned because
>   I want to fully understand why the algorithm works before making
>   that change. However, AFAICT, they behave correctly for all
>   (positive) shift amounts. This is not the case for the
>   small_const_nbits versions. If for example nbits = n =
>   BITS_PER_LONG, the shift operators turn into no-ops (at least on
>   x86), so one get *dst = *src, whereas one would expect to get
>   *dst=0. That difference in behaviour is somewhat annoying.

yup.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 00/18] lib: bitmap: Various improvements
  2014-07-07 23:20 ` [PATCH 00/18] lib: bitmap: Various improvements Andrew Morton
@ 2014-07-07 23:45   ` Joe Perches
  0 siblings, 0 replies; 21+ messages in thread
From: Joe Perches @ 2014-07-07 23:45 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Rasmus Villemoes, linux-kernel

On Mon, 2014-07-07 at 16:20 -0700, Andrew Morton wrote:
> On Fri,  4 Jul 2014 00:42:46 +0200 Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> 
> > Many functions in lib/bitmap.c start with an expression such as lim =
> > bits/BITS_PER_LONG. Since bits has type (signed) int, and since gcc
> > cannot know that it is in fact non-negative, it generates worse code
> > than it could. These patches, mostly consisting of changing various
> > parameters to unsigned, gives a slight overall code reduction:
> 
> Yes, we have a bad habit of using signed types for things where
> negative values are absurd.

Linus at one time wrote:
http://thread.gmane.org/gmane.linux.kernel/1526174

---------------------------------------------------------
The fact is, the x86 bitop instructions act on a
signed index. Making the index be "unsigned long" would violate the
actual *behavior* of the function, so it would be singularly stupid.
---------------------------------------------------------

> The patches look OK to me.

Me too.

But this does have the unfortunate effect of allowing
bitmap tests on bitmaps that are unable to be set/cleared
when the bitmap is larger than INT_MAX size.

There could be some check to make sure size <= INT_MAX.

The docs could be clearer here too about the actual
maximum size of a bitmap.



^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2014-07-07 23:45 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-03 22:42 [PATCH 00/18] lib: bitmap: Various improvements Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 01/18] lib: bitmap: Make nbits parameter of bitmap_empty unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 02/18] lib: bitmap: Make nbits parameter of bitmap_full unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 03/18] lib: bitmap: Make nbits parameter of bitmap_equal unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 04/18] lib: bitmap: Make nbits parameter of bitmap_complement unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 05/18] lib: bitmap: Remove unnecessary mask from bitmap_complement Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 06/18] lib: bitmap: Make nbits parameter of bitmap_{and,or,xor,andnot} unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 07/18] lib: bitmap: Make nbits parameter of bitmap_intersects unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 08/18] lib: bitmap: Make nbits parameter of bitmap_subset unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 09/18] lib: bitmap: Make nbits parameter of bitmap_weight unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 10/18] lib: bitmap: Make the start index of bitmap_set unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 11/18] lib: bitmap: Make the start index of bitmap_clear unsigned Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 12/18] lib: bitmap: Simplify bitmap_parselist Rasmus Villemoes
2014-07-03 22:42 ` [PATCH 13/18] lib: bitmap: Fix typo in kerneldoc for bitmap_pos_to_ord Rasmus Villemoes
2014-07-03 22:43 ` [PATCH 14/18] lib: bitmap: Change parameter of bitmap_*_region to unsigned Rasmus Villemoes
2014-07-03 22:43 ` [PATCH 15/18] lib: bitmap: Micro-optimize bitmap_allocate_region Rasmus Villemoes
2014-07-03 22:43 ` [PATCH 16/18] lib: bitmap: Add missing mask in bitmap_shift_right Rasmus Villemoes
2014-07-03 22:43 ` [PATCH 17/18] lib: bitmap: Add missing mask in bitmap_and Rasmus Villemoes
2014-07-03 22:43 ` [PATCH 18/18] lib: bitmap: Add missing mask in bitmap_andnot Rasmus Villemoes
2014-07-07 23:20 ` [PATCH 00/18] lib: bitmap: Various improvements Andrew Morton
2014-07-07 23:45   ` Joe Perches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®