[PATCH 0/6] lib/find_bit: fast path for small bitmaps

[PATCH 0/6] lib/find_bit: fast path for small bitmaps

[Yury Norov]

Bitmap operations are much simpler and faster in case of small bitmaps,
whicn fit into a single word. In linux/bitmap.h we have a machinery that
allows compiler to replace actual function call with a few instructions
if bitmaps passed into the function is small and its size is known at
compile time.

find_next_*_bit() lacks this functionality; despite users will
benefit from it a lot. One important example is cpumask subsystem if
NR_CPUS <= BITS_PER_LONG. In the very best case, the compiler may replace
a find_*_bit() call for such a bitmap with a single ffs or ffz instruction.

Yury Norov (6):
  arch: rearrahge headers inclusion order in asm/bitops for m68k and sh
  bitmap: move some macros from linux/bitmap.h to linux/bitops.h
  tools: sync bitops macro definitions with the kernel
  lib: inline _find_next_bit() wrappers
  lib: add fast path for find_next_*_bit()
  lib: add fast path for find_first_*_bit() and find_last_bit()

 arch/m68k/include/asm/bitops.h    |   4 +-
 arch/sh/include/asm/bitops.h      |   3 +-
 include/asm-generic/bitops/find.h | 123 +++++++++++++++++++++++++++---
 include/asm-generic/bitops/le.h   |  45 ++++++++++-
 include/linux/bitmap.h            |  11 ---
 include/linux/bitops.h            |  23 +++---
 lib/find_bit.c                    |  68 ++---------------
 tools/include/linux/bitmap.h      |  11 ---
 tools/include/linux/bitops.h      |  11 +++
 9 files changed, 188 insertions(+), 111 deletions(-)

-- 
2.25.1

[PATCH 1/6] arch: rearrahge headers inclusion order in asm/bitops for m68k and sh

[Yury Norov]

m68k and sh include bitmap/find.h prior to ffs/fls headers. New
fast-path implementation in find.h requires ffs/fls. Reordering
the order of headers inclusion helps to prevent compile-time
implicit-function-declaration error.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 arch/m68k/include/asm/bitops.h | 4 ++--
 arch/sh/include/asm/bitops.h   | 3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/m68k/include/asm/bitops.h b/arch/m68k/include/asm/bitops.h
index 10133a968c8e..093590c9e70f 100644
--- a/arch/m68k/include/asm/bitops.h
+++ b/arch/m68k/include/asm/bitops.h
@@ -440,8 +440,6 @@ static inline unsigned long ffz(unsigned long word)
 
 #endif
 
-#include <asm-generic/bitops/find.h>
-
 #ifdef __KERNEL__
 
 #if defined(CONFIG_CPU_HAS_NO_BITFIELDS)
@@ -531,4 +529,6 @@ static inline int __fls(int x)
 #include <asm-generic/bitops/hweight.h>
 #endif /* __KERNEL__ */
 
+#include <asm-generic/bitops/find.h>
+
 #endif /* _M68K_BITOPS_H */
diff --git a/arch/sh/include/asm/bitops.h b/arch/sh/include/asm/bitops.h
index 450b5854d7c6..792bbe1237dc 100644
--- a/arch/sh/include/asm/bitops.h
+++ b/arch/sh/include/asm/bitops.h
@@ -58,7 +58,6 @@ static inline unsigned long __ffs(unsigned long word)
 	return result;
 }
 
-#include <asm-generic/bitops/find.h>
 #include <asm-generic/bitops/ffs.h>
 #include <asm-generic/bitops/hweight.h>
 #include <asm-generic/bitops/lock.h>
@@ -69,4 +68,6 @@ static inline unsigned long __ffs(unsigned long word)
 #include <asm-generic/bitops/__fls.h>
 #include <asm-generic/bitops/fls64.h>
 
+#include <asm-generic/bitops/find.h>
+
 #endif /* __ASM_SH_BITOPS_H */
-- 
2.25.1

[PATCH 2/6] bitmap: move some macros from linux/bitmap.h to linux/bitops.h

