On Fri, 1 May 2009, Sam Ravnborg wrote:
> This is the way I want to go where we have more complete
> definitions in the shared file and we try to keep the arch
> linker scripts to the arch specifc stuff.
I like the general look of this. Indeed, I was planning to work on
something like this as a follow-on to the linker script cleanup work I've
done so far.
Some comments on the details are below.
> +#define PAGE_ALIGNED_DATA(page_align) \
> + . = ALIGN((page_align)); \
> + *(.data.page_aligned)
Why does this need an argument? You should be able to just align to
PAGE_SIZE.
Also, I'm not sure what you're trying to do with the double parentheses
here (it also appears inconsistently in several other places in this
patch).
> +/* use 0 as page_align if page_aligned data is not used */
> +#define RW_DATA(page_align, readmostly_align, cache_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)) \
> + }
I think there are several architectures that have some other stuff in
their .data output section (e.g. powerpc, frv, ia64), so we won't be able
to use this on all architectures.
Perhaps we want an intermediate macro between DATA_DATA and RW_DATA that
is just the contents of .data here that those architectuere can use?
> +#define INITDATA(initsetup_align) \
> + .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) { \
> + INIT_DATA \
> + . = ALIGN(initsetup_align); \
> + VMLINUX_SYMBOL(__setup_start) = .; \
> + *(.init.setup) \
[...]
> +#define INITDATA(initdata_align) \
> + . = ALIGN((initdata_align)); \
> + .init.data : AT(ADDR(.init.data) - LOAD_OFFSET) { \
> + INIT_DATA \
> + }
You define two different macros called INITDATA; I'm not sure what the
intention is here.
> +#define INIT_TASK \
> + *(.data.init_task)
> +
> +#define INIT_TASK_DATA(align)
> + . = ALIGN((align)); \
> + .data.init_task : { \
> + INIT_TASK \
> + }
> +
You mentioned elsewhere you thought .data.init_task needs to be its own
output section rather than part of the .data output section; why is that?
There are several architectures on which it is part of the .data output
section (e.g. sh, um, avr32).
Also, I think it is possible that INIT_TASK_DATA could just align to
THREAD_SIZE rather than taking an argument. While working on my patches
for this I noticed there were only a couple of architectures where the
alignment wasn't THREAD_SIZE (or a value equal to THREAD_SIZE was used).
One exception was parisc, where the aligment is 16384 and THREAD_SIZE is
always at least that but could be bigger in some configs where PAGE_SIZE
is bigger. I'm not sure whether this one exception is a bug.
-Tim Abbott
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
