| Message ID | 20231009151026.66145-9-aleksander.lobakin@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ip_tunnel: convert __be16 tunnel flags to bitmaps | expand |
On Mon, Oct 09, 2023 at 05:10:20PM +0200, Alexander Lobakin wrote: > The number of times yet another open coded > `BITS_TO_LONGS(nbits) * sizeof(long)` can be spotted is huge. > Some generic helper is long overdue. OK, I see your point. Indeed, opencoding this again and again may be annoying. Acked-by: Yury Norov <yury.norov@gmail.com> > Add one, bitmap_size(), but with one detail. > BITS_TO_LONGS() uses DIV_ROUND_UP(). The latter works well when both > divident and divisor are compile-time constants or when the divisor > is not a pow-of-2. When it is however, the compilers sometimes tend > to generate suboptimal code (GCC 13): > > 48 83 c0 3f add $0x3f,%rax > 48 c1 e8 06 shr $0x6,%rax > 48 8d 14 c5 00 00 00 00 lea 0x0(,%rax,8),%rdx > > %BITS_PER_LONG is always a pow-2 (either 32 or 64), but GCC still does > full division of `nbits + 63` by it and then multiplication by 8. > Instead of BITS_TO_LONGS(), use ALIGN() and then divide by 8. GCC: > > 8d 50 3f lea 0x3f(%rax),%edx > c1 ea 03 shr $0x3,%edx > 81 e2 f8 ff ff 1f and $0x1ffffff8,%edx > > Now it divides `nbits + 63` by 8 and then masks bits[2:0]. > bloat-o-meter: > > add/remove: 0/0 grow/shrink: 20/133 up/down: 156/-773 (-617) > > Clang does it better and generates the same code before/after starting > from -O1, except that with the ALIGN() approach it uses %edx and thus > still saves some bytes: > > add/remove: 0/0 grow/shrink: 9/133 up/down: 18/-538 (-520) > > Note that we can't expand DIV_ROUND_UP() by adding a check and using > this approach there, as it's used in array declarations where > expressions are not allowed. > Add this helper to tools/ as well. > > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> > --- > drivers/md/dm-clone-metadata.c | 5 ----- > include/linux/bitmap.h | 8 +++++--- > include/linux/cpumask.h | 2 +- > lib/math/prime_numbers.c | 2 -- > tools/include/linux/bitmap.h | 8 +++++--- > 5 files changed, 11 insertions(+), 14 deletions(-) > > diff --git a/drivers/md/dm-clone-metadata.c b/drivers/md/dm-clone-metadata.c > index c43d55672bce..47c1fa7aad8b 100644 > --- a/drivers/md/dm-clone-metadata.c > +++ b/drivers/md/dm-clone-metadata.c > @@ -465,11 +465,6 @@ static void __destroy_persistent_data_structures(struct dm_clone_metadata *cmd) > > /*---------------------------------------------------------------------------*/ > > -static size_t bitmap_size(unsigned long nr_bits) > -{ > - return BITS_TO_LONGS(nr_bits) * sizeof(long); > -} > - > static int __dirty_map_init(struct dirty_map *dmap, unsigned long nr_words, > unsigned long nr_regions) > { > diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h > index 03644237e1ef..63e422f8ba3d 100644 > --- a/include/linux/bitmap.h > +++ b/include/linux/bitmap.h > @@ -237,9 +237,11 @@ extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp, > #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) > #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) > > +#define bitmap_size(nbits) (ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE) > + > static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) > { > - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > + unsigned int len = bitmap_size(nbits); > > if (small_const_nbits(nbits)) > *dst = 0; > @@ -249,7 +251,7 @@ static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) > > static inline void bitmap_fill(unsigned long *dst, unsigned int nbits) > { > - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > + unsigned int len = bitmap_size(nbits); > > if (small_const_nbits(nbits)) > *dst = ~0UL; > @@ -260,7 +262,7 @@ static inline void bitmap_fill(unsigned long *dst, unsigned int nbits) > static inline void bitmap_copy(unsigned long *dst, const unsigned long *src, > unsigned int nbits) > { > - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > + unsigned int len = bitmap_size(nbits); > > if (small_const_nbits(nbits)) > *dst = *src; > diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h > index f10fb87d49db..dbdbf1451cad 100644 > --- a/include/linux/cpumask.h > +++ b/include/linux/cpumask.h > @@ -821,7 +821,7 @@ static inline int cpulist_parse(const char *buf, struct cpumask *dstp) > */ > static inline unsigned int cpumask_size(void) > { > - return BITS_TO_LONGS(large_cpumask_bits) * sizeof(long); > + return bitmap_size(large_cpumask_bits); > } > > /* > diff --git a/lib/math/prime_numbers.c b/lib/math/prime_numbers.c > index d42cebf7407f..d3b64b10da1c 100644 > --- a/lib/math/prime_numbers.c > +++ b/lib/math/prime_numbers.c > @@ -6,8 +6,6 @@ > #include <linux/prime_numbers.h> > #include <linux/slab.h> > > -#define bitmap_size(nbits) (BITS_TO_LONGS(nbits) * sizeof(unsigned long)) > - > struct primes { > struct rcu_head rcu; > unsigned long last, sz; > diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h > index f3566ea0f932..81a2299ace15 100644 > --- a/tools/include/linux/bitmap.h > +++ b/tools/include/linux/bitmap.h > @@ -2,6 +2,7 @@ > #ifndef _TOOLS_LINUX_BITMAP_H > #define _TOOLS_LINUX_BITMAP_H > > +#include <linux/align.h> > #include <string.h> > #include <linux/bitops.h> > #include <linux/find.h> > @@ -25,13 +26,14 @@ bool __bitmap_intersects(const unsigned long *bitmap1, > #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) > #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) > > +#define bitmap_size(nbits) (ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE) > + > static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) > { > if (small_const_nbits(nbits)) > *dst = 0UL; > else { > - int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); > - memset(dst, 0, len); > + memset(dst, 0, bitmap_size(nbits)); > } > } > > @@ -83,7 +85,7 @@ static inline void bitmap_or(unsigned long *dst, const unsigned long *src1, > */ > static inline unsigned long *bitmap_zalloc(int nbits) > { > - return calloc(1, BITS_TO_LONGS(nbits) * sizeof(unsigned long)); > + return calloc(1, bitmap_size(nbits)); > } > > /* > -- > 2.41.0
diff --git a/drivers/md/dm-clone-metadata.c b/drivers/md/dm-clone-metadata.c index c43d55672bce..47c1fa7aad8b 100644 --- a/drivers/md/dm-clone-metadata.c +++ b/drivers/md/dm-clone-metadata.c @@ -465,11 +465,6 @@ static void __destroy_persistent_data_structures(struct dm_clone_metadata *cmd) /*---------------------------------------------------------------------------*/ -static size_t bitmap_size(unsigned long nr_bits) -{ - return BITS_TO_LONGS(nr_bits) * sizeof(long); -} - static int __dirty_map_init(struct dirty_map *dmap, unsigned long nr_words, unsigned long nr_regions) { diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h index 03644237e1ef..63e422f8ba3d 100644 --- a/include/linux/bitmap.h +++ b/include/linux/bitmap.h @@ -237,9 +237,11 @@ extern int bitmap_print_list_to_buf(char *buf, const unsigned long *maskp, #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) +#define bitmap_size(nbits) (ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE) + static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) { - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); + unsigned int len = bitmap_size(nbits); if (small_const_nbits(nbits)) *dst = 0; @@ -249,7 +251,7 @@ static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) static inline void bitmap_fill(unsigned long *dst, unsigned int nbits) { - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); + unsigned int len = bitmap_size(nbits); if (small_const_nbits(nbits)) *dst = ~0UL; @@ -260,7 +262,7 @@ static inline void bitmap_fill(unsigned long *dst, unsigned int nbits) static inline void bitmap_copy(unsigned long *dst, const unsigned long *src, unsigned int nbits) { - unsigned int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); + unsigned int len = bitmap_size(nbits); if (small_const_nbits(nbits)) *dst = *src; diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h index f10fb87d49db..dbdbf1451cad 100644 --- a/include/linux/cpumask.h +++ b/include/linux/cpumask.h @@ -821,7 +821,7 @@ static inline int cpulist_parse(const char *buf, struct cpumask *dstp) */ static inline unsigned int cpumask_size(void) { - return BITS_TO_LONGS(large_cpumask_bits) * sizeof(long); + return bitmap_size(large_cpumask_bits); } /* diff --git a/lib/math/prime_numbers.c b/lib/math/prime_numbers.c index d42cebf7407f..d3b64b10da1c 100644 --- a/lib/math/prime_numbers.c +++ b/lib/math/prime_numbers.c @@ -6,8 +6,6 @@ #include <linux/prime_numbers.h> #include <linux/slab.h> -#define bitmap_size(nbits) (BITS_TO_LONGS(nbits) * sizeof(unsigned long)) - struct primes { struct rcu_head rcu; unsigned long last, sz; diff --git a/tools/include/linux/bitmap.h b/tools/include/linux/bitmap.h index f3566ea0f932..81a2299ace15 100644 --- a/tools/include/linux/bitmap.h +++ b/tools/include/linux/bitmap.h @@ -2,6 +2,7 @@ #ifndef _TOOLS_LINUX_BITMAP_H #define _TOOLS_LINUX_BITMAP_H +#include <linux/align.h> #include <string.h> #include <linux/bitops.h> #include <linux/find.h> @@ -25,13 +26,14 @@ bool __bitmap_intersects(const unsigned long *bitmap1, #define BITMAP_FIRST_WORD_MASK(start) (~0UL << ((start) & (BITS_PER_LONG - 1))) #define BITMAP_LAST_WORD_MASK(nbits) (~0UL >> (-(nbits) & (BITS_PER_LONG - 1))) +#define bitmap_size(nbits) (ALIGN(nbits, BITS_PER_LONG) / BITS_PER_BYTE) + static inline void bitmap_zero(unsigned long *dst, unsigned int nbits) { if (small_const_nbits(nbits)) *dst = 0UL; else { - int len = BITS_TO_LONGS(nbits) * sizeof(unsigned long); - memset(dst, 0, len); + memset(dst, 0, bitmap_size(nbits)); } } @@ -83,7 +85,7 @@ static inline void bitmap_or(unsigned long *dst, const unsigned long *src1, */ static inline unsigned long *bitmap_zalloc(int nbits) { - return calloc(1, BITS_TO_LONGS(nbits) * sizeof(unsigned long)); + return calloc(1, bitmap_size(nbits)); } /*