2009-07-27 18:34:05

by John Reiser

[permalink] [raw]
Subject: ftrace: __start_mcount_loc should be .init.rodata

__start_mcount_loc[] is unused after init, yet occupies RAM forever
as part of .rodata. 152kiB is typical on a 64-bit architecture. Instead,
__start_mcount_loc should be in the interval [__init_begin, __init_end)
so that the space is reclaimed after init.

__start_mcount_loc[] is generated during the load portion
of kernel build, and is used only by ftrace_init(). ftrace_init is declared
'__init' and is in .init.text, which is freed after init.
__start_mcount_loc is placed into .rodata by a call to MCOUNT_REC inside
the RO_DATA macro of include/asm-generic/vmlinux.lds.h. The array *is*
read-only, but more importantly it is not used after init. So the call to
MCOUNT_REC should be moved from RO_DATA to INIT_DATA.

This patch has been tested on x86_64 with CONFIG_DEBUG_PAGEALLOC=y
which verifies that the address range never is accessed after init.

[Please cc: me if replying only to the linux-kernel list.]

Signed off by: John Reiser <[email protected]>

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 6ad76bf..98b37cf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -91,7 +91,8 @@
#endif

#ifdef CONFIG_FTRACE_MCOUNT_RECORD
-#define MCOUNT_REC() VMLINUX_SYMBOL(__start_mcount_loc) = .; \
+#define MCOUNT_REC() . = ALIGN(8); \
+ VMLINUX_SYMBOL(__start_mcount_loc) = .; \
*(__mcount_loc) \
VMLINUX_SYMBOL(__stop_mcount_loc) = .;
#else
@@ -331,7 +332,6 @@
/* __*init sections */ \
__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
*(.ref.rodata) \
- MCOUNT_REC() \
DEV_KEEP(init.rodata) \
DEV_KEEP(exit.rodata) \
CPU_KEEP(init.rodata) \
@@ -455,6 +455,7 @@
MEM_DISCARD(init.data) \
KERNEL_CTORS() \
*(.init.rodata) \
+ MCOUNT_REC() \
DEV_DISCARD(init.rodata) \
CPU_DISCARD(init.rodata) \
MEM_DISCARD(init.rodata)

--


2009-07-27 20:44:28

by Steven Rostedt

[permalink] [raw]
Subject: Re: ftrace: __start_mcount_loc should be .init.rodata




On Mon, 27 Jul 2009, John Reiser wrote:

> __start_mcount_loc[] is unused after init, yet occupies RAM forever
> as part of .rodata. 152kiB is typical on a 64-bit architecture. Instead,
> __start_mcount_loc should be in the interval [__init_begin, __init_end)
> so that the space is reclaimed after init.

Are you sure about that?

>
> __start_mcount_loc[] is generated during the load portion
> of kernel build, and is used only by ftrace_init(). ftrace_init is declared
> '__init' and is in .init.text, which is freed after init.
> __start_mcount_loc is placed into .rodata by a call to MCOUNT_REC inside
> the RO_DATA macro of include/asm-generic/vmlinux.lds.h. The array *is*
> read-only, but more importantly it is not used after init. So the call to
> MCOUNT_REC should be moved from RO_DATA to INIT_DATA.
>
> This patch has been tested on x86_64 with CONFIG_DEBUG_PAGEALLOC=y
> which verifies that the address range never is accessed after init.
>
> [Please cc: me if replying only to the linux-kernel list.]
>
> Signed off by: John Reiser <[email protected]>
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 6ad76bf..98b37cf 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -91,7 +91,8 @@
> #endif
>
> #ifdef CONFIG_FTRACE_MCOUNT_RECORD
> -#define MCOUNT_REC() VMLINUX_SYMBOL(__start_mcount_loc) = .; \
> +#define MCOUNT_REC() . = ALIGN(8); \
> + VMLINUX_SYMBOL(__start_mcount_loc) = .; \
> *(__mcount_loc) \
> VMLINUX_SYMBOL(__stop_mcount_loc) = .;
> #else
> @@ -331,7 +332,6 @@
> /* __*init sections */ \
> __init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \

Isn't this in __init_rodata? Doesn't this get removed too?

I may be confused by all the uses of "init" in this file.

-- Steve