[Yury Norov]

In the following patches of the series they are used by
find_bit subsystem.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 include/linux/bitmap.h | 11 -----------
 include/linux/bitops.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h
index 70a932470b2d..5bacbc8785eb 100644
--- a/include/linux/bitmap.h
+++ b/include/linux/bitmap.h
@@ -219,17 +219,6 @@ extern unsigned int bitmap_ord_to_pos(const unsigned long *bitmap, unsigned int
 extern int bitmap_print_to_pagebuf(bool list, char *buf,
 				   const unsigned long *maskp, int nmaskbits);
 
-#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
-#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
-
-/*
- * The static inlines below do not handle constant nbits==0 correctly,
- * so make such users (should any ever turn up) call the out-of-line
- * versions.
- */
-#define small_const_nbits(nbits) \
-	(__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG && (nbits) > 0)
-
 static inline void bitmap_zero(unsigned long *dst, unsigned int nbits)
 {
 	unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long);
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a5a48303b0f1..a0e138bbb8ce 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -7,6 +7,17 @@
 
 #include <uapi/linux/kernel.h>
 
+#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
+#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
+
+/*
+ * The static inlines below do not handle constant nbits==0 correctly,
+ * so make such users (should any ever turn up) call the out-of-line
+ * versions.
+ */
+#define small_const_nbits(nbits) \
+	(__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG && (nbits) > 0)
+
 /* Set bits in the first 'n' bytes when loaded from memory */
 #ifdef __LITTLE_ENDIAN
 #  define aligned_byte_mask(n) ((1UL << 8*(n))-1)
-- 
2.25.1

[PATCH 3/6] tools: sync bitops macro definitions with the kernel

[Yury Norov]

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 tools/include/linux/bitmap.h | 11 -----------
 tools/include/linux/bitops.h | 11 +++++++++++
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h
index 477a1cae513f..266a9291dd1f 100644
--- a/tools/include/linux/bitmap.h
+++ b/tools/include/linux/bitmap.h
@@ -19,17 +19,6 @@ int __bitmap_equal(const unsigned long *bitmap1,
 		   const unsigned long *bitmap2, unsigned int bits);
 void bitmap_clear(unsigned long *map, unsigned int start, int len);
 
-#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
-
-#define BITMAP_LAST_WORD_MASK(nbits)					\
-(									\
-	((nbits) % BITS_PER_LONG) ?					\
-		(1UL<<((nbits) % BITS_PER_LONG))-1 : ~0UL		\
-)
-
-#define small_const_nbits(nbits) \
-	(__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG)
-
 static inline void bitmap_zero(unsigned long *dst, int nbits)
 {
 	if (small_const_nbits(nbits))
diff --git a/tools/include/linux/bitops.h b/tools/include/linux/bitops.h
index 5fca38fe1ba8..1c70919cb216 100644
--- a/tools/include/linux/bitops.h
+++ b/tools/include/linux/bitops.h
@@ -14,6 +14,17 @@
 #include <linux/bits.h>
 #include <linux/compiler.h>
 
+#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
+#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
+
+/*
+ * The static inlines below do not handle constant nbits==0 correctly,
+ * so make such users (should any ever turn up) call the out-of-line
+ * versions.
+ */
+#define small_const_nbits(nbits) \
+	(__builtin_constant_p(nbits) && (nbits) <= BITS_PER_LONG && (nbits) > 0)
+
 #define BITS_PER_TYPE(type)	(sizeof(type) * BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
 #define BITS_TO_U64(nr)		DIV_ROUND_UP(nr, BITS_PER_TYPE(u64))
-- 
2.25.1

[PATCH 4/6] lib: inline _find_next_bit() wrappers

[Yury Norov]

lib/find_bit.c declares five single-line wrappers for _find_next_bit().
We may turn those wrappers to inline functions. It eliminates
unneeded function calls and opens room for compile-time optimizations.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 include/asm-generic/bitops/find.h | 28 ++++++++++++----
 include/asm-generic/bitops/le.h   | 17 +++++++---
 lib/find_bit.c                    | 56 ++-----------------------------
 3 files changed, 37 insertions(+), 64 deletions(-)

diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
index 9fdf21302fdf..7ad70dab8e93 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -2,6 +2,10 @@
 #ifndef _ASM_GENERIC_BITOPS_FIND_H_
 #define _ASM_GENERIC_BITOPS_FIND_H_
 
+extern unsigned long _find_next_bit(const unsigned long *addr1,
+		const unsigned long *addr2, unsigned long nbits,
+		unsigned long start, unsigned long invert, unsigned long le);
+
 #ifndef find_next_bit
 /**
  * find_next_bit - find the next set bit in a memory region
@@ -12,8 +16,12 @@
  * Returns the bit number for the next set bit
  * If no bits are set, returns @size.
  */
-extern unsigned long find_next_bit(const unsigned long *addr, unsigned long
-		size, unsigned long offset);
+static inline
+unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
+			    unsigned long offset)
+{
+	return _find_next_bit(addr, NULL, size, offset, 0UL, 0);
+}
 #endif
 
 #ifndef find_next_and_bit
@@ -27,9 +35,13 @@ extern unsigned long find_next_bit(const unsigned long *addr, unsigned long
  * Returns the bit number for the next set bit
  * If no bits are set, returns @size.
  */
-extern unsigned long find_next_and_bit(const unsigned long *addr1,
+static inline
+unsigned long find_next_and_bit(const unsigned long *addr1,
 		const unsigned long *addr2, unsigned long size,
-		unsigned long offset);
+		unsigned long offset)
+{
+	return _find_next_bit(addr1, addr2, size, offset, 0UL, 0);
+}
 #endif
 
 #ifndef find_next_zero_bit
@@ -42,8 +54,12 @@ extern unsigned long find_next_and_bit(const unsigned long *addr1,
  * Returns the bit number of the next zero bit
  * If no bits are zero, returns @size.
  */
-extern unsigned long find_next_zero_bit(const unsigned long *addr, unsigned
-		long size, unsigned long offset);
+static inline
+unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
+				 unsigned long offset)
+{
+	return _find_next_bit(addr, NULL, size, offset, ~0UL, 0);
+}
 #endif
 
 #ifdef CONFIG_GENERIC_FIND_FIRST_BIT
diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index 188d3eba3ace..4cf44ea16ec0 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -4,6 +4,7 @@
 
 #include <asm/types.h>
 #include <asm/byteorder.h>
+#include <asm-generic/bitops/find.h>
 
 #if defined(__LITTLE_ENDIAN)
 
@@ -32,13 +33,21 @@ static inline unsigned long find_first_zero_bit_le(const void *addr,
 #define BITOP_LE_SWIZZLE	((BITS_PER_LONG-1) & ~0x7)
 
 #ifndef find_next_zero_bit_le
-extern unsigned long find_next_zero_bit_le(const void *addr,
-		unsigned long size, unsigned long offset);
+static inline
+unsigned long find_next_zero_bit_le(const void *addr, unsigned
+		long size, unsigned long offset)
+{
+	return _find_next_bit(addr, NULL, size, offset, ~0UL, 1);
+}
 #endif
 
 #ifndef find_next_bit_le
-extern unsigned long find_next_bit_le(const void *addr,
-		unsigned long size, unsigned long offset);
+static inline
+unsigned long find_next_bit_le(const void *addr, unsigned
+		long size, unsigned long offset)
+{
+	return _find_next_bit(addr, NULL, size, offset, 0UL, 1);
+}
 #endif
 
 #ifndef find_first_zero_bit_le
diff --git a/lib/find_bit.c b/lib/find_bit.c
index f67f86fd2f62..b03a101367f8 100644
--- a/lib/find_bit.c
+++ b/lib/find_bit.c
@@ -29,7 +29,7 @@
  *    searching it for one bits.
  *  - The optional "addr2", which is anded with "addr1" if present.
  */
-static unsigned long _find_next_bit(const unsigned long *addr1,
+unsigned long _find_next_bit(const unsigned long *addr1,
 		const unsigned long *addr2, unsigned long nbits,
 		unsigned long start, unsigned long invert, unsigned long le)
 {
@@ -68,37 +68,7 @@ static unsigned long _find_next_bit(const unsigned long *addr1,
 
 	return min(start + __ffs(tmp), nbits);
 }
-#endif
-
-#ifndef find_next_bit
-/*
- * Find the next set bit in a memory region.
- */
-unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
-			    unsigned long offset)
-{
-	return _find_next_bit(addr, NULL, size, offset, 0UL, 0);
-}
-EXPORT_SYMBOL(find_next_bit);
-#endif
-
-#ifndef find_next_zero_bit
-unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
-				 unsigned long offset)
-{
-	return _find_next_bit(addr, NULL, size, offset, ~0UL, 0);
-}
-EXPORT_SYMBOL(find_next_zero_bit);
-#endif
-
-#if !defined(find_next_and_bit)
-unsigned long find_next_and_bit(const unsigned long *addr1,
-		const unsigned long *addr2, unsigned long size,
-		unsigned long offset)
-{
-	return _find_next_bit(addr1, addr2, size, offset, 0UL, 0);
-}
-EXPORT_SYMBOL(find_next_and_bit);
+EXPORT_SYMBOL(_find_next_bit);
 #endif
 
 #ifndef find_first_bit
@@ -157,28 +127,6 @@ unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
 EXPORT_SYMBOL(find_last_bit);
 #endif
 
-#ifdef __BIG_ENDIAN
-
-#ifndef find_next_zero_bit_le
-unsigned long find_next_zero_bit_le(const void *addr, unsigned
-		long size, unsigned long offset)
-{
-	return _find_next_bit(addr, NULL, size, offset, ~0UL, 1);
-}
-EXPORT_SYMBOL(find_next_zero_bit_le);
-#endif
-
-#ifndef find_next_bit_le
-unsigned long find_next_bit_le(const void *addr, unsigned
-		long size, unsigned long offset)
-{
-	return _find_next_bit(addr, NULL, size, offset, 0UL, 1);
-}
-EXPORT_SYMBOL(find_next_bit_le);
-#endif
-
-#endif /* __BIG_ENDIAN */
-
 unsigned long find_next_clump8(unsigned long *clump, const unsigned long *addr,
 			       unsigned long size, unsigned long offset)
 {
-- 
2.25.1

[PATCH 5/6] lib: add fast path for find_next_*_bit()

[Yury Norov]

Similarly to bitmap functions, find_next_*_bit() users will benefit
if we'll handle a case of bitmaps that fit into a single word. In the
very best case, the compiler may replace a function call with a
single ffs or ffz instruction.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 include/asm-generic/bitops/find.h | 39 +++++++++++++++++++++++++++++++
 include/asm-generic/bitops/le.h   | 28 ++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
index 7ad70dab8e93..d45096069011 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -20,6 +20,18 @@ static inline
 unsigned long find_next_bit(const unsigned long *addr, unsigned long size,
 			    unsigned long offset)
 {
+	if (small_const_nbits(size)) {
+		unsigned long val;
+
+		if (unlikely(offset >= size))
+			return size;
+
+		val = *addr & BITMAP_FIRST_WORD_MASK(offset)
+				& BITMAP_LAST_WORD_MASK(size);
+
+		return val ? __ffs(val) : size;
+	}
+
 	return _find_next_bit(addr, NULL, size, offset, 0UL, 0);
 }
 #endif
@@ -40,6 +52,18 @@ unsigned long find_next_and_bit(const unsigned long *addr1,
 		const unsigned long *addr2, unsigned long size,
 		unsigned long offset)
 {
+	if (small_const_nbits(size)) {
+		unsigned long val;
+
+		if (unlikely(offset >= size))
+			return size;
+
+		val = *addr1 & *addr2 & BITMAP_FIRST_WORD_MASK(offset)
+				      & BITMAP_LAST_WORD_MASK(size);
+
+		return val ? __ffs(val) : size;
+	}
+
 	return _find_next_bit(addr1, addr2, size, offset, 0UL, 0);
 }
 #endif
@@ -58,6 +82,21 @@ static inline
 unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
 				 unsigned long offset)
 {
+	if (small_const_nbits(size)) {
+		unsigned long val, idx;
+
+		if (unlikely(offset >= size))
+			return size;
+
+		val = *addr | ~BITMAP_FIRST_WORD_MASK(offset);
+		if (val == ~0UL)
+			return size;
+
+		idx = ffz(val);
+
+		return idx < size ? idx : size;
+	}
+
 	return _find_next_bit(addr, NULL, size, offset, ~0UL, 0);
 }
 #endif
diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index 4cf44ea16ec0..f4a76d3d145f 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -5,6 +5,7 @@
 #include <asm/types.h>
 #include <asm/byteorder.h>
 #include <asm-generic/bitops/find.h>
+#include <linux/swab.h>
 
 #if defined(__LITTLE_ENDIAN)
 
@@ -37,6 +38,21 @@ static inline
 unsigned long find_next_zero_bit_le(const void *addr, unsigned
 		long size, unsigned long offset)
 {
+	if (small_const_nbits(size)) {
+		unsigned long val = *(const unsigned long *)addr, idx;
+
+		if (unlikely(offset >= size))
+			return size;
+
+		val = swab(val) | ~BITMAP_FIRST_WORD_MASK(offset);
+		if (val == ~0UL)
+			return size;
+
+		idx = ffz(val);
+
+		return idx < size ? idx : size;
+	}
+
 	return _find_next_bit(addr, NULL, size, offset, ~0UL, 1);
 }
 #endif
@@ -46,6 +62,18 @@ static inline
 unsigned long find_next_bit_le(const void *addr, unsigned
 		long size, unsigned long offset)
 {
+	if (small_const_nbits(size)) {
+		unsigned long val = *(const unsigned long *)addr;
+
+		if (unlikely(offset >= size))
+			return size;
+
+		val = swab(val) & BITMAP_FIRST_WORD_MASK(offset)
+				& BITMAP_LAST_WORD_MASK(size);
+
+		return val ? __ffs(val) : size;
+	}
+
 	return _find_next_bit(addr, NULL, size, offset, 0UL, 1);
 }
 #endif
-- 
2.25.1

[PATCH 6/6] lib: add fast path for find_first_*_bit() and find_last_bit()

[Yury Norov]

Similarly to bitmap functions, users will benefit if we'll handle
a case of small-size bitmaps that fit into a single word.

While here, move the find_last_bit() declaration to bitops/find.h
where other find_*_bit() functions sit.

Signed-off-by: Yury Norov <yury.norov@gmail.com>
---
 include/asm-generic/bitops/find.h | 56 ++++++++++++++++++++++++++++---
 include/linux/bitops.h            | 12 -------
 lib/find_bit.c                    | 12 +++----
 3 files changed, 58 insertions(+), 22 deletions(-)

diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h
index d45096069011..8708e333fc69 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -5,6 +5,9 @@
 extern unsigned long _find_next_bit(const unsigned long *addr1,
 		const unsigned long *addr2, unsigned long nbits,
 		unsigned long start, unsigned long invert, unsigned long le);
+extern unsigned long _find_first_bit(const unsigned long *addr, unsigned long size);
+extern unsigned long _find_first_zero_bit(const unsigned long *addr, unsigned long size);
+extern unsigned long _find_last_bit(const unsigned long *addr, unsigned long size);
 
 #ifndef find_next_bit
 /**
@@ -111,8 +114,21 @@ unsigned long find_next_zero_bit(const unsigned long *addr, unsigned long size,
  * Returns the bit number of the first set bit.
  * If no bits are set, returns @size.
  */
-extern unsigned long find_first_bit(const unsigned long *addr,
-				    unsigned long size);
+static inline
+unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
+{
+	if (small_const_nbits(size)) {
+		unsigned long idx;
+
+		if (!*addr)
+			return size;
+
+		idx = __ffs(*addr);
+		return idx < size ? idx : size;
+	}
+
+	return _find_first_bit(addr, size);
+}
 
 /**
  * find_first_zero_bit - find the first cleared bit in a memory region
@@ -122,8 +138,20 @@ extern unsigned long find_first_bit(const unsigned long *addr,
  * Returns the bit number of the first cleared bit.
  * If no bits are zero, returns @size.
  */
-extern unsigned long find_first_zero_bit(const unsigned long *addr,
-					 unsigned long size);
+static inline
+unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
+{
+	if (small_const_nbits(size)) {
+		unsigned long idx;
+		if (*addr == ~0UL)
+			return size;
+
+		idx = ffz(*addr);
+		return idx < size ? idx : size;
+	}
+
+	return _find_first_zero_bit(addr, size);
+}
 #else /* CONFIG_GENERIC_FIND_FIRST_BIT */
 
 #ifndef find_first_bit
@@ -135,6 +163,26 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,
 
 #endif /* CONFIG_GENERIC_FIND_FIRST_BIT */
 
+#ifndef find_last_bit
+/**
+ * find_last_bit - find the last set bit in a memory region
+ * @addr: The address to start the search at
+ * @size: The number of bits to search
+ *
+ * Returns the bit number of the last set bit, or size.
+ */
+static inline
+unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
+{
+	if (small_const_nbits(size)) {
+		unsigned long val = *addr & BITMAP_LAST_WORD_MASK(size);
+		return val ? __fls(val) : size;
+	}
+
+	return _find_last_bit(addr, size);
+}
+#endif
+
 /**
  * find_next_clump8 - find next 8-bit clump with set bits in a memory region
  * @clump: location to store copy of found clump
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a0e138bbb8ce..1d41193be4ab 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -297,17 +297,5 @@ static __always_inline void __assign_bit(long nr, volatile unsigned long *addr,
 })
 #endif
 
-#ifndef find_last_bit
-/**
- * find_last_bit - find the last set bit in a memory region
- * @addr: The address to start the search at
- * @size: The number of bits to search
- *
- * Returns the bit number of the last set bit, or size.
- */
-extern unsigned long find_last_bit(const unsigned long *addr,
-				   unsigned long size);
-#endif
-
 #endif /* __KERNEL__ */
 #endif
diff --git a/lib/find_bit.c b/lib/find_bit.c
index b03a101367f8..0f8e2e369b1d 100644
--- a/lib/find_bit.c
+++ b/lib/find_bit.c
@@ -75,7 +75,7 @@ EXPORT_SYMBOL(_find_next_bit);
 /*
  * Find the first set bit in a memory region.
  */
-unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
+unsigned long _find_first_bit(const unsigned long *addr, unsigned long size)
 {
 	unsigned long idx;
 
@@ -86,14 +86,14 @@ unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
 
 	return size;
 }
-EXPORT_SYMBOL(find_first_bit);
+EXPORT_SYMBOL(_find_first_bit);
 #endif
 
 #ifndef find_first_zero_bit
 /*
  * Find the first cleared bit in a memory region.
  */
-unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
+unsigned long _find_first_zero_bit(const unsigned long *addr, unsigned long size)
 {
 	unsigned long idx;
 
@@ -104,11 +104,11 @@ unsigned long find_first_zero_bit(const unsigned long *addr, unsigned long size)
 
 	return size;
 }
-EXPORT_SYMBOL(find_first_zero_bit);
+EXPORT_SYMBOL(_find_first_zero_bit);
 #endif
 
 #ifndef find_last_bit
-unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
+unsigned long _find_last_bit(const unsigned long *addr, unsigned long size)
 {
 	if (size) {
 		unsigned long val = BITMAP_LAST_WORD_MASK(size);
@@ -124,7 +124,7 @@ unsigned long find_last_bit(const unsigned long *addr, unsigned long size)
 	}
 	return size;
 }
-EXPORT_SYMBOL(find_last_bit);
+EXPORT_SYMBOL(_find_last_bit);
 #endif
 
 unsigned long find_next_clump8(unsigned long *clump, const unsigned long *addr,
-- 
2.25.1

Re: [PATCH 1/6] arch: rearrahge headers inclusion order in asm/bitops for m68k and sh

[Geert Uytterhoeven]

On Thu, Jan 21, 2021 at 1:06 AM Yury Norov <yury.norov@gmail.com> wrote:
> m68k and sh include bitmap/find.h prior to ffs/fls headers. New
> fast-path implementation in find.h requires ffs/fls. Reordering
> the order of headers inclusion helps to prevent compile-time
> implicit-function-declaration error.
>
> Signed-off-by: Yury Norov <yury.norov@gmail.com>
> ---
>  arch/m68k/include/asm/bitops.h | 4 ++--

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/6] arch: rearrahge headers inclusion order in asm/bitops for m68k and sh

[John Paul Adrian Glaubitz]

Hi Yury!

On 1/21/21 1:06 AM, Yury Norov wrote:
> m68k and sh include bitmap/find.h prior to ffs/fls headers. New
> fast-path implementation in find.h requires ffs/fls. Reordering
> the order of headers inclusion helps to prevent compile-time
> implicit-function-declaration error.

Can you fix the commit message?

"arch: rearrahge headers inclusion order in asm/bitops for m68k and sh"
       ^^^^^^^^^
       rearrange

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH 2/6] bitmap: move some macros from linux/bitmap.h to linux/bitops.h

[Andy Shevchenko]

On Wed, Jan 20, 2021 at 04:06:26PM -0800, Yury Norov wrote:
> In the following patches of the series they are used by
> find_bit subsystem.

s/subsystem/API/

...

> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -7,6 +7,17 @@
>  
>  #include <uapi/linux/kernel.h>
>  
> +#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
> +#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))

Hmm... Naming here is not in the bitops namespace.
I would expect BITS rather than BITMAP for these two.

So, we have at least the following options:
 - split to a separate header, like bitmap_macros.h
 - s/BITMAP/BITS/ and either define BITMAP_* as respective BITS_* or rename it
   everywhere in bitmap.*
 - your variant
 - ...???...


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/6] tools: sync bitops macro definitions with the kernel

[Andy Shevchenko]

On Wed, Jan 20, 2021 at 04:06:27PM -0800, Yury Norov wrote:

Commit message?

> Signed-off-by: Yury Norov <yury.norov@gmail.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 4/6] lib: inline _find_next_bit() wrappers

