| Message ID | f69941b076bf8fec89b6bec5573fdb79483c2a75.1720597744.git.christophe.leroy@csgroup.eu (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] arch/x86: Drop own definition of pgd,p4d_leaf | expand |
On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote: > Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related > issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h > > Do the same for p4d_leaf() and pgd_leaf() to avoid getting them > erroneously defined by architectures that do not implement the > related page level. > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback > --- Thanks. I'd drop the inline functions, but no strong opinions. Reviewed-by: Peter Xu <peterx@redhat.com>
Le 10/07/2024 à 16:46, Peter Xu a écrit : > On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote: >> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related >> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h >> >> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them >> erroneously defined by architectures that do not implement the >> related page level. >> >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >> --- >> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback >> --- > > Thanks. I'd drop the inline functions, but no strong opinions. Inline functions enable type checking. With a macro you would be able to write pud_leaf(pgd) without the compiler noticing the mistake. All other helpers in asm-generic/pgtable-nopXd.h are functions so from my point of view it makes sense to keep them as functions not macros. > > Reviewed-by: Peter Xu <peterx@redhat.com> > Thanks.
On Wed, Jul 10, 2024 at 02:54:36PM +0000, LEROY Christophe wrote: > > > Le 10/07/2024 à 16:46, Peter Xu a écrit : > > On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote: > >> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related > >> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h > >> > >> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them > >> erroneously defined by architectures that do not implement the > >> related page level. > >> > >> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > >> --- > >> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback > >> --- > > > > Thanks. I'd drop the inline functions, but no strong opinions. > > Inline functions enable type checking. > > With a macro you would be able to write pud_leaf(pgd) without the > compiler noticing the mistake. > > All other helpers in asm-generic/pgtable-nopXd.h are functions so from > my point of view it makes sense to keep them as functions not macros. Whoever fallbacks to the pgtable.h pxx_leaf() will still use macros and lose the type check again. I'd rather rely on cross-arch builds and most of real *_leaf() users will always detect a type mismatch. Totally no big deal, and I agree keeping them match nopxd.h rules makes sense. Thanks,
Le 10/07/2024 à 20:41, Peter Xu a écrit : > On Wed, Jul 10, 2024 at 02:54:36PM +0000, LEROY Christophe wrote: >> >> >> Le 10/07/2024 à 16:46, Peter Xu a écrit : >>> On Wed, Jul 10, 2024 at 09:51:22AM +0200, Christophe Leroy wrote: >>>> Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related >>>> issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h >>>> >>>> Do the same for p4d_leaf() and pgd_leaf() to avoid getting them >>>> erroneously defined by architectures that do not implement the >>>> related page level. >>>> >>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> >>>> --- >>>> v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback >>>> --- >>> >>> Thanks. I'd drop the inline functions, but no strong opinions. >> >> Inline functions enable type checking. >> >> With a macro you would be able to write pud_leaf(pgd) without the >> compiler noticing the mistake. >> >> All other helpers in asm-generic/pgtable-nopXd.h are functions so from >> my point of view it makes sense to keep them as functions not macros. > > Whoever fallbacks to the pgtable.h pxx_leaf() will still use macros and > lose the type check again. I'd rather rely on cross-arch builds and most > of real *_leaf() users will always detect a type mismatch. > > Totally no big deal, and I agree keeping them match nopxd.h rules makes > sense. > Surprisingly, having both a macro and a static inline simultaneously defining pud_leaf() on loongarch was not a problem but as soon as there are two macros the compiler cries. I will wait a bit more to see if robots report anything else then I'll send an update fix. Christophe
On Thu, Jul 11, 2024 at 01:40:17PM +0000, LEROY Christophe wrote: > I will wait a bit more to see if robots report anything else then I'll > send an update fix. In case helpful, I normally use this small bunch of scripts to test cross-compiles (and I keep adding new configs that can fail any of my trees): https://gitlab.com/peterx/lkb-harness/ Just FYI.. I think different people use different set of tools.
diff --git a/include/asm-generic/pgtable-nop4d.h b/include/asm-generic/pgtable-nop4d.h index 03b7dae47dd4..ed7ba008469f 100644 --- a/include/asm-generic/pgtable-nop4d.h +++ b/include/asm-generic/pgtable-nop4d.h @@ -21,6 +21,8 @@ typedef struct { pgd_t pgd; } p4d_t; static inline int pgd_none(pgd_t pgd) { return 0; } static inline int pgd_bad(pgd_t pgd) { return 0; } static inline int pgd_present(pgd_t pgd) { return 1; } +static inline int pgd_leaf(pgd_t pgd) { return 0; } +#define pgd_leaf pgd_leaf static inline void pgd_clear(pgd_t *pgd) { } #define p4d_ERROR(p4d) (pgd_ERROR((p4d).pgd)) diff --git a/include/asm-generic/pgtable-nopmd.h b/include/asm-generic/pgtable-nopmd.h index b01349a312fa..e178ace2e23e 100644 --- a/include/asm-generic/pgtable-nopmd.h +++ b/include/asm-generic/pgtable-nopmd.h @@ -31,6 +31,7 @@ static inline int pud_none(pud_t pud) { return 0; } static inline int pud_bad(pud_t pud) { return 0; } static inline int pud_present(pud_t pud) { return 1; } static inline int pud_leaf(pud_t pud) { return 0; } +#define pud_leaf pud_leaf static inline void pud_clear(pud_t *pud) { } #define pmd_ERROR(pmd) (pud_ERROR((pmd).pud)) diff --git a/include/asm-generic/pgtable-nopud.h b/include/asm-generic/pgtable-nopud.h index eb70c6d7ceff..655dfebea91c 100644 --- a/include/asm-generic/pgtable-nopud.h +++ b/include/asm-generic/pgtable-nopud.h @@ -28,6 +28,8 @@ typedef struct { p4d_t p4d; } pud_t; static inline int p4d_none(p4d_t p4d) { return 0; } static inline int p4d_bad(p4d_t p4d) { return 0; } static inline int p4d_present(p4d_t p4d) { return 1; } +static inline int p4d_leaf(p4d_t p4d) { return 0; } +#define p4d_leaf p4d_leaf static inline void p4d_clear(p4d_t *p4d) { } #define pud_ERROR(pud) (p4d_ERROR((pud).p4d))
Commit 2c8a81dc0cc5 ("riscv/mm: fix two page table check related issues") added pud_leaf() in include/asm-generic/pgtable-nopmd.h Do the same for p4d_leaf() and pgd_leaf() to avoid getting them erroneously defined by architectures that do not implement the related page level. Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> --- v2: Added pXd_leaf macro as well in asm-generic/pgtable-nopXd.h to cohabit with the fallback --- include/asm-generic/pgtable-nop4d.h | 2 ++ include/asm-generic/pgtable-nopmd.h | 1 + include/asm-generic/pgtable-nopud.h | 2 ++ 3 files changed, 5 insertions(+)