Hi Christophe,
On 27/08/2024 11:49, Christophe Leroy wrote:
...
quoted
These are still two headers outside of the vdso/ namespace. For arm64
we had concluded that this is never safe, and any vdso header should
only include other vdso headers so we never pull in anything that
e.g. depends on memory management headers that are in turn broken
for the compat vdso.
The array_size.h header is really small, so that one could
probably just be moved into the vdso/ namespace. The minmax.h
header is already rather complex, so it may be better to just
open-code the usage of MIN/MAX where needed?
It is used at two places only so yes can to that.
Could you please clarify where minmax is needed? I tried to build Jason's master
tree for x86, commenting the header and it seems building fine. I might be
missing something.
Same for ARRAY_SIZE(->reserved) by the way, easy to do opencode, we also have it
only once
I have a similar issue to figure out why linux/array_size.h and
uapi/linux/random.h are needed. It seems that I can build the object without
them. Could you please explain?
In general, the philosophy adopted to split the headers is to extract the set of
functions used by vDSOs from the linux headers and place them in the vdso headers.
Consequently the linux header includes to vdso one. E.g.: linux/time64.h
includes vdso/time64.h.
IMHO we should follow the same approach, if at all for consistency, unless there
is a valid reason.
...
quoted hunk ↗ jump to hunk
quoted
Including uapi/linux/mman.h may still be problematic on
some architectures if they change it in a way that is
incompatible with compat vdso, but at least that can't
accidentally rely on CONFIG_64BIT or something else that
would be wrong there.
Yes that one is tricky. Because uapi/linux/mman.h includes asm/mman.h with the
intention to include uapi/asm/mman.h but when built from the kernel in reality
you get arch/powerpc/include/asm/mman.h and I had to add some ifdefery to
kick-out kernel oddities it contains that pull additional kernel headers.
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 17a77d47ed6d..42a51a993d94 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
#include <uapi/asm/mman.h>
-#ifdef CONFIG_PPC64
+#if defined(CONFIG_PPC64) && !defined(BUILD_VDSO)
#include <asm/cputable.h>
#include <linux/mm.h>
I agree with Arnd here. uapi/linux/mman.h can cause us problems in the long run.
I am attaching a patch to provide my view on how to minimize the headers
included and use only the vdso/ namespace. Please, before using the code,
consider that I conducted very limited testing.
Note: It should apply clean on Jason's tree.
Let me know your thoughts.
Christophe
--
Regards,
Vincenzo