> *(.ref.rodata) \
> - MCOUNT_REC() \
> DEV_KEEP(init.rodata) \
> DEV_KEEP(exit.rodata) \
> CPU_KEEP(init.rodata) \
> @@ -455,6 +455,7 @@
> MEM_DISCARD(init.data) \
> KERNEL_CTORS() \
> *(.init.rodata) \
> + MCOUNT_REC() \
> DEV_DISCARD(init.rodata) \
> CPU_DISCARD(init.rodata) \
> MEM_DISCARD(init.rodata)
>
> --
>
>

2009-07-28 00:11:18

by John Reiser

[permalink] [raw]
Subject: Re: ftrace: __start_mcount_loc should be .init.rodata

Steven Rostedt wrote:
>
>
> On Mon, 27 Jul 2009, John Reiser wrote:
>
>> __start_mcount_loc[] is unused after init, yet occupies RAM forever
>> as part of .rodata. 152kiB is typical on a 64-bit architecture. Instead,
>> __start_mcount_loc should be in the interval [__init_begin, __init_end)
>> so that the space is reclaimed after init.
>
> Are you sure about that?

Data follows from a direct A<->B comparison.

My .config is same as Fedora Project, except for CONFIG_DEBUG_PAGEALLOC=y.
----- /var/log/messages
ftrace: allocating 18889 entries ...
# 18889 * 8 = 151112 bytes (147kiB) for __start_mcount_loc[]
on 64-bit architecture
-----

With MCOUNT_REC() in original RO_DATA in include/asm-generic/vmlinux.lds.h:
===================================
----- /var/log/messages
kernel: Freeing initrd memory: 3134k freed
kernel: debug: unmapping init memory ffffffff8163d000..ffffffff81783000
## The only message about unmapping; length is 0x146000 or 1335296 bytes
kernel: Write protecting the kernel read-only data: 1844k ## 148kiB more than later
-----

----- System.map
ffffffff8155c850 R __start_mcount_loc # below unmapped range
ffffffff81581698 R __stop_mcount_loc
-----

----- "readelf --sections vmlinux" has these "\.init" strings:
[12] __init_rodata PROGBITS ffffffff8155c850 0075c850 # _below_ unmapped range
[15] .init.rodata PROGBITS ffffffff815f01c0 007f01c0 # _below_ unmapped range
[25] .data.init_task PROGBITS ffffffff81612000 00a12000
[28] .init.text PROGBITS ffffffff8163d000 00a3d000 # low end of unmapped range
[29] .init.data PROGBITS ffffffff81675620 00a75620
[30] .init.setup PROGBITS ffffffff8175d980 00b5d980
[31] .initcall.init PROGBITS ffffffff8175eca0 00b5eca0
[33] .x86_cpu_dev.init PROGBITS ffffffff8175fa00 00b5fa00
[39] .init.ramfs PROGBITS ffffffff81776000 00b76000 # still in unmapped range
01 .data .init.rodata .data.cacheline_aligned .data.read_mostly
## Note that .init.rodata is in segment 1, with .data
03 .data.init_task .data.page_aligned .smp_locks .init.text .init.data .init.setup .initcall.init .con_initcall.init .x86_cpu_dev.init .secu
rity_initcall.init .parainstructions .altinstructions .altinstr_replacement .exit.text .init.ramfs .data.percpu .data_nosave .bss
## Note that .init.text and .init.data are in segment 3
-----


With MCOUNT_REC() moved to INIT_DATA as patched to include/asm-generic/vmlinux.lds.h:
==================================
----- /var/log/messages:
kernel: Freeing initrd memory: 3134k freed
kernel: debug: unmapping init memory ffffffff81619000..ffffffff81784000
## The only message about unmapping; length is 0x16b000 or 1486848 bytes
kernel: Write protecting the kernel read-only data: 1696k ## 148kiB less than before
-----

----- System.map
ffffffff816a28d0 T __start_mcount_loc # within unmapped range
ffffffff816c7718 T __stop_mcount_loc
-----

