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

[Catalin Marinas <catalin.marinas@arm.com>]

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

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

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

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

-- 
Catalin