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
