[RFC PATCH v1 00/57] Boot-time page size selection for arm64

[Ryan Roberts]

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<---------------------------
>