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
