Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1459370 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] arm64: put objects into proper sections Jisheng Zhang <jszhang@marvell.com> - 2016-08-10 20:20 +0200
    [PATCH 3/3] arm64: kernel: declare cpu_ops __read_mostly Jisheng Zhang <jszhang@marvell.com> - 2016-08-10 20:30 +0200
    Re: [PATCH 0/3] arm64: put objects into proper sections Kees Cook <keescook@chromium.org> - 2016-08-10 20:50 +0200
    Re: [PATCH 0/3] arm64: put objects into proper sections Jisheng Zhang <jszhang@marvell.com> - 2016-08-10 22:30 +0200
    [PATCH 1/3] arm64: vdso: add __init section marker to alloc_vectors_page Jisheng Zhang <jszhang@marvell.com> - 2016-08-10 22:30 +0200
    [PATCH 2/3] arm64: vdso: put read only/mostly objects into proper sections Jisheng Zhang <jszhang@marvell.com> - 2016-08-10 22:40 +0200

#1459370 — [PATCH 0/3] arm64: put objects into proper sections

This series tries to put objects into proper sections to avoid
pontential unnecessary DDR traffics:

patch1 puts alloc_vectors_page() into .init section. This is a clean up
patch.

patch2 puts vdso_pages, vdso_spec and vectors_page into read_mostly
section, since they are read mostly in hot path.

patch3 puts cpu_ops into read_mostly section, since it's read mostly
int hot path such as arm_cpuidle_suspend().


Before this series:

ffff000008d660f0 b undef_lock
ffff000008d660f8 b vectors_page
ffff000008d66100 b vdso_pages
ffff000008d66108 b vdso_spec
ffff000008d66148 B cpu_ops
ffff000008d66348 b patch_lock

After this series:

ffff000008ca5b08 D static_key_initialized
ffff000008ca5b10 d vectors_page
ffff000008ca5b18 d vdso_pages
ffff000008ca5b20 d vdso_spec
ffff000008ca5b60 D cpu_ops
ffff000008ca5d60 D elf_hwcap
ffff000008ca5d68 D compat_elf_hwcap
ffff000008ca5d6c D compat_elf_hwcap2

Although there's no obvious issue with current code, because undef_lock and
patch_lock is rarely used, but the situation may change in the future, so
let's move them into proper sections now.

Jisheng Zhang (3):
  arm64: vdso: add __init section marker to alloc_vectors_page
  arm64: vdso: put read only/mostly objects into proper sections
  arm64: kernel: declare cpu_ops __read_mostly

 arch/arm64/kernel/cpu_ops.c |  2 +-
 arch/arm64/kernel/vdso.c    | 31 +++++++++++++++----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

-- 
2.8.1

[toc] | [next] | [standalone]

#1459420 — [PATCH 3/3] arm64: kernel: declare cpu_ops __read_mostly

cpu_ops is initialized once by cpu_read_ops(), and thereafter is mostly
read in hot path, such as arm_cpuidle_suspend().

