On Sat, Oct 21, 2023 at 11:08:31AM -0700, Linus Torvalds wrote:
> in case you or somebody has a better idea for BITS_TO_LONG handling
> than just "you need to check for zero before and after".
>
> On Sat, 21 Oct 2023 at 10:56, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > If you *do* want to add proper overflow handling, you'd need to either
> > fix BITS_TO_LONGS() some way (which is actually non-trivial since it
> > needs to be able to stay a constant and only use the argument once),
> > or you do something like
> >
> > if (!bits)
> > return ZERO_SIZE_PTR;
> > longs = BITS_TO_LONG(bits);
> > if (!longs)
> > return NULL;
> > return vzalloc(longs * sizeof(long));
This might work.
BITS_TO_<TYPE>(bits) utilizes __KERNEL_DIV_ROUND_UP, which may potentially
result in an overflow condition when
bits > ULONG_MAX - sizeof(<TYPE>) * 8 + 1.
To resolve this issue, avoid using the overflow-prone
__KERNEL_DIV_ROUND_UP. To meet the requirements of BITS_TO<TYPE>(bits)
for remaining constant and preventing side effects from multiple
argument uses, employ __is_constexpr to differentiate between constant
and non-constant cases, employing a helper function in the latter.
In the constant case, this ensures compatibility with constructs like
DECLARE_BITMAP. While in the non-constant case, the __bits_to_elem_count
function could be optimized for potentially improved code generation
by compilers, though this might come at the expense of readability and
visual consistency between the constant and non-constant cases.
I could further investigate if this approach, in general, appears acceptable.
---
include/linux/bitops.h | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 2ba557e067fe..72be25d4b95d 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -15,11 +15,21 @@
# define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n)))
#endif
+static inline unsigned long __bits_to_elem_count(size_t nr, size_t sz)
+{
+ return nr / sz + (nr % sz ? 1 : 0);
+}
+
+#define BITS_TO_ELEM_COUNT(nr, sz) \
+ __builtin_choose_expr(__is_constexpr(nr), \
+ (nr) / sz + ((nr) % sz ? 1 : 0), \
+ __bits_to_elem_count((nr), sz))
+
#define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
-#define BITS_TO_LONGS(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
-#define BITS_TO_U64(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u64))
-#define BITS_TO_U32(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(u32))
-#define BITS_TO_BYTES(nr) __KERNEL_DIV_ROUND_UP(nr, BITS_PER_TYPE(char))
+#define BITS_TO_LONGS(nr) BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(long))
+#define BITS_TO_U64(nr) BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(u64))
+#define BITS_TO_U32(nr) BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(u32))
+#define BITS_TO_BYTES(nr) BITS_TO_ELEM_COUNT(nr, BITS_PER_TYPE(char))
extern unsigned int __sw_hweight8(unsigned int w);
extern unsigned int __sw_hweight16(unsigned int w);
--
2.39.2
