Re: [PATCH] random: vDSO: Redefine PAGE_SIZE and PAGE_MASK

[LEROY Christophe]

Hi Vicenzo,

Le 27/08/2024 à 18:05, Vincenzo Frascino a écrit :
> Hi Christophe,
> 
> On 27/08/2024 11:49, Christophe Leroy wrote:
> 
> ...
> 
> 
> 
> 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.
> 

Your patch looks nice, maybe a bit too much. For instance getrandom.c 
can include directly asm/vdso/page.h instead of creating vdso/page.h

Or create a vdso/page.h that only use CONFIG_PAGE_SHIFT and doesn't 
include anything from architectures.

We should also keep PROT_READ and PROT_WRITE in getrandom.c , that's 
better for readability. Same for MAP_DROPPABLE | MAP_ANONYMOUS. I can't 
see the benefit of hiding them in a header.

I can't see which header provides you with min_t() or ARRAY_SIZE().

I think you should also work on removing headers included by 
arch/x86/include/asm/vdso/gettimeofday.h which is included by 
include/vdso/datapage.h :

   #include <uapi/linux/time.h>
   #include <asm/vgtod.h>
   #include <asm/vvar.h>
   #include <asm/unistd.h>
   #include <asm/msr.h>
   #include <asm/pvclock.h>
   #include <clocksource/hyperv_timer.h>

As a comparison, the one from powerpc only includes the following one so 
it pulls a lot less non-vdso headers:

   #include <asm/vdso/timebase.h>
   #include <asm/barrier.h>
   #include <asm/unistd.h>
   #include <uapi/linux/time.h>

Christophe