Hi Willy,

On Fri, Jun 25, 2021 at 5:50 PM Willy Tarreau [off-list ref] wrote:

On Fri, Jun 25, 2021 at 05:38:03PM +0200, Pali Rohár wrote:

quoted

On Friday 25 June 2021 17:22:31 Geert Uytterhoeven wrote:

quoted

quoted

+/*
+ * DIV_U64_ROUND_CLOSEST - unsigned 64bit divide with 32bit divisor rounded to nearest integer
+ * @dividend: unsigned 64bit dividend
+ * @divisor: unsigned 32bit divisor
+ *
+ * Divide unsigned 64bit dividend by unsigned 32bit divisor
+ * and round to closest integer.
+ *
+ * Return: dividend / divisor rounded to nearest integer
+ */
+#define DIV_U64_ROUND_CLOSEST(dividend, divisor)       \
+       ({ u32 _tmp = (divisor); div_u64((u64)(dividend) + _tmp / 2, _tmp); })

Given "dividend" should already be an unsigned 64-bit value, I don't
think the cast to "u64" is needed. Similar macros in this file also
don't have the cast.

It is just to ensure that plus operation between dividend and _tmp is
evaluated in 64-bit context to prevent overflow. Just a case when user
calls this macro with 32-bit dividend param. As it is a macro (and not
inline function) type is not automatically enforced.

I agree, a large u32 argument added to _tmp/2 could overflow and remain
32 bits, yielding an incorrect result. The cast is mandatory here (and
will either emit no code, or be useful).

Fair enough.
So we want to add a cast to DIV64_U64_ROUND_CLOSEST() above, too?

The only trap I'm seeing is if a negative signed int is passed in dividend,
it will be sign-extended and will give a large u64 value. A preliminary
u32 cast could avoid this but would break valid u64 arguments, and I'd
claim we never know what the user wants if this happens in the first place.

Yep.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel