2017-03-20 19:41:43

by Thomas Garnier

[permalink] [raw]
Subject: [PATCH tip v2] x86/mm: Correct fixmap header usage on adaptable MODULES_END

This patch removes fixmap headers on non-x86 code introduced by the
adaptable MODULE_END change. It is also removed in the 32-bit pgtable
header. Instead, it is added by default in the pgtable generic header
for both architectures.

Signed-off-by: Thomas Garnier <[email protected]>
---
arch/x86/include/asm/pgtable.h | 1 +
arch/x86/include/asm/pgtable_32.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 ----
6 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 6f6f351e0a81..78d1fc32e947 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -598,6 +598,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
#include <linux/mm_types.h>
#include <linux/mmdebug.h>
#include <linux/log2.h>
+#include <asm/fixmap.h>

static inline int pte_none(pte_t pte)
{
diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
index fbc73360aea0..bfab55675c16 100644
--- a/arch/x86/include/asm/pgtable_32.h
+++ b/arch/x86/include/asm/pgtable_32.h
@@ -14,7 +14,6 @@
*/
#ifndef __ASSEMBLY__
#include <asm/processor.h>
-#include <asm/fixmap.h>
#include <linux/threads.h>
#include <asm/paravirt.h>

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


2017-03-21 02:01:02

by Wei Yang

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

On Mon, Mar 20, 2017 at 12:40:24PM -0700, Thomas Garnier wrote:
>This patch removes fixmap headers on non-x86 code introduced by the
>adaptable MODULE_END change. It is also removed in the 32-bit pgtable
>header. Instead, it is added by default in the pgtable generic header
>for both architectures.
>
>Signed-off-by: Thomas Garnier <[email protected]>
>---
> arch/x86/include/asm/pgtable.h | 1 +
> arch/x86/include/asm/pgtable_32.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 ----
> 6 files changed, 1 insertion(+), 8 deletions(-)
>
>diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
>index 6f6f351e0a81..78d1fc32e947 100644
>--- a/arch/x86/include/asm/pgtable.h
>+++ b/arch/x86/include/asm/pgtable.h
>@@ -598,6 +598,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
> #include <linux/mm_types.h>
> #include <linux/mmdebug.h>
> #include <linux/log2.h>
>+#include <asm/fixmap.h>
>
> static inline int pte_none(pte_t pte)
> {
>diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
>index fbc73360aea0..bfab55675c16 100644
>--- a/arch/x86/include/asm/pgtable_32.h
>+++ b/arch/x86/include/asm/pgtable_32.h
>@@ -14,7 +14,6 @@
> */
> #ifndef __ASSEMBLY__
> #include <asm/processor.h>
>-#include <asm/fixmap.h>
> #include <linux/threads.h>
> #include <asm/paravirt.h>
>

Yep, I thinks the above two is what I mean.

>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>
>

Hmm... your code is already merged in upstream?

When I look into current Torvalds tree, it looks not include the <asm/fixmap.h>

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/module.c

Which tree your change is based on? Do I miss something?

> #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 same as this one.

> /*
> * 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


At last, you have tested both on x86-32 and x86-64 platform?

--
Wei Yang
Help you, Help me


Attachments:
(No filename) (3.27 kB)
signature.asc (819.00 B)
Download all attachments

2017-03-21 07:17:33

by Ingo Molnar

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


* Thomas Garnier <[email protected]> wrote:

> This patch removes fixmap headers on non-x86 code introduced by the
> adaptable MODULE_END change. It is also removed in the 32-bit pgtable
> header. Instead, it is added by default in the pgtable generic header
> for both architectures.
>
> Signed-off-by: Thomas Garnier <[email protected]>
> ---
> arch/x86/include/asm/pgtable.h | 1 +
> arch/x86/include/asm/pgtable_32.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 ----
> 6 files changed, 1 insertion(+), 8 deletions(-)

So I already have v1 and there's no explanation about the changes from v1 to v2.

The interdiff between v1 and v2 is below, it only affects x86, presumably it's
done to simplify the header usage slightly: instead of including fixmap.h in both
pgtable_32/64.h it's only included in the common pgtable.h file.

That's a sensible cleanup of the original patch and I'd rather not rebase it (as
tip:x86/mm has other changes as well), so could I've applied the delta cleanup on
top of the existing changes, with its own changelog.

Thanks,

Ingo

============>
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 84f6ec4d47ec..9f6809545269 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -601,6 +601,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
#include <linux/mm_types.h>
#include <linux/mmdebug.h>
#include <linux/log2.h>
+#include <asm/fixmap.h>

static inline int pte_none(pte_t pte)
{
diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
index fbc73360aea0..bfab55675c16 100644
--- a/arch/x86/include/asm/pgtable_32.h
+++ b/arch/x86/include/asm/pgtable_32.h
@@ -14,7 +14,6 @@
*/
#ifndef __ASSEMBLY__
#include <asm/processor.h>
-#include <asm/fixmap.h>
#include <linux/threads.h>
#include <asm/paravirt.h>

diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 13709cf74ab6..1a4bc71534d4 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -13,7 +13,6 @@
#include <asm/processor.h>
#include <linux/bitops.h>
#include <linux/threads.h>
-#include <asm/fixmap.h>

extern pud_t level3_kernel_pgt[512];
extern pud_t level3_ident_pgt[512];

Subject: [tip:x86/mm] x86/headers: Simplify asm/fixmap.h inclusion into asm/pgtable*.h

Commit-ID: ef37bc361442545a5be3c56c49a08c3153032127
Gitweb: http://git.kernel.org/tip/ef37bc361442545a5be3c56c49a08c3153032127
Author: Thomas Garnier <[email protected]>
AuthorDate: Tue, 21 Mar 2017 08:17:25 +0100
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 21 Mar 2017 08:21:17 +0100

x86/headers: Simplify asm/fixmap.h inclusion into asm/pgtable*.h

Instead of including fixmap.h twice in pgtable_32.h and pgtable_64.h,
include it only once, in the common asm/pgtable.h header.

Signed-off-by: Thomas Garnier <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Paul Gortmaker <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Xiao Guangrong <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: zijun_hu <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Generated this patch from two other patches and wrote changelog. ]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/include/asm/pgtable.h | 1 +
arch/x86/include/asm/pgtable_32.h | 1 -
arch/x86/include/asm/pgtable_64.h | 1 -
3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 160256b..18a6f54 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -603,6 +603,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
#include <linux/mm_types.h>
#include <linux/mmdebug.h>
#include <linux/log2.h>
+#include <asm/fixmap.h>

static inline int pte_none(pte_t pte)
{
diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
index fbc7336..bfab5567 100644
--- a/arch/x86/include/asm/pgtable_32.h
+++ b/arch/x86/include/asm/pgtable_32.h
@@ -14,7 +14,6 @@
*/
#ifndef __ASSEMBLY__
#include <asm/processor.h>
-#include <asm/fixmap.h>
#include <linux/threads.h>
#include <asm/paravirt.h>

diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
index 13709cf..1a4bc71 100644
--- a/arch/x86/include/asm/pgtable_64.h
+++ b/arch/x86/include/asm/pgtable_64.h
@@ -13,7 +13,6 @@
#include <asm/processor.h>
#include <linux/bitops.h>
#include <linux/threads.h>
-#include <asm/fixmap.h>

extern pud_t level3_kernel_pgt[512];
extern pud_t level3_ident_pgt[512];

2017-03-21 16:04:56

by Thomas Garnier

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

On Tue, Mar 21, 2017 at 12:17 AM, Ingo Molnar <[email protected]> wrote:
>
> * Thomas Garnier <[email protected]> wrote:
>
>> This patch removes fixmap headers on non-x86 code introduced by the
>> adaptable MODULE_END change. It is also removed in the 32-bit pgtable
>> header. Instead, it is added by default in the pgtable generic header
>> for both architectures.
>>
>> Signed-off-by: Thomas Garnier <[email protected]>
>> ---
>> arch/x86/include/asm/pgtable.h | 1 +
>> arch/x86/include/asm/pgtable_32.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 ----
>> 6 files changed, 1 insertion(+), 8 deletions(-)
>
> So I already have v1 and there's no explanation about the changes from v1 to v2.
>
> The interdiff between v1 and v2 is below, it only affects x86, presumably it's
> done to simplify the header usage slightly: instead of including fixmap.h in both
> pgtable_32/64.h it's only included in the common pgtable.h file.
>

Correct, simplify the header and explains better.

> That's a sensible cleanup of the original patch and I'd rather not rebase it (as
> tip:x86/mm has other changes as well), so could I've applied the delta cleanup on
> top of the existing changes, with its own changelog.

I understand. Thanks for merging a clean-up version of this patch.

>
> Thanks,
>
> Ingo
>
> ============>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 84f6ec4d47ec..9f6809545269 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -601,6 +601,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
> #include <linux/mm_types.h>
> #include <linux/mmdebug.h>
> #include <linux/log2.h>
> +#include <asm/fixmap.h>
>
> static inline int pte_none(pte_t pte)
> {
> diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
> index fbc73360aea0..bfab55675c16 100644
> --- a/arch/x86/include/asm/pgtable_32.h
> +++ b/arch/x86/include/asm/pgtable_32.h
> @@ -14,7 +14,6 @@
> */
> #ifndef __ASSEMBLY__
> #include <asm/processor.h>
> -#include <asm/fixmap.h>
> #include <linux/threads.h>
> #include <asm/paravirt.h>
>
> diff --git a/arch/x86/include/asm/pgtable_64.h b/arch/x86/include/asm/pgtable_64.h
> index 13709cf74ab6..1a4bc71534d4 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -13,7 +13,6 @@
> #include <asm/processor.h>
> #include <linux/bitops.h>
> #include <linux/threads.h>
> -#include <asm/fixmap.h>
>
> extern pud_t level3_kernel_pgt[512];
> extern pud_t level3_ident_pgt[512];
>



--
Thomas

2017-03-21 16:09:07

by Thomas Garnier

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

On Mon, Mar 20, 2017 at 6:52 PM, Wei Yang <[email protected]> wrote:
> On Mon, Mar 20, 2017 at 12:40:24PM -0700, Thomas Garnier wrote:
>>This patch removes fixmap headers on non-x86 code introduced by the
>>adaptable MODULE_END change. It is also removed in the 32-bit pgtable
>>header. Instead, it is added by default in the pgtable generic header
>>for both architectures.
>>
>>Signed-off-by: Thomas Garnier <[email protected]>
>>---
>> arch/x86/include/asm/pgtable.h | 1 +
>> arch/x86/include/asm/pgtable_32.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 ----
>> 6 files changed, 1 insertion(+), 8 deletions(-)
>>
>>diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
>>index 6f6f351e0a81..78d1fc32e947 100644
>>--- a/arch/x86/include/asm/pgtable.h
>>+++ b/arch/x86/include/asm/pgtable.h
>>@@ -598,6 +598,7 @@ pte_t *populate_extra_pte(unsigned long vaddr);
>> #include <linux/mm_types.h>
>> #include <linux/mmdebug.h>
>> #include <linux/log2.h>
>>+#include <asm/fixmap.h>
>>
>> static inline int pte_none(pte_t pte)
>> {
>>diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
>>index fbc73360aea0..bfab55675c16 100644
>>--- a/arch/x86/include/asm/pgtable_32.h
>>+++ b/arch/x86/include/asm/pgtable_32.h
>>@@ -14,7 +14,6 @@
>> */
>> #ifndef __ASSEMBLY__
>> #include <asm/processor.h>
>>-#include <asm/fixmap.h>
>> #include <linux/threads.h>
>> #include <asm/paravirt.h>
>>
>
> Yep, I thinks the above two is what I mean.
>
>>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>
>>
>
> Hmm... your code is already merged in upstream?

It was merged on tip x86, it was my point before (as Ingo says after too).

>
> When I look into current Torvalds tree, it looks not include the <asm/fixmap.h>
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/module.c
>
> Which tree your change is based on? Do I miss something?

tip mm x86, before the first patch.

>
>> #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 same as this one.
>
>> /*
>> * 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
>
>
> At last, you have tested both on x86-32 and x86-64 platform?

I did, I know have a collected set of configs for both 32-bit and
64-bit and a script merging each config and building. It should reduce
the risk of exotic configurations not working.

>
> --
> Wei Yang
> Help you, Help me



--
Thomas