Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
Date:   Sun, 22 Oct 2023 15:17:54 +0200
From:   Vasily Gorbik <gor@linux.ibm.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     Andy Shevchenko <andriy.shevchenko@intel.com>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        Heiko Carstens <hca@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org
Subject: Re: [GIT PULL] s390 fixes for 6.6-rc7
Message-ID: <your-ad-here.call-01697980674-ext-9589@work.hours>
References: <your-ad-here.call-01697881440-ext-2458@work.hours>
 <CAHk-=wgTUz1bdY6zvsN4ED0arCLE8Sb==1GH8d0sjm5bu7zesQ@mail.gmail.com>
 <CAHk-=wjxAk=PQBX4OvscNfGKfc+M8iKmNd6D39wOZzXV0k729w@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAHk-=wjxAk=PQBX4OvscNfGKfc+M8iKmNd6D39wOZzXV0k729w@mail.gmail.com>
Precedence: bulk

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@linux-foundation.org> 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