On Tue, Apr 18, 2023 at 12:02:47AM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@xxxxxxxx>
>
> The new module statistics code mixes 64-bit types and wordsized 'long'
> variables, which leads to build failures on 32-bit architectures:
Doh. 0-day had not complained!
> kernel/module/stats.c: In function 'read_file_mod_stats':
> kernel/module/stats.c:291:29: error: passing argument 1 of 'atomic64_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
> 291 | total_size = atomic64_read(&total_mod_size);
> x86_64-linux-ld: kernel/module/stats.o: in function `read_file_mod_stats':
> stats.c:(.text+0x2b2): undefined reference to `__udivdi3'
>
> To fix this, the code has to use one of the two types consistently.
>
> Change them all to fixed-size 64-bit ones here.
>
> Fixes: 0d4ab68ce983 ("module: add debug stats to help identify memory pressure")
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> I have no idea if there is a risk of these variables actually
> overflowing 'long' on 32-bit machines. If they provably can't, it
> would be better to do the opposite patch.
I had originally used atomic64_t and added a debugfs knob for it but
Linus had advised against it because its not a stat we care too much
on 32-bit and atomic64 is nasty on 32-bit [0].
So I went with atomic_long and the cast becuase we're just reading.
Is there a way to fix this without doing the fully jump? If not oh well.
[0] https://lkml.kernel.org/r/CAHk-=whH+OsAY+9qLc9Hz+-W8u=dvD3NLWHemOQpZPcgZa52fA@xxxxxxxxxxxxxx
Luis
> ---
> kernel/module/internal.h | 14 +++++------
> kernel/module/main.c | 10 ++++----
> kernel/module/stats.c | 50 ++++++++++++++++++++--------------------
> 3 files changed, 37 insertions(+), 37 deletions(-)
>
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index 9ba5f8df15bc..c1710b74027c 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -175,13 +175,13 @@ enum fail_dup_mod_reason {
>
> #ifdef CONFIG_MODULE_STATS
>
> -#define mod_stat_add_long(count, var) atomic_long_add(count, var)
> +#define mod_stat_add_64(count, var) atomic64_add(count, var)
> #define mod_stat_inc(name) atomic_inc(name)
>
> -extern atomic_long_t total_mod_size;
> -extern atomic_long_t total_text_size;
> -extern atomic_long_t invalid_kread_bytes;
> -extern atomic_long_t invalid_decompress_bytes;
> +extern atomic64_t total_mod_size;
> +extern atomic64_t total_text_size;
> +extern atomic64_t invalid_kread_bytes;
> +extern atomic64_t invalid_decompress_bytes;
>
> extern atomic_t modcount;
> extern atomic_t failed_kreads;
> @@ -189,7 +189,7 @@ extern atomic_t failed_decompress;
> struct mod_fail_load {
> struct list_head list;
> char name[MODULE_NAME_LEN];
> - atomic_long_t count;
> + atomic64_t count;
> unsigned long dup_fail_mask;
> };
>
> @@ -199,7 +199,7 @@ void mod_stat_bump_becoming(struct load_info *info, int flags);
>
> #else
>
> -#define mod_stat_add_long(name, var)
> +#define mod_stat_add_64(name, var)
> #define mod_stat_inc(name)
>
> static inline int try_add_failed_module(const char *name, size_t len,
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 1ed373145278..d1b213310e4b 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -2600,8 +2600,8 @@ static noinline int do_init_module(struct module *mod)
> mutex_unlock(&module_mutex);
> wake_up_all(&module_wq);
>
> - mod_stat_add_long(text_size, &total_text_size);
> - mod_stat_add_long(total_size, &total_mod_size);
> + mod_stat_add_64(text_size, &total_text_size);
> + mod_stat_add_64(total_size, &total_mod_size);
>
> mod_stat_inc(&modcount);
>
> @@ -3052,7 +3052,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
> err = copy_module_from_user(umod, len, &info);
> if (err) {
> mod_stat_inc(&failed_kreads);
> - mod_stat_add_long(len, &invalid_kread_bytes);
> + mod_stat_add_64(len, &invalid_kread_bytes);
> return err;
> }
>
> @@ -3081,7 +3081,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
> READING_MODULE);
> if (len < 0) {
> mod_stat_inc(&failed_kreads);
> - mod_stat_add_long(len, &invalid_kread_bytes);
> + mod_stat_add_64(len, &invalid_kread_bytes);
> return len;
> }
>
> @@ -3090,7 +3090,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
> vfree(buf); /* compressed data is no longer needed */
> if (err) {
> mod_stat_inc(&failed_decompress);
> - mod_stat_add_long(len, &invalid_decompress_bytes);
> + mod_stat_add_64(len, &invalid_decompress_bytes);
> return err;
> }
> } else {
> diff --git a/kernel/module/stats.c b/kernel/module/stats.c
> index bbf90190a3fe..c4db7f64cdae 100644
> --- a/kernel/module/stats.c
> +++ b/kernel/module/stats.c
> @@ -196,12 +196,12 @@ static LIST_HEAD(dup_failed_modules);
> * a separate module request was being issued for each CPU on a system.
> */
>
> -atomic_long_t total_mod_size;
> -atomic_long_t total_text_size;
> -atomic_long_t invalid_kread_bytes;
> -atomic_long_t invalid_decompress_bytes;
> -static atomic_long_t invalid_becoming_bytes;
> -static atomic_long_t invalid_mod_bytes;
> +atomic64_t total_mod_size;
> +atomic64_t total_text_size;
> +atomic64_t invalid_kread_bytes;
> +atomic64_t invalid_decompress_bytes;
> +static atomic64_t invalid_becoming_bytes;
> +static atomic64_t invalid_mod_bytes;
> atomic_t modcount;
> atomic_t failed_kreads;
> atomic_t failed_decompress;
> @@ -222,21 +222,21 @@ static const char *mod_fail_to_str(struct mod_fail_load *mod_fail)
>
> void mod_stat_bump_invalid(struct load_info *info, int flags)
> {
> - atomic_long_add(info->len * 2, &invalid_mod_bytes);
> + atomic64_add(info->len * 2, &invalid_mod_bytes);
> atomic_inc(&failed_load_modules);
> #if defined(CONFIG_MODULE_DECOMPRESS)
> if (flags & MODULE_INIT_COMPRESSED_FILE)
> - atomic_long_add(info->compressed_len, &invalid_mod_byte);
> + atomic64_add(info->compressed_len, &invalid_mod_bytes);
> #endif
> }
>
> void mod_stat_bump_becoming(struct load_info *info, int flags)
> {
> atomic_inc(&failed_becoming);
> - atomic_long_add(info->len, &invalid_becoming_bytes);
> + atomic64_add(info->len, &invalid_becoming_bytes);
> #if defined(CONFIG_MODULE_DECOMPRESS)
> if (flags & MODULE_INIT_COMPRESSED_FILE)
> - atomic_long_add(info->compressed_len, &invalid_becoming_bytes);
> + atomic64_add(info->compressed_len, &invalid_becoming_bytes);
> #endif
> }
>
> @@ -247,7 +247,7 @@ int try_add_failed_module(const char *name, size_t len, enum fail_dup_mod_reason
> list_for_each_entry_rcu(mod_fail, &dup_failed_modules, list,
> lockdep_is_held(&module_mutex)) {
> if (strlen(mod_fail->name) == len && !memcmp(mod_fail->name, name, len)) {
> - atomic_long_inc(&mod_fail->count);
> + atomic64_inc(&mod_fail->count);
> __set_bit(reason, &mod_fail->dup_fail_mask);
> goto out;
> }
> @@ -258,7 +258,7 @@ int try_add_failed_module(const char *name, size_t len, enum fail_dup_mod_reason
> return -ENOMEM;
> memcpy(mod_fail->name, name, len);
> __set_bit(reason, &mod_fail->dup_fail_mask);
> - atomic_long_inc(&mod_fail->count);
> + atomic64_inc(&mod_fail->count);
> list_add_rcu(&mod_fail->list, &dup_failed_modules);
> out:
> return 0;
> @@ -331,12 +331,12 @@ static ssize_t read_file_mod_stats(struct file *file, char __user *user_buf,
>
> if (live_mod_count && total_size) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Average mod size",
> - DIV_ROUND_UP(total_size, live_mod_count));
> + DIV64_U64_ROUND_UP(total_size, live_mod_count));
> }
>
> if (live_mod_count && text_size) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Average mod text size",
> - DIV_ROUND_UP(text_size, live_mod_count));
> + DIV64_U64_ROUND_UP(text_size, live_mod_count));
> }
>
> /*
> @@ -349,25 +349,25 @@ static ssize_t read_file_mod_stats(struct file *file, char __user *user_buf,
> WARN_ON_ONCE(ikread_bytes && !fkreads);
> if (fkreads && ikread_bytes) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Avg fail kread bytes",
> - DIV_ROUND_UP(ikread_bytes, fkreads));
> + DIV64_U64_ROUND_UP(ikread_bytes, fkreads));
> }
>
> WARN_ON_ONCE(ibecoming_bytes && !fbecoming);
> if (fbecoming && ibecoming_bytes) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Avg fail becoming bytes",
> - DIV_ROUND_UP(ibecoming_bytes, fbecoming));
> + DIV64_U64_ROUND_UP(ibecoming_bytes, fbecoming));
> }
>
> WARN_ON_ONCE(idecompress_bytes && !fdecompress);
> if (fdecompress && idecompress_bytes) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Avg fail decomp bytes",
> - DIV_ROUND_UP(idecompress_bytes, fdecompress));
> + DIV64_U64_ROUND_UP(idecompress_bytes, fdecompress));
> }
>
> WARN_ON_ONCE(imod_bytes && !floads);
> if (floads && imod_bytes) {
> len += scnprintf(buf + len, size - len, "%25s\t%llu\n", "Average fail load bytes",
> - DIV_ROUND_UP(imod_bytes, floads));
> + DIV64_U64_ROUND_UP(imod_bytes, floads));
> }
>
> /* End of our debug preamble header. */
> @@ -407,16 +407,16 @@ static const struct file_operations fops_mod_stats = {
> .llseek = default_llseek,
> };
>
> -#define mod_debug_add_ulong(name) debugfs_create_ulong(#name, 0400, mod_debugfs_root, (unsigned long *) &name.counter)
> +#define mod_debug_add_u64(name) debugfs_create_u64(#name, 0400, mod_debugfs_root, (s64 *)&name.counter)
> #define mod_debug_add_atomic(name) debugfs_create_atomic_t(#name, 0400, mod_debugfs_root, &name)
> static int __init module_stats_init(void)
> {
> - mod_debug_add_ulong(total_mod_size);
> - mod_debug_add_ulong(total_text_size);
> - mod_debug_add_ulong(invalid_kread_bytes);
> - mod_debug_add_ulong(invalid_decompress_bytes);
> - mod_debug_add_ulong(invalid_becoming_bytes);
> - mod_debug_add_ulong(invalid_mod_bytes);
> + mod_debug_add_u64(total_mod_size);
> + mod_debug_add_u64(total_text_size);
> + mod_debug_add_u64(invalid_kread_bytes);
> + mod_debug_add_u64(invalid_decompress_bytes);
> + mod_debug_add_u64(invalid_becoming_bytes);
> + mod_debug_add_u64(invalid_mod_bytes);
>
> mod_debug_add_atomic(modcount);
> mod_debug_add_atomic(failed_kreads);
> --
> 2.39.2
>