The fact that it is mostly read and not written to makes it candidates
for __read_mostly declarations.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 arch/arm64/kernel/cpu_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpu_ops.c b/arch/arm64/kernel/cpu_ops.c
index c7cfb8f..c1e4802 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] __read_mostly;
 
 static const struct cpu_operations *dt_supported_cpu_ops[] __initconst = {
 	&smp_spin_table_ops,
-- 
2.8.1

[toc] | [prev] | [next] | [standalone]

#1459492

On Wed, Aug 10, 2016 at 3:05 AM, Jisheng Zhang <jszhang@marvell.com> wrote:
> + Kees,
>
> On Wed, 10 Aug 2016 15:19:16 +0800 Jisheng Zhang wrote:
>
>> This series tries to put objects into proper sections to avoid
>> pontential unnecessary DDR traffics:
>>
>> patch1 puts alloc_vectors_page() into .init section. This is a clean up
>> patch.
>>
>> patch2 puts vdso_pages, vdso_spec and vectors_page into read_mostly
>> section, since they are read mostly in hot path.
>>
>> patch3 puts cpu_ops into read_mostly section, since it's read mostly
>> int hot path such as arm_cpuidle_suspend().
>
> Is it better to mark them as __ro_after_init, suggestions are appreciated.

If something is actually never updated after __init, yes, please use
__ro_after_init.

-Kees

>
> Thanks,
> Jisheng
>
>>
>>
>> Before this series:
>>
>> ffff000008d660f0 b undef_lock
>> ffff000008d660f8 b vectors_page
>> ffff000008d66100 b vdso_pages
>> ffff000008d66108 b vdso_spec
>> ffff000008d66148 B cpu_ops
>> ffff000008d66348 b patch_lock
>>
>> After this series:
>>
>> ffff000008ca5b08 D static_key_initialized
>> ffff000008ca5b10 d vectors_page
>> ffff000008ca5b18 d vdso_pages
>> ffff000008ca5b20 d vdso_spec
>> ffff000008ca5b60 D cpu_ops
>> ffff000008ca5d60 D elf_hwcap
>> ffff000008ca5d68 D compat_elf_hwcap
>> ffff000008ca5d6c D compat_elf_hwcap2
>>
>> Although there's no obvious issue with current code, because undef_lock and
>> patch_lock is rarely used, but the situation may change in the future, so
>> let's move them into proper sections now.
>>
>> Jisheng Zhang (3):
>>   arm64: vdso: add __init section marker to alloc_vectors_page
>>   arm64: vdso: put read only/mostly objects into proper sections
>>   arm64: kernel: declare cpu_ops __read_mostly
>>
>>  arch/arm64/kernel/cpu_ops.c |  2 +-
>>  arch/arm64/kernel/vdso.c    | 31 +++++++++++++++----------------
>>  2 files changed, 16 insertions(+), 17 deletions(-)
>>
>



-- 
Kees Cook
Nexus Security

[toc] | [prev] | [next] | [standalone]

#1459840

+ Kees,

On Wed, 10 Aug 2016 15:19:16 +0800 Jisheng Zhang wrote:

> This series tries to put objects into proper sections to avoid
> pontential unnecessary DDR traffics:
> 
> patch1 puts alloc_vectors_page() into .init section. This is a clean up
> patch.
> 
> patch2 puts vdso_pages, vdso_spec and vectors_page into read_mostly
> section, since they are read mostly in hot path.
> 
> patch3 puts cpu_ops into read_mostly section, since it's read mostly
> int hot path such as arm_cpuidle_suspend().

Is it better to mark them as __ro_after_init, suggestions are appreciated.

Thanks,
Jisheng

> 
> 
> Before this series:
> 
> ffff000008d660f0 b undef_lock
> ffff000008d660f8 b vectors_page
> ffff000008d66100 b vdso_pages
> ffff000008d66108 b vdso_spec
> ffff000008d66148 B cpu_ops
> ffff000008d66348 b patch_lock
> 
> After this series:
> 
> ffff000008ca5b08 D static_key_initialized
> ffff000008ca5b10 d vectors_page
> ffff000008ca5b18 d vdso_pages
> ffff000008ca5b20 d vdso_spec
> ffff000008ca5b60 D cpu_ops
> ffff000008ca5d60 D elf_hwcap
> ffff000008ca5d68 D compat_elf_hwcap
> ffff000008ca5d6c D compat_elf_hwcap2
> 
> Although there's no obvious issue with current code, because undef_lock and
> patch_lock is rarely used, but the situation may change in the future, so
> let's move them into proper sections now.
> 
> Jisheng Zhang (3):
>   arm64: vdso: add __init section marker to alloc_vectors_page
>   arm64: vdso: put read only/mostly objects into proper sections
>   arm64: kernel: declare cpu_ops __read_mostly
> 
>  arch/arm64/kernel/cpu_ops.c |  2 +-
>  arch/arm64/kernel/vdso.c    | 31 +++++++++++++++----------------
>  2 files changed, 16 insertions(+), 17 deletions(-)
> 

[toc] | [prev] | [next] | [standalone]

#1459849 — [PATCH 1/3] arm64: vdso: add __init section marker to alloc_vectors_page

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

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 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

[toc] | [prev] | [next] | [standalone]

#1459868 — [PATCH 2/3] arm64: vdso: put read only/mostly objects into proper sections

vdso_pages and vdso_spec are initialized by vdso_init(), thereafter are
mostly read during vdso special mapping handling.

vectors_page is initialized by alloc_vectors_page(), thereafter is
mostly read during aarch32 vectors special mapping handling.

The fact that they are mostly read and not written to makes them
candidates for __read_mostly declarations.

The vm_special_mapping spec is never modified, so mark it as const.

This patch also removes global vdso_pagelist.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 arch/arm64/kernel/vdso.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
index e320e8f..e62da76 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 __read_mostly;
 
 /*
  * 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] __read_mostly;
 
 static int __init alloc_vectors_page(void)
 {
@@ -88,7 +87,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,
 
@@ -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] __read_mostly = {
+	{
+		.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;
 }
-- 
2.8.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web