Prev: [PATCH][GIT PULL] tracing: Fix uninitialized variable of tracing/trace output
Next: viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5
From: Vivek Goyal on 9 Apr 2010 17:30
On Fri, Apr 09, 2010 at 03:49:57PM -0400, Jason Baron wrote:
> The jump label table is more optimal accessed if the entries are continguous.
> Sorting the table accomplishes this. Do the sort at build-time. Adds a '-j'
> option to 'modpost' which replaces the vmlinux, with a sorted jump label
> section vmlinux. I've tested this on x86 with relocatable and it works fine
> there as well. Note that I have not sorted the jump label table in modules.
> This is b/c the jump label names can be exported by the core kernel, and thus
> I don't have them available at buildtime. This could be solved by either
> finding the correct ones in the vmlinux, or by embedding the name of the jump
> label in the module tables (and not just a pointer), but the module tables
> tend to be smaller, and thus their is less value to this kind of change
> anyway. The kernel continues to do the sort, just in case, but at least for
> the vmlinux, this is just a verfication that the jump label table has
> already been sorted.
>
> Signed-off-by: Jason Baron <jbaron(a)redhat.com>
> ---

[ CCing Eric ]

[..]
> +static void swap_jump_label_entries(struct jump_entry *previous, struct jump_entry *next)
> +{
> + struct jump_entry tmp;
> +
> + tmp = *next;
> + *next = *previous;
> + *previous = tmp;
> +}
> +
> +static void sort_jump_label_table(struct elf_info *info, Elf_Ehdr *hdr)
> +{
> + int swapped = 0;
> + struct jump_entry *iter, *iter_next;
> + char *name, *next_name;
> + Elf_Shdr *sechdrs = info->sechdrs;
> + unsigned long jump_table, jump_table_end;
> + unsigned long jump_strings, jump_strings_addr;
> +
> + if ((info->jump_sec == 0) && (info->jump_strings_sec == 0))
> + return;
> +
> + jump_table = (unsigned long)hdr + sechdrs[info->jump_sec].sh_offset;
> + jump_table_end = jump_table + sechdrs[info->jump_sec].sh_size;
> + jump_strings = (unsigned long)hdr +
> + sechdrs[info->jump_strings_sec].sh_offset;
> + jump_strings_addr = sechdrs[info->jump_strings_sec].sh_addr;
> +
> + do {
> + swapped = 0;
> + iter = iter_next = (struct jump_entry *)jump_table;
> + iter_next++;
> + for (; iter_next < (struct jump_entry *)jump_table_end;
> + iter++, iter_next++) {
> + name = jump_strings + (iter->name - jump_strings_addr);
> + next_name = jump_strings +
> + (iter_next->name - jump_strings_addr);
> + if (strcmp(name, next_name) > 0) {
> + swap_jump_label_entries(iter, iter_next);
> + swapped = 1;


Jason,

As we were chatting about this, it looks like you are modifying vmlinux
section outside the knowledge of compiler. So theoritically associated
relocation section knowledge is no more valid and it can be a problem during
i386 relocatable kernels where we read the section's relocation inforamtion
and perform the relocations at runtime.

I know you have tested this on i386 and it works for you. I guess it works
because all the entries in the section are same and we apply same relocation
offset to all entries so even changing the order of entries is not impacting.

But conceptually, changing the vmlinux section outside knowledge of compiler
and assuming that we don't have to change the associated relocation section
probably is not the best thing.

I am not sure how to fix it. May be rely back on boot time sorting, or if
there is a way to relink sections after sorting etc. I just wanted to raise
a concern. May be other people (Eric, hpa) have ideas whether it is a valid
concern or not or how to handle it better.

Thanks
Vivek
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Roland McGrath on 9 Apr 2010 17:40
I think it just happens to come out harmless. The relocs.c extraction
stuff doesn't actually care about the exact address to be relocated, it
only needs to know that a relocated value sits at a given place in the
binary. The sorting rearranges the addresses in the text without
rearranging their corresponding relocs, but it's still the case that at
exactly each of those spots in text there is a relocated address. The
boot-time "relocation" is just a blind adjustment to all those spots,
without reference to the original relocation details. So it Just Works.
I'm not at all sure this is how we want things to be. It's rather
nonobvious and fragile if we change any of the related magic. But I think
it is entirely reliable in today's code that it will do the right thing.


Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: [PATCH][GIT PULL] tracing: Fix uninitialized variable of tracing/trace output
Next: viafb: Do not probe for LVDS/TMDS on OLPC XO-1.5