| Message ID | 20210316015424.1999082-7-yury.norov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lib/find_bit: fast path for small bitmaps | expand |
On 16/03/2021 02.54, Yury Norov wrote: > find_bit would also benefit from small_const_nbits() optimizations. > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > --- > include/asm-generic/bitsperlong.h | 9 +++++++++ > include/linux/bitmap.h | 3 --- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/include/asm-generic/bitsperlong.h b/include/asm-generic/bitsperlong.h > index 3905c1c93dc2..96032f4f908f 100644 > --- a/include/asm-generic/bitsperlong.h > +++ b/include/asm-generic/bitsperlong.h > @@ -23,4 +23,13 @@ > #define BITS_PER_LONG_LONG 64 > #endif > > +#define SMALL_CONST(n) (__builtin_constant_p(n) && (unsigned long)(n) < BITS_PER_LONG) > + > +/* > + * The valid number of bits for a bitmap to be small enough, or in other words, > + * fit into a single machine word is 1 to BITS_PER_LONG inclusively. 0 is not a > + * valid number for size, and most probably a sing of error. > + */ > +#define small_const_nbits(n) SMALL_CONST((n) - 1) > + I still don't see the point of introducing SMALL_CONST and still don't like the much-too-generic-name - especially since AFAICT you don't actually use it anywhere outside the definition of small_const_nbits()? > #endif /* __ASM_GENERIC_BITS_PER_LONG */ > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index adf7bd9f0467..bc13a890ecc1 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -224,9 +224,6 @@ extern int bitmap_print_to_pagebuf(bool list, char *buf, > * so make such users (should any ever turn up) call the out-of-line > * versions. > */ You added another comment near its new definition, but the left-behind comment in bitmap.h is now somewhat confusing, no? I suggest expanding your new comment a bit so it's clear why we're interested in whether a bitmap is known at compile-time to consist of exactly one word: /* small_const_nbits(n) is true precisely when it is known at compile-time that BITMAP_SIZE(n) is 1, i.e. 1 <= n <= BITS_PER_LONG. This allows various bit/bitmap APIs to provide a fast inline implementation. Bitmaps of size 0 are very rare, and a compile-time-known-size 0 is most likely a sign of error. They will be handled correctly by the bit/bitmap APIs, but using the out-of-line functions, so that the inline implementations can unconditionally dereference the pointer(s). */ > -#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); > Rasmus
On Tue, Mar 16, 2021 at 09:56:49AM +0100, Rasmus Villemoes wrote: > On 16/03/2021 02.54, Yury Norov wrote: > > find_bit would also benefit from small_const_nbits() optimizations. > > > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > --- > > include/asm-generic/bitsperlong.h | 9 +++++++++ > > include/linux/bitmap.h | 3 --- > > 2 files changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/include/asm-generic/bitsperlong.h b/include/asm-generic/bitsperlong.h > > index 3905c1c93dc2..96032f4f908f 100644 > > --- a/include/asm-generic/bitsperlong.h > > +++ b/include/asm-generic/bitsperlong.h > > @@ -23,4 +23,13 @@ > > #define BITS_PER_LONG_LONG 64 > > #endif > > > > +#define SMALL_CONST(n) (__builtin_constant_p(n) && (unsigned long)(n) < BITS_PER_LONG) > > + > > +/* > > + * The valid number of bits for a bitmap to be small enough, or in other words, > > + * fit into a single machine word is 1 to BITS_PER_LONG inclusively. 0 is not a > > + * valid number for size, and most probably a sing of error. > > + */ > > +#define small_const_nbits(n) SMALL_CONST((n) - 1) > > + > > I still don't see the point of introducing SMALL_CONST and still don't > like the much-too-generic-name - especially since AFAICT you don't > actually use it anywhere outside the definition of small_const_nbits()? > > > #endif /* __ASM_GENERIC_BITS_PER_LONG */ > > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > > index adf7bd9f0467..bc13a890ecc1 100644 > > --- a/include/linux/bitmap.h > > +++ b/include/linux/bitmap.h > > @@ -224,9 +224,6 @@ extern int bitmap_print_to_pagebuf(bool list, char *buf, > > * so make such users (should any ever turn up) call the out-of-line > > * versions. > > */ > > You added another comment near its new definition, but the left-behind > comment in bitmap.h is now somewhat confusing, no? I suggest expanding > your new comment a bit so it's clear why we're interested in whether a > bitmap is known at compile-time to consist of exactly one word: > > /* > small_const_nbits(n) is true precisely when it is known at compile-time > that BITMAP_SIZE(n) is 1, i.e. 1 <= n <= BITS_PER_LONG. This allows > various bit/bitmap APIs to provide a fast inline implementation. Bitmaps > of size 0 are very rare, and a compile-time-known-size 0 is most likely > a sign of error. They will be handled correctly by the bit/bitmap APIs, > but using the out-of-line functions, so that the inline implementations > can unconditionally dereference the pointer(s). > */ Ok, make sense > > -#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); > > > > Rasmus
diff --git a/include/asm-generic/bitsperlong.h b/include/asm-generic/bitsperlong.h index 3905c1c93dc2..96032f4f908f 100644 --- a/include/asm-generic/bitsperlong.h +++ b/include/asm-generic/bitsperlong.h @@ -23,4 +23,13 @@ #define BITS_PER_LONG_LONG 64 #endif +#define SMALL_CONST(n) (__builtin_constant_p(n) && (unsigned long)(n) < BITS_PER_LONG) + +/* + * The valid number of bits for a bitmap to be small enough, or in other words, + * fit into a single machine word is 1 to BITS_PER_LONG inclusively. 0 is not a + * valid number for size, and most probably a sing of error. + */ +#define small_const_nbits(n) SMALL_CONST((n) - 1) + #endif /* __ASM_GENERIC_BITS_PER_LONG */ diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index adf7bd9f0467..bc13a890ecc1 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -224,9 +224,6 @@ extern int bitmap_print_to_pagebuf(bool list, char *buf, * 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);
find_bit would also benefit from small_const_nbits() optimizations. Signed-off-by: Yury Norov <yury.norov@gmail.com> --- include/asm-generic/bitsperlong.h | 9 +++++++++ include/linux/bitmap.h | 3 --- 2 files changed, 9 insertions(+), 3 deletions(-)