Re: [PATCH tip] x86/mm: Correct fixmap header usage on adaptable MODULES_END

[Wei Yang]

On Fri, Mar 17, 2017 at 10:50:34AM -0700, Thomas Garnier wrote:
>This patch remove fixmap header usage on non-x86 code that was
>introduced by the adaptable MODULE_END change.

Hi, Thomas

In this patch, it looks you are trying to do two things for my understanding:
1. To include <asm/fixmap.h> in asm/pagetable_64.h and remove the include in
some of the x86 files
2. Remove <asm/fixmap.h> in mm/vmalloc.c

I think your change log covers the second task in the patch, but not not talk
about the first task you did in the patch. If you could mention it in commit
log, it would be good for maintain.

BTW, I have little knowledge about MODULE_END. By searching the code
MODULE_END is not used in arch/x86. If you would like to mention the commit
which introduce the problem, it would be more helpful to review the code.

>
>Signed-off-by: Thomas Garnier <thgarnie@xxxxxxxxxx>
>---
>Based on tip:x86/mm
>---
> arch/x86/include/asm/pgtable_64.h | 1 +
> arch/x86/kernel/module.c          | 1 -
> arch/x86/mm/dump_pagetables.c     | 1 -
> arch/x86/mm/kasan_init_64.c       | 1 -
> mm/vmalloc.c                      | 4 ----
> 5 files changed, 1 insertion(+), 7 deletions(-)
>
>diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
>index 73c7ccc38912..67608d4abc2c 100644
>--- a/arch/x86/include/asm/pgtable_64.h
>+++ b/arch/x86/include/asm/pgtable_64.h
>@@ -13,6 +13,7 @@
> #include <asm/processor.h>
> #include <linux/bitops.h>
> #include <linux/threads.h>
>+#include <asm/fixmap.h>
> 

Hmm... I see in both pgtable_32.h and pgtable_64.h will include <asm/fixmap.h>
after this change. And pgtable_32.h and pgtable_64.h will be included only in
pgtable.h. So is it possible to include <asm/fixmap.h> in pgtable.h for once
instead of include it in both files? Any concerns you would have?

> extern pud_t level3_kernel_pgt[512];
> extern pud_t level3_ident_pgt[512];
>diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
>index fad61caac75e..477ae806c2fa 100644
>--- a/arch/x86/kernel/module.c
>+++ b/arch/x86/kernel/module.c
>@@ -35,7 +35,6 @@
> #include <asm/page.h>
> #include <asm/pgtable.h>
> #include <asm/setup.h>
>-#include <asm/fixmap.h>
> 
> #if 0
> #define DEBUGP(fmt, ...)				\
>diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
>index 75efeecc85eb..58b5bee7ea27 100644
>--- a/arch/x86/mm/dump_pagetables.c
>+++ b/arch/x86/mm/dump_pagetables.c
>@@ -20,7 +20,6 @@
> 
> #include <asm/kasan.h>
> #include <asm/pgtable.h>
>-#include <asm/fixmap.h>
> 
> /*
>  * The dumper groups pagetable entries of the same type into one, and for
>diff --git a/arch/x86/mm/kasan_init_64.c b/arch/x86/mm/kasan_init_64.c
>index 1bde19ef86bd..8d63d7a104c3 100644
>--- a/arch/x86/mm/kasan_init_64.c
>+++ b/arch/x86/mm/kasan_init_64.c
>@@ -9,7 +9,6 @@
> 
> #include <asm/tlbflush.h>
> #include <asm/sections.h>
>-#include <asm/fixmap.h>
> 
> extern pgd_t early_level4_pgt[PTRS_PER_PGD];
> extern struct range pfn_mapped[E820_X_MAX];
>diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>index b7d2a23349f4..0dd80222b20b 100644
>--- a/mm/vmalloc.c
>+++ b/mm/vmalloc.c
>@@ -36,10 +36,6 @@
> #include <asm/tlbflush.h>
> #include <asm/shmparam.h>
> 
>-#ifdef CONFIG_X86
>-# include <asm/fixmap.h>
>-#endif
>-
> #include "internal.h"
> 
> struct vfree_deferred {
>-- 
>2.12.0.367.g23dc2f6d3c-goog

-- 
Wei Yang
Help you, Help me

Attachment: signature.asc
Description: PGP signature