Re: [patch 1/6] Use EXTRA_RWDATA in architectures
From: Mathieu Desnoyers <hidden>
Date: 2007-05-30 13:06:26
Also in:
lkml
Hi Sam, Thanks for the comments, will integrate in my next post. I noticed that this version was against 2.6.21-mm2, not what I thought it was (2.6.22-rc2-mm1). Mathieu * Sam Ravnborg (sam@ravnborg.org) wrote:
Hi Mathieuquoted
Adds a place to declare rw data that will not be far from the .data content, therefore limiting the impact on cache of data declared in sections part of the EXTRA_RWDATA.No comments on the aim of the path - but a few implementation comments. I'm glad to see asm-generic/vmlinux.lds.h used - thanks. The general naming is USAGE_{DATA|TEXT} To keep with this albait vague then established naming scheme please consider using: EXTRARW_DATAquoted
Index: linux-2.6-lttng/arch/h8300/kernel/vmlinux.lds.S ===================================================================--- linux-2.6-lttng.orig/arch/h8300/kernel/vmlinux.lds.S 2007-05-15 18:41:05.000000000 -0400 +++ linux-2.6-lttng/arch/h8300/kernel/vmlinux.lds.S 2007-05-15 18:44:20.000000000 -0400@@ -105,7 +105,9 @@ . = ALIGN(0x4) ; *(.data) . = ALIGN(0x4) ; - *(.data.*) + *(.data.*) + . = ALIGN(0x4) ; + EXTRA_RWDATAThe usage of ALIGN seems a bit arbitary. If there is a requirement to align data for EXTRA_RWDATA usage then pass alignment requirment as parameter or do alignment in the define.quoted
Index: linux-2.6-lttng/arch/ia64/kernel/vmlinux.lds.S ===================================================================--- linux-2.6-lttng.orig/arch/ia64/kernel/vmlinux.lds.S 2007-05-15 18:41:05.000000000 -0400 +++ linux-2.6-lttng/arch/ia64/kernel/vmlinux.lds.S 2007-05-15 18:44:20.000000000 -0400@@ -214,7 +214,7 @@ data : { } :data .data : AT(ADDR(.data) - LOAD_OFFSET) - { *(.data) *(.data1) *(.gnu.linkonce.d*) CONSTRUCTORS } + { *(.data) *(.data1) *(.gnu.linkonce.d*) EXTRA_RWDATA CONSTRUCTORS }Try to think of linker scripts a C-files. In C you would never do like this: - int i; struct type foo; + int i; struct type foo; struct type2 bar; You would certainly place the new variable on a new line and maybe fix the bad code-style in another patch. My point is that you should not continue the bad-taste style used here but just add EXTRA_RWDATA on a single line. If no-one beats me I plan to go through all linker scrip files and add some sanity with respect to indent style etc. so they start to follow C-style for indent.quoted
Index: linux-2.6-lttng/arch/xtensa/kernel/vmlinux.lds.S ===================================================================--- linux-2.6-lttng.orig/arch/xtensa/kernel/vmlinux.lds.S 2007-05-15 18:41:05.000000000 -0400 +++ linux-2.6-lttng/arch/xtensa/kernel/vmlinux.lds.S 2007-05-15 18:44:20.000000000 -0400@@ -144,7 +144,7 @@ _fdata = .; .data : { - *(.data) CONSTRUCTORS + *(.data) EXTRA_RWDATA CONSTRUCTORS . = ALIGN(XCHAL_ICACHE_LINESIZE); *(.data.cacheline_aligned)again..quoted
} Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h ===================================================================--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-05-15 18:44:11.000000000 -0400 +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-05-15 18:44:20.000000000 -0400@@ -143,6 +143,8 @@ \ . = ALIGN(4096); +#define EXTRA_RWDATA +Please add comment that describe the purpose of EXTRA_RWDATA - verbatim copy from changlog is almost enough. Sam
-- Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68