Re: [PATCH v4 6/6] elf: Don't cache symbol nor relocation tables with mmap

[Sam James <sam@gentoo.org>]

"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:
>
> FAIL: MIPS eh-frame 3
>
> in the linker testsuite for mipsisa32r2el-elf target, this is opt-in by
> each backend.
>
> 	* 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.
> 	(_bfd_elf_link_keep_memory): New.
> 	* libbfd-in.h (_bfd_link_keep_memory): Removed.
> 	* linker.c (_bfd_link_keep_memory): Likewise.
> 	* libbfd.h: Regenerated.
> ---
>  bfd/elf-bfd.h      |  3 ++
>  bfd/elf32-i386.c   |  2 +-
>  bfd/elf64-x86-64.c |  2 +-
>  bfd/elflink.c      | 70 ++++++++++++++++++++++++++++++++++++++--------
>  bfd/libbfd-in.h    |  3 --
>  bfd/libbfd.h       |  3 --
>  bfd/linker.c       | 35 -----------------------
>  7 files changed, 63 insertions(+), 55 deletions(-)
>
> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
> index da7c5208017..6ed6f13cba2 100644
> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -3144,6 +3144,9 @@ extern bool _bfd_elf_mmap_section
>  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 8aa1533d0ba..606e38b472c 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 f1e06040a5a..e52fa932ffc 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 4602fb3d10f..d7f5fe90017 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,51 @@ _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
> +
> +     FAIL: MIPS eh-frame 3
> +
> +     in the linker testsuite for mipsisa32r2el-elf target, this is
> +     opt-in by each backend.  */

Please add a reference to PR31458.

> +  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 effe1b86b53..f89de00b551 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 c80f5a86ed1..3ed2e55a34b 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;
> -}