On 07/11/2024 12:35, Catalin Marinas wrote:
> On Wed, Nov 06, 2024 at 11:37:58AM +0000, Ryan Roberts wrote:
>> On 31/10/2024 21:07, Catalin Marinas wrote:
>>> So, first of all, I'd like to understand the overall maintainability
>>> impact better. I assume you tested mostly defconfig. If you run an
>>> allmodconfig build with make -k, how many build failures do you get with
>>> this patchset? Similarly for some distro configs.
>>
>> I've roughly done:
>>
>> make alldefconfig &&
>> ./scripts/config --enable CONFIG_ARM64_BOOT_TIME_PAGE_SIZE &&
>> make -s -j`nproc` -k &> allmodconfig.log
>
> Is it alldefconfig or allmodconfig? The former has a lot less symbols
> enabled than even defconfig (fairly close to allnoconfig actually):
Eek, that was a typo when I wrote the email... I built allmodconfig - the big one.
>
> $ make defconfig
> $ grep -v "^#\|^$" .config | wc -l
> 4449
>
> $ make alldefconfig
> $ grep -v "^#\|^$" .config | wc -l
> 713
>
> $ make allmodconfig
> $ grep -v "^#\|^$" .config | wc -l
> 14401
>
>> In the end, I decided to go for r'(\S+\.[ch]):.*(error|note):', which is any
>> files described as having an error or being the callsite of the thing with the
>> error. I think this is likely most accurate from eyeballing the log:
>
> I think that's good enough to give us a rough idea.
>
>> | | C&H files | percentage of |
>> | directory | w/ error | all C&H files |
>> |------------|---------------|---------------|
>> | arch/arm64 | 7 | 1.3% |
>> | drivers | 127 | 0.4% |
>> | fs | 25 | 1.1% |
>> | include | 27 | 0.4% |
>> | init | 1 | 8.3% |
>> | kernel | 7 | 1.3% |
>> | lib | 1 | 0.2% |
>> | mm | 6 | 3.2% |
>> | net | 7 | 0.4% |
>> | security | 2 | 0.8% |
>> | sound | 21 | 0.8% |
>> |------------|---------------|---------------|
>> | TOTAL | 231 | 0.4% |
>> |------------|---------------|---------------|
>
> This doesn't look that bad _if_ you actually built most modules. But if
> it was alldefconfig, you likely missed the majority of modules.
I definitely built allmodconfig, so I guess "this doesn't look bad" :)
>
>>> Do we have any better way to detect this other than actual compilation
>>> on arm64? Can we hack something around COMPILE_TEST like redefine
>>> PAGE_SIZE (for modules only) to a variable so that we have a better
>>> chance of detecting build failures when modules are only tested on other
>>> architectures?
>>
>> I can certainly look into this. But if the concern is that drivers are not being
>> compiled against arm64, what is the likelyhood of them being compiled against
>> COMPILE_TEST?
>
> Hopefully some CIs out there catching them. Well, if we are to fix them
> anyway, we might as well eventually force a non-const PAGE_SIZE
> generically even if it returns a constant.
>
> I'm building allmod now with something like below (and some hacks in
> arch and core code to use STATIC_PAGE_* as I did not apply your
> patches). alldefconfig passes with my hacks but, as you can see, the
> non-const PAGE_SIZE kicks in only if MODULE is defined. So, not an
> accurate test, just to get a feel of the modules problem.
Nice. I guess that's pretty much the change we would add for x86 with COMPILE_TEST.
>
> ----------8<---------------------------
> diff --git a/arch/arm64/include/asm/page-def.h b/arch/arm64/include/asm/page-def.h
> index 792e9fe881dc..71a761f86b15 100644
> --- a/arch/arm64/include/asm/page-def.h
> +++ b/arch/arm64/include/asm/page-def.h
> @@ -12,7 +12,19 @@
>
> /* PAGE_SHIFT determines the page size */
> #define PAGE_SHIFT CONFIG_PAGE_SHIFT
> -#define PAGE_SIZE (_AC(1, UL) << PAGE_SHIFT)
> +#define STATIC_PAGE_SIZE (_AC(1, UL) << PAGE_SHIFT)
> +#define STATIC_PAGE_MASK (~(STATIC_PAGE_SIZE-1))
> +
> +#if !defined(MODULE) || defined(__ASSEMBLY__)
> +#define PAGE_SIZE STATIC_PAGE_SIZE
> +#else
> +static inline unsigned long __runtime_page_size(void)
> +{
> + return 1UL << PAGE_SHIFT;
> +}
> +#define PAGE_SIZE (__runtime_page_size())
> +#endif
> +
> #define PAGE_MASK (~(PAGE_SIZE-1))
>
> #endif /* __ASM_PAGE_DEF_H */
> ----------8<---------------------------
>