----- "readelf --sections vmlinux" has these "\.init" strings:
[14] .init.rodata PROGBITS ffffffff815cb1c0 007cb1c0 # _below_ unmapped range
[24] .data.init_task PROGBITS ffffffff815ee000 009ee000
[27] .init.text PROGBITS ffffffff81619000 00a19000 # low end of unmapped range
[28] .init.data PROGBITS ffffffff81651620 00a51620
[29] .init.setup PROGBITS ffffffff8175e7c0 00b5e7c0
[30] .initcall.init PROGBITS ffffffff8175fae0 00b5fae0
[32] .x86_cpu_dev.init PROGBITS ffffffff81760840 00b60840
[38] .init.ramfs PROGBITS ffffffff81777000 00b77000 # still in unmapped range
01 .data .init.rodata .data.cacheline_aligned .data.read_mostly
03 .data.init_task .data.page_aligned .smp_locks .init.text .init.data .init.setup .initcall.init .con_initcall.init .x86_cpu_dev.init .secu
rity_initcall.init .parainstructions .altinstructions .altinstr_replacement .exit.text .init.ramfs .data.percpu .data_nosave .bss
-----

Perusing the source:
arch/x86/mm/init_64.c: free_init_pages() called from
./arch/x86/kernel/alternative.c: free_init_pages("SMP alternatives",
./arch/x86/mm/init_64.c: free_init_pages("unused kernel memory",
./arch/x86/mm/init_64.c: free_init_pages("initrd memory", start, end);
All other calls also are architecture-specific and in other architectures,
and also are similar to these.


>> @@ -331,7 +332,6 @@
>> /* __*init sections */ \
>> __init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
>
> Isn't this in __init_rodata? Doesn't this get removed too?

In my vmlinux it is _not_ removed; it is with .rodata, not .init.rodata
(and not .init.data), and is outside (below) the removed range.

--

2009-09-13 15:03:21

by John Reiser

[permalink] [raw]
Subject: [tip:tracing/core] ftrace: __start_mcount_loc should be .init.rodata

Commit-ID: 4b3b4c5e64ce26612646867ee354373620063534
Gitweb: http://git.kernel.org/tip/4b3b4c5e64ce26612646867ee354373620063534
Author: John Reiser <[email protected]>
AuthorDate: Mon, 27 Jul 2009 11:23:50 -0700
Committer: Steven Rostedt <[email protected]>
CommitDate: Sat, 12 Sep 2009 21:57:29 -0400

ftrace: __start_mcount_loc should be .init.rodata

__start_mcount_loc[] is unused after init, yet occupies RAM forever
as part of .rodata. 152kiB is typical on a 64-bit architecture. Instead,
__start_mcount_loc should be in the interval [__init_begin, __init_end)
so that the space is reclaimed after init.

__start_mcount_loc[] is generated during the load portion
of kernel build, and is used only by ftrace_init(). ftrace_init is declared
'__init' and is in .init.text, which is freed after init.
__start_mcount_loc is placed into .rodata by a call to MCOUNT_REC inside
the RO_DATA macro of include/asm-generic/vmlinux.lds.h. The array *is*
read-only, but more importantly it is not used after init. So the call to
MCOUNT_REC should be moved from RO_DATA to INIT_DATA.

This patch has been tested on x86_64 with CONFIG_DEBUG_PAGEALLOC=y
which verifies that the address range never is accessed after init.

Signed-off-by: John Reiser <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>


---
include/asm-generic/vmlinux.lds.h | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 6ad76bf..98b37cf 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -91,7 +91,8 @@
#endif

#ifdef CONFIG_FTRACE_MCOUNT_RECORD
-#define MCOUNT_REC() VMLINUX_SYMBOL(__start_mcount_loc) = .; \
+#define MCOUNT_REC() . = ALIGN(8); \
+ VMLINUX_SYMBOL(__start_mcount_loc) = .; \
*(__mcount_loc) \
VMLINUX_SYMBOL(__stop_mcount_loc) = .;
#else
@@ -331,7 +332,6 @@
/* __*init sections */ \
__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) { \
*(.ref.rodata) \
- MCOUNT_REC() \
DEV_KEEP(init.rodata) \
DEV_KEEP(exit.rodata) \
CPU_KEEP(init.rodata) \
@@ -455,6 +455,7 @@
MEM_DISCARD(init.data) \
KERNEL_CTORS() \
*(.init.rodata) \
+ MCOUNT_REC() \
DEV_DISCARD(init.rodata) \
CPU_DISCARD(init.rodata) \
MEM_DISCARD(init.rodata)