Thread (6 messages) flat view 6 messages, 2 authors, 2016-08-11

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

From: mark.rutland@arm.com (Mark Rutland)
Date: 2016-08-11 15:06:26
Also in: lkml

Hi,

On Thu, Aug 11, 2016 at 09:36:49PM +0800, Jisheng Zhang wrote:
quoted hunk ↗ jump to 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