On Tue, Apr 17, 2007 at 03:44:53PM +1000, David Gibson wrote:
> 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.
Duh, which is gone in the last version. Sorry, I don't seem to be
very with it today.
--
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
