On Mon, Apr 16, 2007 at 10:23:39PM -0700, Nishanth Aravamudan wrote:
> On 17.04.2007 [11:16:17 +1000], David Gibson wrote:
> > On Tue, Apr 03, 2007 at 07:31:09AM -0700, Nishanth Aravamudan wrote:
> > > On 03.04.2007 [09:01:34 -0500], Adam Litke wrote:
> > > > On Mon, 2007-04-02 at 18:27 -0700, Nishanth Aravamudan wrote:
> > > > > ppc: rework plt detection
> > > > >
> > > > > We currently emit LONG(0) into the .plt section of relinked ppc
> > > > > binaries
> > > > > to make it appear in the filesz (on-disk) portion of the data/bss
> > > > > segment. This is a problem, however, for powerpc64, where the ABI
> > > > > specifies that the .plt section is NOBITS (x86 and x86_64 mark the
> > > > > .plt
> > > > > PROGBITS). Given that the program is being relinked to begin with,
> > > > > however, it seems logical to simply add a tag, similar to
> > > > > __executable_start, to make finding the "libhuge" filesz easy to find.
> > > > > Do exactly this, via __libhuge_filesz, and skip the extracopy
> > > > > detection
> > > > > if this symbol is defined. Tested on powerpc64.
> > > >
> > > > Looks good to me...
> > > >
> > > > What do you think about adding some comments into the linker scripts
> > > > here so it will be easy to remember what this is for later?
> > >
> > > Updated patch below:
> > >
> > >
> > > ppc: rework plt detection
> > >
> > > We currently emit LONG(0) into the .plt section to make it appear in the
> > > filesz (on-disk) portion of the data/bss segment. This is a problem,
> > > however, for powerpc64, where the ABI specifies that the .plt section is
> > > NOBITS (x86 and x86_64 mark the .plt PROGBITS). Given that the program
> > > is being relinked to begin with, however, it seems logical to simply add
> > > a tag, similar to __executable_start, to make finding the "libhuge"
> > > filesz easy to find. Do exactly this, via __libhuge_filesz, and skip the
> > > extracopy detection if this symbol is defined. Tested on powerpc64.
> >
> > Nice idea, definitely neater than the LONG(0) thing. However, I don't
> > see how it safely lets up bypass extracopy detection - the stdin,
> > stdout variables that libc initializes will still lie in the BSS
> > section after the __libhuge_filesz symbol.
>
> Yep, that was a bug waiting to happen...(and does on at least one
> benchmark).
>
> > > diff --git a/elflink.c b/elflink.c
> > > index 5058248..00b403a 100644
> > > --- a/elflink.c
> > > +++ b/elflink.c
> > > @@ -414,6 +414,7 @@ static void get_extracopy(struct seg_info *seg,
> > > Elf_Phdr *phdr, int phnum)
> > > int ret, numsyms, found_sym = 0;
> > > void *start, *end, *start_orig, *end_orig;
> > > void *sym_start, *sym_end;
> > > + extern void __libhuge_filesz __attribute__((weak));
> > >
> > > end_orig = seg->vaddr + seg->memsz;
> > > start_orig = seg->vaddr + seg->filesz;
> > > @@ -422,6 +423,13 @@ static void get_extracopy(struct seg_info *seg,
> > > Elf_Phdr *phdr, int phnum)
> > > if (!minimal_copy)
> > > goto bail2;
> > >
> > > + if (&__libhuge_filesz) {
> > > + found_sym = 1;
> > > + start = start_orig;
> > > + end = &__libhuge_filesz;
> > > + goto found;
> >
> > Hrm... I don' think this is a reasonable use of a goto. I'd prefer to
> > see the other logic in an else branch here.
>
> See the follow-up patches, the check is moved after the extracopy
> algorithm.
Yes, sorry, saw that after. I still don't like the goto, though.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Libhugetlbfs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
