Re: linux-next: build failure after merge of the block tree

John Garry
2024-06-21T17:03:07Z

On 21/06/2024 16:58, Jens Axboe wrote:
> On 6/21/24 9:40 AM, Mark Brown wrote:
>> Hi all,
>>
>> After merging the block tree, today's linux-next build (arm
>> multi_v7_defconfig) failed like this:
>>
>> In file included from /tmp/next/build/include/linux/printk.h:10,
>> from /tmp/next/build/include/linux/kernel.h:31,
>> from /tmp/next/build/block/blk-settings.c:5:
>> /tmp/next/build/block/blk-settings.c: In function 'blk_validate_atomic_write_limits':
>> /tmp/next/build/include/asm-generic/div64.h:222:35: warning: comparison of distinct pointer types lacks a cast
>> 222 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
>> | ^~
>> /tmp/next/build/include/linux/once_lite.h:28:41: note: in definition of macro 'DO_ONCE_LITE_IF'
>> 28 | bool __ret_do_once = !!(condition); \
>> | ^~~~~~~~~
>> /tmp/next/build/block/blk-settings.c:200:21: note: in expansion of macro 'WARN_ON_ONCE'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~~~~~~~
>> /tmp/next/build/block/blk-settings.c:200:34: note: in expansion of macro 'do_div'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~
>> /tmp/next/build/include/asm-generic/div64.h:234:32: warning: right shift count >= width of type [-Wshift-count-overflow]
>> 234 | } else if (likely(((n) >> 32) == 0)) { \
>> | ^~
>> /tmp/next/build/include/linux/once_lite.h:28:41: note: in definition of macro 'DO_ONCE_LITE_IF'
>> 28 | bool __ret_do_once = !!(condition); \
>> | ^~~~~~~~~
>> /tmp/next/build/block/blk-settings.c:200:21: note: in expansion of macro 'WARN_ON_ONCE'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~~~~~~~
>> /tmp/next/build/include/asm-generic/div64.h:234:20: note: in expansion of macro 'likely'
>> 234 | } else if (likely(((n) >> 32) == 0)) { \
>> | ^~~~~~
>> /tmp/next/build/block/blk-settings.c:200:34: note: in expansion of macro 'do_div'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~
>> /tmp/next/build/include/asm-generic/div64.h:238:36: error: passing argument 1 of '__div64_32' from incompatible pointer type [-Werror=incompatible-pointer-types]
>> 238 | __rem = __div64_32(&(n), __base); \
>> | ^~~~
>> | |
>> | unsigned int *
>> /tmp/next/build/include/linux/once_lite.h:28:41: note: in definition of macro 'DO_ONCE_LITE_IF'
>> 28 | bool __ret_do_once = !!(condition); \
>> | ^~~~~~~~~
>> /tmp/next/build/block/blk-settings.c:200:21: note: in expansion of macro 'WARN_ON_ONCE'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~~~~~~~
>> /tmp/next/build/block/blk-settings.c:200:34: note: in expansion of macro 'do_div'
>> 200 | if (WARN_ON_ONCE(do_div(chunk_sectors, boundary_sectors)))
>> | ^~~~~~
>> In file included from /tmp/next/build/include/linux/math.h:6,
>> from /tmp/next/build/include/linux/kernel.h:27,
>> from /tmp/next/build/block/blk-settings.c:5:
>> /tmp/next/build/arch/arm/include/asm/div64.h:24:45: note: expected 'uint64_t *' {aka 'long long unsigned int *'} but argument is of type 'unsigned int *'
>> 24 | static inline uint32_t __div64_32(uint64_t *n, uint32_t base)
>> | ~~~~~~~~~~^
>> cc1: some warnings being treated as errors

I did a i386 build, and no complaints.

Now I see that the asm-generic version for 32b architectures check for a
64b dividend, but x86_32 does not. As for arm32, its version of do_div
expects a uint64_t *.

Maybe this would not be a bad change to have:

--- a/arch/x86/include/asm/div64.h
+++ b/arch/x86/include/asm/div64.h
@@ -22,6 +22,7 @@
#define do_div(n, base) \
({ \
unsigned long __upper, __low, __high, __mod, __base; \
+ (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \
__base = (base); \
if (__builtin_constant_p(__base) && is_power_of_2(__base)) { \
__mod = n & (__base - 1); \

to catch this same error on i386.

>
> We need something ala:
>
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index 37fe4c8f6b6b..cd33eaba4610 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -175,7 +175,7 @@ static void blk_atomic_writes_update_limits(struct queue_limits *lim)
>
> static void blk_validate_atomic_write_limits(struct queue_limits *lim)
> {
> - unsigned int chunk_sectors = lim->chunk_sectors;
> + u64 chunk_sectors = lim->chunk_sectors;
> unsigned int boundary_sectors;
>
> if (!lim->atomic_write_hw_max)
>

Sure, but I think that we can also use the following, since both are
unsigned int:
if (WARN_ON_ONCE(chunk_sectors % boundary_sectors))

I'll send a fix.

John