* [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®