On Wed, Nov 06, 2024 at 11:37:58AM +0000, Ryan Roberts wrote:

On 31/10/2024 21:07, Catalin Marinas wrote:

quoted

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.

quoted

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