[Andy Shevchenko]

On Wed, Jan 20, 2021 at 04:06:28PM -0800, Yury Norov wrote:
> lib/find_bit.c declares five single-line wrappers for _find_next_bit().
> We may turn those wrappers to inline functions. It eliminates
> unneeded function calls and opens room for compile-time optimizations.

...

> --- a/include/asm-generic/bitops/le.h
> +++ b/include/asm-generic/bitops/le.h
> @@ -4,6 +4,7 @@
>  
>  #include <asm/types.h>
>  #include <asm/byteorder.h>
> +#include <asm-generic/bitops/find.h>

I'm wondering if generic header inclusion should go before arch-dependent ones.

...

> -#ifndef find_next_bit

> -#ifndef find_next_zero_bit

> -#if !defined(find_next_and_bit)

> -#ifndef find_next_zero_bit_le

> -#ifndef find_next_bit_le

Shouldn't you leave these in new wrappers as well?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 5/6] lib: add fast path for find_next_*_bit()

[Andy Shevchenko]

On Wed, Jan 20, 2021 at 04:06:29PM -0800, Yury Norov wrote:
> Similarly to bitmap functions, find_next_*_bit() users will benefit
> if we'll handle a case of bitmaps that fit into a single word. In the
> very best case, the compiler may replace a function call with a
> single ffs or ffz instruction.

