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

LEROY Christophe
2024-08-27T17:38:58Z

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