[PATCH v6 4/5] elf: Don't cache symbol nor relocation tables with mmap
H.J. Lu
hjl.tools@gmail.com
Sun Mar 10 01:11:13 GMT 2024
On Sat, Mar 9, 2024 at 4:44 PM Sam James <sam@gentoo.org> wrote:
>
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
> > During a "-j 8" LLVM 17 debug build on a machine with 32GB RAM and 16GB
> > swap, ld was killed by kernel because of out of memory:
> >
> > [79437.949336] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0,global_oom,task_memcg=/user.slice/user-1000.slice/session-9.scope,task=ld,pid=797431,uid=1000
> > [79437.949349] Out of memory: Killed process 797431 (ld) total-vm:9219600kB, anon-rss:6558156kB, file-rss:1792kB, shmem-rss:0kB, UID:1000 pgtables:17552kB oom_score_adj:0
> >
> > Don't cache symbol nor relocation tables if they are mapped in. Data to
> > link the 3.5GB clang executable in LLVM 17 debug build on Linux/x86-64
> > with 32GB RAM is:
> >
> > stdio mmap improvement
> > user 86.73 87.02 -0.3%
> > system 9.55 9.21 3.6%
> > total 100.40 97.66 0.7%
> > maximum set(GB) 17.34 13.14 24%
> > page faults 4047667 3042877 25%
> >
> > and data to link the 275M cc1plus executable in GCC 14 stage 1 build is:
> >
> > user 5.41 5.44 -0.5%
> > system 0.80 0.76 5%
> > total 6.25 6.26 -0.2%
> > maximum set(MB) 1323 968 27%
> > page faults 323451 236371 27%
> >
> > These improve the overall system performance for parallel build by
> > reducing memory usage and page faults.
> >
> > Also rename _bfd_link_keep_memory to _bfd_elf_link_keep_memory. Since
> > the --no-keep-memory linker option causes:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=31458
> >
> > this is opt-in by each backend. Add the --keep-memory linker option to
> > always cache the symbol and relocation tables to mitigate --no-keep-memory
> > bugs like:
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=31466
> >
> > bfd/
> >
> > * elf-bfd.h (_bfd_elf_link_keep_memory): New.
> > * elf32-i386.c (elf_i386_scan_relocs): Replace
> > _bfd_link_keep_memory with _bfd_elf_link_keep_memory.
> > * elf64-x86-64.c (elf_x86_64_scan_relocs): Likewise.
> > * elflink.c (_bfd_elf_link_iterate_on_relocs): Likewise.
> > (elf_link_add_object_symbols): Likewise.
> > (init_reloc_cookie): Likewise.
> > (init_reloc_cookie_rels): Likewise.
> > (_bfd_elf_link_keep_memory): New.
> > * libbfd-in.h (_bfd_link_keep_memory): Removed.
> > * linker.c (_bfd_link_keep_memory): Likewise.
> > * libbfd.h: Regenerated.
> >
> > include/
> >
> > * bfdlink.h (bfd_link_info): Add keep_memory_is_set.
> >
> > ld/
> >
> > * NEWS: Mention --keep-memory.
> > * ld.texi: Document --keep-memory.
> > * ldlex.h (option_values): Add OPTION_KEEP_MEMORY.
> > * lexsup.c (ld_options): Add --keep-memory.
> > (parse_args): Handle OPTION_KEEP_MEMORY and set
> > link_info.keep_memory_is_set for OPTION_NO_KEEP_MEMORY.
> > ---
> > bfd/elf-bfd.h | 3 ++
> > bfd/elf32-i386.c | 2 +-
> > bfd/elf64-x86-64.c | 2 +-
> > bfd/elflink.c | 72 ++++++++++++++++++++++++++++++++++++++--------
> > bfd/libbfd-in.h | 3 --
> > bfd/libbfd.h | 3 --
> > bfd/linker.c | 35 ----------------------
> > include/bfdlink.h | 3 ++
> > ld/NEWS | 2 ++
> > ld/ld.texi | 14 +++++----
> > ld/ldlex.h | 3 +-
> > ld/lexsup.c | 11 +++++--
> > 12 files changed, 90 insertions(+), 63 deletions(-)
> >
> > diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> > index e7c2cd19bed..b9ec590cd4e 100644
> > --- a/bfd/elf-bfd.h
> > +++ b/bfd/elf-bfd.h
> > @@ -3144,6 +3144,9 @@ extern bool _bfd_elf_mmap_section_contents
> > extern void _bfd_elf_munmap_section_contents
> > (asection *, void *);
> >
> > +extern bool _bfd_elf_link_keep_memory
> > + (struct bfd_link_info *);
> > +
> > /* Large common section. */
> > extern asection _bfd_elf_large_com_section;
> >
> > diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
> > index 0f963c373be..d1bd11094de 100644
> > --- a/bfd/elf32-i386.c
> > +++ b/bfd/elf32-i386.c
> > @@ -1932,7 +1932,7 @@ elf_i386_scan_relocs (bfd *abfd,
> >
> > if (elf_section_data (sec)->this_hdr.contents != contents)
> > {
> > - if (!converted && !_bfd_link_keep_memory (info))
> > + if (!converted && !_bfd_elf_link_keep_memory (info))
> > _bfd_elf_munmap_section_contents (sec, contents);
> > else
> > {
> > diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
> > index 2e3f86840e5..5e1fad069d1 100644
> > --- a/bfd/elf64-x86-64.c
> > +++ b/bfd/elf64-x86-64.c
> > @@ -2590,7 +2590,7 @@ elf_x86_64_scan_relocs (bfd *abfd, struct bfd_link_info *info,
> >
> > if (elf_section_data (sec)->this_hdr.contents != contents)
> > {
> > - if (!converted && !_bfd_link_keep_memory (info))
> > + if (!converted && !_bfd_elf_link_keep_memory (info))
> > _bfd_elf_munmap_section_contents (sec, contents);
> > else
> > {
> > diff --git a/bfd/elflink.c b/bfd/elflink.c
> > index 2a0586722e8..e5f2c75cb74 100644
> > --- a/bfd/elflink.c
> > +++ b/bfd/elflink.c
> > @@ -4205,10 +4205,9 @@ _bfd_elf_link_iterate_on_relocs
> > || bfd_is_abs_section (o->output_section))
> > continue;
> >
> > - internal_relocs = _bfd_elf_link_info_read_relocs (abfd, info,
> > - o, NULL,
> > - NULL,
> > - _bfd_link_keep_memory (info));
> > + internal_relocs = _bfd_elf_link_info_read_relocs
> > + (abfd, info, o, NULL, NULL,
> > + _bfd_elf_link_keep_memory (info));
> > if (internal_relocs == NULL)
> > return false;
> >
> > @@ -5574,10 +5573,9 @@ elf_link_add_object_symbols (bfd *abfd, struct bfd_link_info *info)
> > && (s->flags & SEC_DEBUGGING) != 0))
> > continue;
> >
> > - internal_relocs = _bfd_elf_link_info_read_relocs (abfd, info,
> > - s, NULL,
> > - NULL,
> > - _bfd_link_keep_memory (info));
> > + internal_relocs = _bfd_elf_link_info_read_relocs
> > + (abfd, info, s, NULL, NULL,
> > + _bfd_elf_link_keep_memory (info));
> > if (internal_relocs == NULL)
> > goto error_free_vers;
> >
> > @@ -13630,7 +13628,7 @@ init_reloc_cookie (struct elf_reloc_cookie *cookie,
> > info->callbacks->einfo (_("%P%X: can not read symbols: %E\n"));
> > return false;
> > }
> > - if (_bfd_link_keep_memory (info) )
> > + if (_bfd_elf_link_keep_memory (info) )
> > {
> > symtab_hdr->contents = (bfd_byte *) cookie->locsyms;
> > info->cache_size += (cookie->locsymcount
> > @@ -13667,9 +13665,9 @@ init_reloc_cookie_rels (struct elf_reloc_cookie *cookie,
> > }
> > else
> > {
> > - cookie->rels = _bfd_elf_link_info_read_relocs (abfd, info, sec,
> > - NULL, NULL,
> > - _bfd_link_keep_memory (info));
> > + cookie->rels = _bfd_elf_link_info_read_relocs
> > + (abfd, info, sec, NULL, NULL,
> > + _bfd_elf_link_keep_memory (info));
> > if (cookie->rels == NULL)
> > return false;
> > cookie->rel = cookie->rels;
> > @@ -15599,3 +15597,53 @@ _bfd_elf_add_dynamic_tags (bfd *output_bfd, struct bfd_link_info *info,
> >
> > return true;
> > }
> > +
> > +/* Return false if linker should avoid caching relocation information
> > + and symbol tables of input files in memory. */
> > +
> > +bool
> > +_bfd_elf_link_keep_memory (struct bfd_link_info *info)
> > +{
> > +#ifdef USE_MMAP
> > + /* Don't cache symbol nor relocation tables if they are mapped in.
> > + NB: Since the --no-keep-memory linker option causes:
> > +
> > + https://sourceware.org/bugzilla/show_bug.cgi?id=31458
> > +
> > + this is opt-in by each backend. */
> > + if (!info->keep_memory_is_set)
> > + {
> > + const struct elf_backend_data *bed
> > + = get_elf_backend_data (info->output_bfd);
> > + if (bed->use_mmap)
> > + return false;
> > + }
> > +#endif
> > + bfd *abfd;
> > + bfd_size_type size;
> > +
> > + if (!info->keep_memory)
> > + return false;
> > +
> > + if (info->max_cache_size == (bfd_size_type) -1)
> > + return true;
> > +
> > + abfd = info->input_bfds;
> > + size = info->cache_size;
> > + do
> > + {
> > + if (size >= info->max_cache_size)
> > + {
> > + /* Over the limit. Reduce the memory usage. */
> > + info->keep_memory = false;
> > + return false;
> > + }
> > + if (!abfd)
> > + break;
> > + size += abfd->alloc_size;
> > + abfd = abfd->link.next;
> > + }
> > + while (1);
> > +
> > + return true;
> > +}
> > diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
> > index 9d5396a0354..8ad9d36e396 100644
> > --- a/bfd/libbfd-in.h
> > +++ b/bfd/libbfd-in.h
> > @@ -848,9 +848,6 @@ extern bfd_byte * _bfd_write_unsigned_leb128
> > extern struct bfd_link_info *_bfd_get_link_info (bfd *)
> > ATTRIBUTE_HIDDEN;
> >
> > -extern bool _bfd_link_keep_memory (struct bfd_link_info *)
> > - ATTRIBUTE_HIDDEN;
> > -
> > extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
> > extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
> > extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
> > diff --git a/bfd/libbfd.h b/bfd/libbfd.h
> > index 4b605c4ce6c..17a6095ba00 100644
> > --- a/bfd/libbfd.h
> > +++ b/bfd/libbfd.h
> > @@ -854,9 +854,6 @@ extern bfd_byte * _bfd_write_unsigned_leb128
> > extern struct bfd_link_info *_bfd_get_link_info (bfd *)
> > ATTRIBUTE_HIDDEN;
> >
> > -extern bool _bfd_link_keep_memory (struct bfd_link_info *)
> > - ATTRIBUTE_HIDDEN;
> > -
> > extern uintptr_t _bfd_pagesize ATTRIBUTE_HIDDEN;
> > extern uintptr_t _bfd_pagesize_m1 ATTRIBUTE_HIDDEN;
> > extern uintptr_t _bfd_minimum_mmap_size ATTRIBUTE_HIDDEN;
> > diff --git a/bfd/linker.c b/bfd/linker.c
> > index 36cca9624c2..eb42a78b622 100644
> > --- a/bfd/linker.c
> > +++ b/bfd/linker.c
> > @@ -3556,38 +3556,3 @@ _bfd_nolink_bfd_define_start_stop (struct bfd_link_info *info ATTRIBUTE_UNUSED,
> > {
> > return (struct bfd_link_hash_entry *) _bfd_ptr_bfd_null_error (sec->owner);
> > }
> > -
> > -/* Return false if linker should avoid caching relocation infomation
> > - and symbol tables of input files in memory. */
> > -
> > -bool
> > -_bfd_link_keep_memory (struct bfd_link_info * info)
> > -{
> > - bfd *abfd;
> > - bfd_size_type size;
> > -
> > - if (!info->keep_memory)
> > - return false;
> > -
> > - if (info->max_cache_size == (bfd_size_type) -1)
> > - return true;
> > -
> > - abfd = info->input_bfds;
> > - size = info->cache_size;
> > - do
> > - {
> > - if (size >= info->max_cache_size)
> > - {
> > - /* Over the limit. Reduce the memory usage. */
> > - info->keep_memory = false;
> > - return false;
> > - }
> > - if (!abfd)
> > - break;
> > - size += abfd->alloc_size;
> > - abfd = abfd->link.next;
> > - }
> > - while (1);
> > -
> > - return true;
> > -}
> > diff --git a/include/bfdlink.h b/include/bfdlink.h
> > index eac07d78364..9397032c6dd 100644
> > --- a/include/bfdlink.h
> > +++ b/include/bfdlink.h
> > @@ -580,6 +580,9 @@ struct bfd_link_info
> > /* TRUE if commonpagesize is set on command-line. */
> > unsigned int commonpagesize_is_set : 1;
> >
> > + /* TRUE if keep_memory is set on command-line. */
> > + unsigned int keep_memory_is_set : 1;
> > +
> > /* Char that may appear as the first char of a symbol, but should be
> > skipped (like symbol_leading_char) when looking up symbols in
> > wrap_hash. Used by PowerPC Linux for 'dot' symbols. */
> > diff --git a/ld/NEWS b/ld/NEWS
> > index f70d2157339..8751b177329 100644
> > --- a/ld/NEWS
> > +++ b/ld/NEWS
> > @@ -1,5 +1,7 @@
> > -*- text -*-
> >
> > +* Add --keep-memory to always cache the symbol and relocation tables.
> > +
> > * Add -plugin-save-temps to store plugin intermediate files permanently.
> >
> > Changes in 2.42:
> > diff --git a/ld/ld.texi b/ld/ld.texi
> > index 6f234752278..792fdf1c2f3 100644
> > --- a/ld/ld.texi
> > +++ b/ld/ld.texi
> > @@ -2140,13 +2140,17 @@ If the map file already exists then it will be overwritten by this
> > operation.
> >
> > @cindex memory usage
> > +@kindex --keep-memory
> > @kindex --no-keep-memory
> > -@item --no-keep-memory
> > +@item --keep-memory
> > @command{ld} normally optimizes for speed over memory usage by caching the
> > -symbol tables of input files in memory. This option tells @command{ld} to
> > -instead optimize for memory usage, by rereading the symbol tables as
> > -necessary. This may be required if @command{ld} runs out of memory space
> > -while linking a large executable.
> > +symbol and relocation tables of input files in memory if @samp{mmap} isn't
> > +used to map them in. The @option{--no-keep-memory} option tells @command{ld}
> > +to instead optimize for memory usage, by rereading the symbol and
> > +relocation tables as necessary. This may be required if @command{ld}
> > +runs out of memory space while linking a large executable.
> > +The @option{--keep-memory} option tells @command{ld} to always cache the
> > +symbol and relocation tables.
>
> Sorry, one more thing: this is tri-state now, right?
>
> i.e. there's three different behaviours:
> 1) --keep-memory -> we cache more
Correct.
> 2) nothing/default -> old behaviour
Default depends on use_mmap. If use_mmap is true,
it is the same as --no-keep-memory. Otherwise, it
is the same as --keep-memory. For x86, the default is
--no-keep-memory.
> 3) --no-keep-memory -> we cache less, as before
Yes.
> If that's right, maybe explicitly say "The default is a compromise
> between explicit --no-keep-memory and --keep-memory. --keep-memory
> behavior is different." or similar?
>
> Right now, it might sound like --keep-memory does the same as not
> passing it at all, unless overridden later.
>
> >
> > @kindex --no-undefined
> > @kindex -z defs
> > diff --git a/ld/ldlex.h b/ld/ldlex.h
> > index d575562a357..49ac8552e1f 100644
> > --- a/ld/ldlex.h
> > +++ b/ld/ldlex.h
> > @@ -43,9 +43,10 @@ enum option_values
> > OPTION_NO_EXPORT_DYNAMIC,
> > OPTION_HELP,
> > OPTION_IGNORE,
> > + OPTION_KEEP_MEMORY,
> > + OPTION_NO_KEEP_MEMORY,
> > OPTION_MAP,
> > OPTION_NO_DEMANGLE,
> > - OPTION_NO_KEEP_MEMORY,
> > OPTION_NO_WARN_MISMATCH,
> > OPTION_NO_WARN_SEARCH_MISMATCH,
> > OPTION_NOINHIBIT_EXEC,
> > diff --git a/ld/lexsup.c b/ld/lexsup.c
> > index dad3b6059ed..96d37ca3093 100644
> > --- a/ld/lexsup.c
> > +++ b/ld/lexsup.c
> > @@ -383,14 +383,16 @@ static const struct ld_option ld_options[] =
> > '\0', NULL, N_("Print option help"), TWO_DASHES },
> > { {"init", required_argument, NULL, OPTION_INIT},
> > '\0', N_("SYMBOL"), N_("Call SYMBOL at load-time"), ONE_DASH },
> > + { {"keep-memory", no_argument, NULL, OPTION_KEEP_MEMORY},
> > + '\0', NULL, N_("Use more memory and less disk I/O"), TWO_DASHES },
> > + { {"no-keep-memory", no_argument, NULL, OPTION_NO_KEEP_MEMORY},
> > + '\0', NULL, N_("Use less memory and more disk I/O"), TWO_DASHES },
> > { {"Map", required_argument, NULL, OPTION_MAP},
> > '\0', N_("FILE/DIR"), N_("Write a linker map to FILE or DIR/<outputname>.map"), ONE_DASH },
> > { {"no-define-common", no_argument, NULL, OPTION_NO_DEFINE_COMMON},
> > '\0', NULL, N_("Do not define Common storage"), TWO_DASHES },
> > { {"no-demangle", no_argument, NULL, OPTION_NO_DEMANGLE },
> > '\0', NULL, N_("Do not demangle symbol names"), TWO_DASHES },
> > - { {"no-keep-memory", no_argument, NULL, OPTION_NO_KEEP_MEMORY},
> > - '\0', NULL, N_("Use less memory and more disk I/O"), TWO_DASHES },
> > { {"no-undefined", no_argument, NULL, OPTION_NO_UNDEFINED},
> > '\0', NULL, N_("Do not allow unresolved references in object files"),
> > TWO_DASHES },
> > @@ -1091,8 +1093,13 @@ parse_args (unsigned argc, char **argv)
> > case OPTION_NO_PRINT_GC_SECTIONS:
> > link_info.print_gc_sections = false;
> > break;
> > + case OPTION_KEEP_MEMORY:
> > + link_info.keep_memory = true;
> > + link_info.keep_memory_is_set = true;
> > + break;
> > case OPTION_NO_KEEP_MEMORY:
> > link_info.keep_memory = false;
> > + link_info.keep_memory_is_set = true;
> > break;
> > case OPTION_NO_UNDEFINED:
> > link_info.unresolved_syms_in_objects = RM_DIAGNOSE;
--
H.J.
More information about the Binutils
mailing list