> +	if (small_const_nbits(size)) {
> +		unsigned long val;
> +
> +		if (unlikely(offset >= size))
> +			return size;

> +		val = *addr & BITMAP_FIRST_WORD_MASK(offset)
> +				& BITMAP_LAST_WORD_MASK(size);

Seems like a new helper can be introduced (BITS or BITMAP namespace depending
on the decision):

#define	_OFFSET_SIZE_MASK(o,s)					\
	(BITMAP_FIRST_WORD_MASK(o) & BITMAP_LAST_WORD_MASK(s))

		val = *addr & BITMAP_OFFSET_SIZE_MASK(offset, size);

And so on below.

> +		return val ? __ffs(val) : size;
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 6/6] lib: add fast path for find_first_*_bit() and find_last_bit()

[Andy Shevchenko]

On Wed, Jan 20, 2021 at 04:06:30PM -0800, Yury Norov wrote:
> Similarly to bitmap functions, users will benefit if we'll handle
> a case of small-size bitmaps that fit into a single word.
> 
> While here, move the find_last_bit() declaration to bitops/find.h
> where other find_*_bit() functions sit.

...

> +static inline
> +unsigned long find_first_bit(const unsigned long *addr, unsigned long size)
> +{
> +	if (small_const_nbits(size)) {
> +		unsigned long idx;
> +
> +		if (!*addr)
> +			return size;
> +
> +		idx = __ffs(*addr);

> +		return idx < size ? idx : size;

But can't we mask it first, then check for 0 (no bit set) otherwise return the
result of __ffs directly?

Same comment for other similar places.

> +	}
> +
> +	return _find_first_bit(addr, size);
> +}


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 2/6] bitmap: move some macros from linux/bitmap.h to linux/bitops.h

