[PATCH v2 0/4] arm64: put objects into proper sections

STALE3678d

Revision v2 of 4 in this series.

6 messages, 2 authors, 2016-08-11 · open the first message on its own page

[PATCH v2 0/4] arm64: put objects into proper sections

From: Jisheng Zhang <hidden>
Date: 2016-08-11 13:41:33

This series tries to put objects into proper sections.

patch1 puts alloc_vectors_page() into .init section. This is a clean up
patch.
patch2 constify vm_special_mapping used for aarch32 vectors page.
patch3 fix the incorrect placement of __initdata and __read_mostly
patch4 apply __ro_after_init to some objects

Previously I only want to mark vdso_pages, vdso_spec, vectors_page and
cpu_ops as __read_mostly for performance reason. Then inspired by
Kees's patch[1], I think __ro_after_init is better.

[1] http://www.spinics.net/lists/arm-kernel/msg523188.html

Changes since v1:
  - use __ro_after_init instead of __read_mostly
  - find more objects to be suitable for __ro_after_init
  - add one more patch to fix the incorrect placement of __initdata and
    __read_mostly

Jisheng Zhang (4):
  arm64: vdso: add __init section marker to alloc_vectors_page
  arm64: vdso: constify vm_special_mapping used for aarch32 vectors page
  arm64: kaslr: Fix incorrect placement of __initdata and __read_mostly
  arm64: apply __ro_after_init to some objects

 arch/arm64/kernel/cpu_ops.c |  2 +-
 arch/arm64/kernel/kaslr.c   |  4 ++--
 arch/arm64/kernel/vdso.c    | 31 +++++++++++++++----------------
 arch/arm64/mm/dma-mapping.c |  2 +-
 arch/arm64/mm/init.c        |  4 ++--
 arch/arm64/mm/mmu.c         |  2 +-
 6 files changed, 22 insertions(+), 23 deletions(-)

-- 
2.8.1

[PATCH v2 2/4] arm64: vdso: constify vm_special_mapping used for aarch32 vectors page

From: Jisheng Zhang <hidden>
Date: 2016-08-11 13:41:30

The vm_special_mapping spec which is used for aarch32 vectors page is
never modified, so mark it as const.

