Messages in this thread | | | Date | Mon, 4 May 2009 12:32:51 -0400 (EDT) | From | Tim Abbott <> | Subject | Re: [PATCH v2 0/6] macros for section name cleanup |
| |
On Mon, 4 May 2009, Sam Ravnborg wrote:
> I'm especially fond of the "minimal" linker script > contained in the beginning of the file.
I like this a lot too.
> + * /DISCARD/ : { > + * EXIT_TEXT > + * EXIT_DATA > + * *(.exitcall.exit) > + * }
You may want to create an EXIT_CALL macro, to remove the explicit mention of .exitcall.exit, which is the only input section referenced directly in the example script.
> +#define READ_MOSTLY_DATA(align) \ > + . = ALIGN(align); \ > + *(.data.read_mostly)
Should READ_MOSTLY_DATA have an ALIGN(align) at the end as well? If the goal is to isolate READ_MOSTLY_DATA from other data at the given alignment level, this would seem appropriate. Something similar may apply to CACHELINE_ALIGNED_DATA as well.
> +/* > + * bss (Block started by Symbol) - uninitialized data > + * zeroed during startup > + */ > +#define SBSS \ > + .sbss : AT(ADDR(.sbss) - LOAD_OFFSET) { \ > + *(.sbss) \ > + *(.scommon) \ > + }
I notice that s390 and powerpc reference a ".dynsbss"; does that belong here?
> +#ifdef CONFIG_BLK_DEV_INITRD > +#define INIT_RAM_FS \ > + . = ALIGN(PAGE_SIZE); \ > + VMLINUX_SYMBOL(__initramfs_start) = .; \ > + *(.init.ramfs) \ > + VMLINUX_SYMBOL(__initramfs_end) = .; > +#else > +#define INITRAMFS > +#endif
I think you want the #else clause to be INIT_RAM_FS here.
> +/* > + * Definition of the high level *_SECTION macros > + * They will fit only a subset of the architectures > + */
Do we want a TEXT_SECTION macro here as well that looks like what appears above in the example script?
> +#define RW_DATA_SECTION(page_align, readmostly_align, cache_align, inittask_align) \ > + . = ALIGN(PAGE_SIZE); \ > + .data : AT(ADDR(.data) - LOAD_OFFSET) { \ > + DATA_DATA \ > + CONSTRUCTORS \ > + NOSAVE_DATA \ > + PAGE_ALIGNED_DATA(page_align) \ > + READMOSTLY_DATA(readmostly_align) \ > + CACHELINE_ALIGNED_DATA(cache_align) \ > + INIT_TASK(inittask_align) \ > + }
How did you pick the order of the sections here? I would think that to pack the .data section efficiently, you'd want to sort by alignment requirements so that e.g. all the at-least-page aligned sections are adjacent (INIT_TASK and the page-aligned sections are separated by some much smaller aligments here). So it would like either the following or the following reversed:
. = ALIGN(PAGE_SIZE); \ .data : AT(ADDR(.data) - LOAD_OFFSET) { \ INIT_TASK(inittask_align) \ NOSAVE_DATA \ PAGE_ALIGNED_DATA(page_align) \ READMOSTLY_DATA(readmostly_align) \ CACHELINE_ALIGNED_DATA(cache_align) \ DATA_DATA \ CONSTRUCTORS \ }
-Tim Abbott
| |