Re: [PATCH v2 0/6] macros for section name cleanup

From: Tim Abbott
Date: Mon May 04 2009 - 12:33:26 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/