Signed-off-by: Jisheng Zhang <redacted>
---
 arch/arm64/kernel/vdso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index e320e8f..10ad8ab 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -88,7 +88,7 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
 {
 	struct mm_struct *mm = current->mm;
 	unsigned long addr = AARCH32_VECTORS_BASE;
-	static struct vm_special_mapping spec = {
+	static const struct vm_special_mapping spec = {
 		.name	= "[vectors]",
 		.pages	= vectors_page,
 
-- 
2.8.1

[PATCH v2 1/4] arm64: vdso: add __init section marker to alloc_vectors_page

From: Jisheng Zhang <hidden>
Date: 2016-08-11 13:41:35

It is not needed after booting, this patch moves the alloc_vectors_page
function to the __init section.

Signed-off-by: Jisheng Zhang <redacted>
---
 arch/arm64/kernel/vdso.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 076312b..e320e8f 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -55,7 +55,7 @@ struct vdso_data *vdso_data = &vdso_data_store.data;
  */
 static struct page *vectors_page[1];
 
-static int alloc_vectors_page(void)
+static int __init alloc_vectors_page(void)
 {
 	extern char __kuser_helper_start[], __kuser_helper_end[];
 	extern char __aarch32_sigret_code_start[], __aarch32_sigret_code_end[];
-- 
2.8.1

[PATCH v2 4/4] arm64: apply __ro_after_init to some objects

From: Jisheng Zhang <hidden>
Date: 2016-08-11 13:41:42

These objects are set during initialization, thereafter are read only.

Previously I only want to mark vdso_pages, vdso_spec, vectors_page and
cpu_ops as __read_mostly from performance point of view. Then inspired
by Kees's patch[1] to apply more __ro_after_init for arm, I think it's
better to mark them as __ro_after_init. What's more, I find some more
objects are also read only after init. So apply __ro_after_init to all
of them.

This patch also removes global vdso_pagelist and tries to clean up
vdso_spec[] assignment code.

[1] http://www.spinics.net/lists/arm-kernel/msg523188.html

Signed-off-by: Jisheng Zhang <redacted>
---
 arch/arm64/kernel/cpu_ops.c |  2 +-
 arch/arm64/kernel/kaslr.c   |  2 +-
 arch/arm64/kernel/vdso.c    | 27 +++++++++++++--------------
 arch/arm64/mm/dma-mapping.c |  2 +-
 arch/arm64/mm/init.c        |  4 ++--
 arch/arm64/mm/mmu.c         |  2 +-
 6 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index c7cfb8f..6d32d1a 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -28,7 +28,7 @@ extern const struct cpu_operations smp_spin_table_ops;
 extern const struct cpu_operations acpi_parking_protocol_ops;
 extern const struct cpu_operations cpu_psci_ops;
 
-const struct cpu_operations *cpu_ops[NR_CPUS];
+const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
 
 static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
 	&smp_spin_table_ops,
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index 8ebabc4..93c47e1 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -20,7 +20,7 @@
 #include <asm/pgtable.h>
 #include <asm/sections.h>
 
-u64 module_alloc_base __read_mostly;
+u64 module_alloc_base __ro_after_init;
 u16 memstart_offset_seed __initdata;
 
 static __init u64 get_kaslr_seed(void *fdt)
diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index 10ad8ab..0af41de 100644
--- a/arch/arm64/kernel/vdso.c
+++ b/arch/arm64/kernel/vdso.c
@@ -37,8 +37,7 @@
 #include <asm/vdso_datapage.h>
 
 extern char vdso_start, vdso_end;
-static unsigned long vdso_pages;
-static struct page **vdso_pagelist;
+static unsigned long vdso_pages __ro_after_init;
 
 /*
  * The vDSO data page.
@@ -53,7 +52,7 @@ struct vdso_data *vdso_data = &vdso_data_store.data;
 /*
  * Create and map the vectors page for AArch32 tasks.
  */
-static struct page *vectors_page[1];
+static struct page *vectors_page[1] __ro_after_init;
 
 static int __init alloc_vectors_page(void)
 {
@@ -110,11 +109,19 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp)
 }
 #endif /* CONFIG_COMPAT */
 
-static struct vm_special_mapping vdso_spec[2];
+static struct vm_special_mapping vdso_spec[2] __ro_after_init = {
+	{
+		.name	= "[vvar]",
+	},
+	{
+		.name	= "[vdso]",
+	},
+};
 
 static int __init vdso_init(void)
 {
 	int i;
+	struct page **vdso_pagelist;
 
 	if (memcmp(&vdso_start, "\177ELF", 4)) {
 		pr_err("vDSO is not a valid ELF object!\n");
@@ -138,16 +145,8 @@ static int __init vdso_init(void)
 	for (i = 0; i < vdso_pages; i++)
 		vdso_pagelist[i + 1] = pfn_to_page(PHYS_PFN(__pa(&vdso_start)) + i);
 
-	/* Populate the special mapping structures */
-	vdso_spec[0] = (struct vm_special_mapping) {
-		.name	= "[vvar]",
-		.pages	= vdso_pagelist,
-	};
-
-	vdso_spec[1] = (struct vm_special_mapping) {
-		.name	= "[vdso]",
-		.pages	= &vdso_pagelist[1],
-	};
+	vdso_spec[0].pages = vdso_pagelist;
+	vdso_spec[1].pages = &vdso_pagelist[1];
 
 	return 0;
 }
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index c4284c4..59d44e7 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -30,7 +30,7 @@
 
 #include <asm/cacheflush.h>
 
-static int swiotlb __read_mostly;
+static int swiotlb __ro_after_init;
 
 static pgprot_t __get_dma_pgprot(unsigned long attrs, pgprot_t prot,
 				 bool coherent)
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index bbb7ee7..e8b81ea 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -55,8 +55,8 @@
  * executes, which assigns it its actual value. So use a default value
  * that cannot be mistaken for a real physical address.
  */
-s64 memstart_addr __read_mostly = -1;
-phys_addr_t arm64_dma_phys_limit __read_mostly;
+s64 memstart_addr __ro_after_init = -1;
+phys_addr_t arm64_dma_phys_limit __ro_after_init;
 
 #ifdef CONFIG_BLK_DEV_INITRD
 static int __init early_initrd(char *p)
diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 4989948..6029bed 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -46,7 +46,7 @@
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
 
-u64 kimage_voffset __read_mostly;
+u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);
 
 /*
-- 
2.8.1

[PATCH v2 3/4] arm64: kaslr: Fix incorrect placement of __initdata and __read_mostly

From: Jisheng Zhang <hidden>
Date: 2016-08-11 13:41:58

__initdata and __read_mostly should be placed after the variable name
for the variable to be placed in the intended section.

Signed-off-by: Jisheng Zhang <redacted>
---
 arch/arm64/kernel/kaslr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/kernel/kaslr.c b/arch/arm64/kernel/kaslr.c
index b054691..8ebabc4 100644
--- a/arch/arm64/kernel/kaslr.c
+++ b/arch/arm64/kernel/kaslr.c
@@ -20,8 +20,8 @@
 #include <asm/pgtable.h>
 #include <asm/sections.h>
 
-u64 __read_mostly module_alloc_base;
-u16 __initdata memstart_offset_seed;
+u64 module_alloc_base __read_mostly;
+u16 memstart_offset_seed __initdata;
 
 static __init u64 get_kaslr_seed(void *fdt)
 {
-- 
2.8.1

Re: [PATCH v2 4/4] arm64: apply __ro_after_init to some objects

From: Mark Rutland <mark.rutland@arm.com>
Date: 2016-08-11 15:06:26

Hi,

On Thu, Aug 11, 2016 at 09:36:49PM +0800, Jisheng Zhang wrote:
quoted hunk
These objects are set during initialization, thereafter are read only.

Previously I only want to mark vdso_pages, vdso_spec, vectors_page and
cpu_ops as __read_mostly from performance point of view. Then inspired
by Kees's patch[1] to apply more __ro_after_init for arm, I think it's
better to mark them as __ro_after_init. What's more, I find some more
objects are also read only after init. So apply __ro_after_init to all
of them.

This patch also removes global vdso_pagelist and tries to clean up
vdso_spec[] assignment code.

[1] http://www.spinics.net/lists/arm-kernel/msg523188.html

Signed-off-by: Jisheng Zhang <redacted>
---
 arch/arm64/kernel/cpu_ops.c |  2 +-
 arch/arm64/kernel/kaslr.c   |  2 +-
 arch/arm64/kernel/vdso.c    | 27 +++++++++++++--------------
 arch/arm64/mm/dma-mapping.c |  2 +-
 arch/arm64/mm/init.c        |  4 ++--
 arch/arm64/mm/mmu.c         |  2 +-
 6 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index c7cfb8f..6d32d1a 100644
--- a/arch/arm64/kernel/cpu_ops.c
+++ b/arch/arm64/kernel/cpu_ops.c
@@ -28,7 +28,7 @@ extern const struct cpu_operations smp_spin_table_ops;
 extern const struct cpu_operations acpi_parking_protocol_ops;
 extern const struct cpu_operations cpu_psci_ops;
 
-const struct cpu_operations *cpu_ops[NR_CPUS];
+const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init;
It looks like we need to include <linux/cache.h> for __ro_after_init.
While we seem to get that via some transitive include, that's not
guaranteed to remain the case, and it's not something we should rely
upon.

Please explicitly include <linux/cache.h> when __ro_after_init is added.
-	/* Populate the special mapping structures */
-	vdso_spec[0] = (struct vm_special_mapping) {
-		.name	= "[vvar]",
-		.pages	= vdso_pagelist,
-	};
-
-	vdso_spec[1] = (struct vm_special_mapping) {
-		.name	= "[vdso]",
-		.pages	= &vdso_pagelist[1],
-	};
+	vdso_spec[0].pages = vdso_pagelist;
+	vdso_spec[1].pages = &vdso_pagelist[1];
While the original code was also this way, could we please make these a
little more consistent? e.g.

	vdso_spec[0].pages = &vdso_pagelist[0];
	vdso_spec[1].pages = &vdso_pagelist[1];

That way it raises few false alarms for anyone looking at the code.

Otherwise, this patch looks largely fine.

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help