[Yury Norov]

On Thu, Jan 21, 2021 at 2:18 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jan 20, 2021 at 04:06:26PM -0800, Yury Norov wrote:
> > In the following patches of the series they are used by
> > find_bit subsystem.
>
> s/subsystem/API/
>
> ...
>
> > --- a/include/linux/bitops.h
> > +++ b/include/linux/bitops.h
> > @@ -7,6 +7,17 @@
> >
> >  #include <uapi/linux/kernel.h>
> >
> > +#define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1)))
> > +#define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1)))
>
> Hmm... Naming here is not in the bitops namespace.
> I would expect BITS rather than BITMAP for these two.
>
> So, we have at least the following options:
>  - split to a separate header, like bitmap_macros.h
>  - s/BITMAP/BITS/ and either define BITMAP_* as respective BITS_* or rename it
>    everywhere in bitmap.*
>  - your variant
>  - ...???...

We have GENMASK in linux/bits.h. I think we should use it here and
drop local ones.
It will also cover the case of  OFFSET macro that you suggested in
your comment to
patch #5.

Re: [PATCH 4/6] lib: inline _find_next_bit() wrappers

[Yury Norov]

On Thu, Jan 21, 2021 at 2:27 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Jan 20, 2021 at 04:06:28PM -0800, Yury Norov wrote:
> > lib/find_bit.c declares five single-line wrappers for _find_next_bit().
> > We may turn those wrappers to inline functions. It eliminates
> > unneeded function calls and opens room for compile-time optimizations.
>
> ...
>
> > --- a/include/asm-generic/bitops/le.h
> > +++ b/include/asm-generic/bitops/le.h
> > @@ -4,6 +4,7 @@
> >
> >  #include <asm/types.h>
> >  #include <asm/byteorder.h>
> > +#include <asm-generic/bitops/find.h>
>
> I'm wondering if generic header inclusion should go before arch-dependent ones.
>
> ...
>
> > -#ifndef find_next_bit
>
> > -#ifndef find_next_zero_bit
>
> > -#if !defined(find_next_and_bit)
>
> > -#ifndef find_next_zero_bit_le
>
> > -#ifndef find_next_bit_le
>
> Shouldn't you leave these in new wrappers as well?
>
> --
> With Best Regards,
> Andy Shevchenko

Could you please elaborate? Wrappers in find.h are protected, functions
in lib/find_bit.c too. Maybe I misunderstood you?..