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

From: John Garry <[email-masked]>
Date: Fri, 21 Jun 2024 18:02:47 +0100

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