FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Annotate anonymous mmap by wks · Pull Request #12119 · ruby/ruby · GitHub

Repository navigation

Annotate anonymous mmap - #12119

Merged
peterzhu2118 merged 2 commits into
ruby:masterfrom
wks:ruby-mmap-anno
Nov 21, 2024
Merged

peterzhu2118 merged 2 commits into
ruby:masterfrom
wks:ruby-mmap-anno

Conversation

wks commented Nov 19, 2024

Copy link
Copy Markdown
Contributor

Use PR_SET_VMA_ANON_NAME to set human-readable names for anonymous virtual memory areas mapped by mmap() when compiled and run on Linux 5.17 or higher. This makes it convenient for developers to debug mmap.

matzbot requested a review from a team November 19, 2024 09:06

nobu commented Nov 19, 2024

Copy link
Copy Markdown
Member

It is questionable that ruby_annotate_mmap is generic to be placed in util.c.
Maybe gc.c?

wks commented Nov 19, 2024

Copy link
Copy Markdown
Contributor Author

It is questionable that ruby_annotate_mmap is generic to be placed in util.c. Maybe gc.c?

Probably. Since it is about memory management, and it is in fact my primary intention to debug memory usage related to the GC heap, it should be reasonable to put it in gc.c. However, the garbage-collected heap is just one (heap_page_body_allocate) among many parts of the Ruby VM that call mmap directly. Other users of mmap (RJIT, YJIT, fiber, IO buffer, shape list...) are not about garbage collection. It is better if there is a module that is solely responsible for the abstraction of mmap (and VirtualAlloc for Windows). But for now, we can interpret gc.c as the module for managing memory, and I'll put ruby_annotate_mmap in it so that this pull request doesn't make too many changes.

wks commented Nov 20, 2024

Copy link
Copy Markdown
Contributor Author

I moved ruby_annotate_mmap to gc.c and internal/gc.h.

I also changed gc/default.c to make it call prctl directly instead of calling ruby_annotate_mmap so that it will still work when the default GC is compiled as a shared library.

XrXr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sounds useful from YJIT's perspective. I assume this will show up in /proc/self/maps so if you induce a crash with e.g. ruby -e 'Process.kill :BUS, $$' it should show up in the report?

peterzhu2118 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks good to me. Thank you for the contribution!

What do you think about instead of requiring places to call mmap + ruby_annotate_mmap, we have a ruby_mmap or xmmap that accepts a string name as an additional parameter? That would reduce the amount of code that need to be added.

Comment thread gc.c Resolved
Comment thread shape.c Resolved
Comment thread shape.c Resolved

wks commented Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

Sounds useful from YJIT's perspective. I assume this will show up in /proc/self/maps so if you induce a crash with e.g. ruby -e 'Process.kill :BUS, $$' it should show up in the report?

Yes. It will be shown in the "* Process memory map:" section of the error output.

$ /miniruby --yjit -e 'Process.kill :BUS, $$'
-e:1: [BUG] Bus Error at 0x000003e800003931
ruby 3.4.0dev (2024-11-20T10:40:00Z ruby-mmap-anno 8363ba458e) +YJIT +PRISM [x86_64-linux]
...
* Process memory map:

5b104e861000-5b104e864000 r-xp 00000000 00:00 0                          [anon:Ruby:rb_yjit_reserve_addr_space]
5b104e864000-5b1056861000 ---p 00000000 00:00 0                          [anon:Ruby:rb_yjit_reserve_addr_space]
5b1056866000-5b10568ab000 r--p 00000000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
5b10568ab000-5b1056d57000 r-xp 00045000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
5b1056d57000-5b1056eed000 r--p 004f1000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
5b1056eed000-5b1056f05000 r--p 00687000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
5b1056f05000-5b1056f07000 rw-p 0069f000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
5b1056f07000-5b1056f1c000 rw-p 00000000 00:00 0 
5b109523e000-5b109547e000 rw-p 00000000 00:00 0                          [heap]
72cd19014000-72cd19200000 r--s 00000000 103:02 789874                    /usr/lib/libc.so.6
72cd19200000-72cd1befe000 r--s 00000000 103:03 90876878                  /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
72cd1bf20000-72cd1bf90000 rw-p 00000000 00:00 0                          [anon:Ruby:GC:default:heap_page_body_allocate]
72cd1bf9f000-72cd1bfa0000 ---p 00000000 00:00 0                          [anon:Ruby:fiber_pool_allocate_memory]
72cd1bfa0000-72cd1c041000 rw-p 00000000 00:00 0                          [anon:Ruby:fiber_pool_allocate_memory]
...
72cd1d33e000-72cd1d3df000 rw-p 00000000 00:00 0                          [anon:Ruby:fiber_pool_allocate_memory]
72cd1d3df000-72cd1d3e0000 ---p 00000000 00:00 0 
72cd1d3e0000-72cd1dbe0000 rw-p 00000000 00:00 0 
72cd1dbe0000-72cd1dc00000 rw-p 00000000 00:00 0                          [anon:Ruby:GC:default:heap_page_body_allocate]
72cd1dc00000-72cd35c00000 rw-p 00000000 00:00 0                          [anon:Ruby:Init_default_shapes:shape_cache]
72cd35c00000-72cd37000000 rw-p 00000000 00:00 0                          [anon:Ruby:Init_default_shapes:shape_list]
72cd37000000-72cd375f0000 r--p 00000000 103:02 915413                    /usr/lib/locale/locale-archive
72cd37600000-72cd37610000 rw-p 00000000 00:00 0                          [anon:Ruby:GC:default:heap_page_body_allocate]
72cd3761f000-72cd37786000 rw-p 00000000 00:00 0 
72cd37786000-72cd377aa000 r--p 00000000 103:02 789874                    /usr/lib/libc.so.6
...

BTW, another way to print the mapping without crashing the process is using GDB. We can set a break point at, for example, gc_start, and use the GDB command info proc mappings. The annotations will also appear in the GDB output, with a slightly different format:

(gdb) info proc mappings 
process 14894
Mapped address spaces:

          Start Addr           End Addr       Size     Offset  Perms  objfile
      0x55554d54f000     0x55554d552000     0x3000        0x0  r-xp   [anon:Ruby:rb_yjit_reserve_addr_space]
      0x55554d552000     0x55555554f000  0x7ffd000        0x0  ---p   [anon:Ruby:rb_yjit_reserve_addr_space]
      0x555555554000     0x555555599000    0x45000        0x0  r--p   /home/wks/projects/mmtk-github/parallels/feature/ruby-mmap-anno/ruby/build-debug/miniruby
...
      0x555555c0a000     0x555555e4a000   0x240000        0x0  rw-p   [heap]
      0x7fffdc310000     0x7fffdc380000    0x70000        0x0  rw-p   [anon:Ruby:GC:default:heap_page_body_allocate]
      0x7fffdc38f000     0x7fffdc390000     0x1000        0x0  ---p   [anon:Ruby:fiber_pool_allocate_memory]
      0x7fffdc390000     0x7fffdc431000    0xa1000        0x0  rw-p   [anon:Ruby:fiber_pool_allocate_memory]
...
      0x7fffdd72e000     0x7fffdd7cf000    0xa1000        0x0  rw-p   [anon:Ruby:fiber_pool_allocate_memory]
      0x7fffdd7cf000     0x7fffdd7d0000     0x1000        0x0  ---p   
      0x7fffdd7d0000     0x7fffddfd0000   0x800000        0x0  rw-p   
      0x7fffddfd0000     0x7fffde000000    0x30000        0x0  rw-p   [anon:Ruby:GC:default:heap_page_body_allocate]
      0x7fffde000000     0x7ffff6000000 0x18000000        0x0  rw-p   [anon:Ruby:Init_default_shapes:shape_cache]
      0x7ffff6000000     0x7ffff7400000  0x1400000        0x0  rw-p   [anon:Ruby:Init_default_shapes:shape_list]
      0x7ffff7400000     0x7ffff79f0000   0x5f0000        0x0  r--p   /usr/lib/locale/locale-archive

wks commented Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

What do you think about instead of requiring places to call mmap + ruby_annotate_mmap, we have a ruby_mmap or xmmap that accepts a string name as an additional parameter? That would reduce the amount of code that need to be added.

Yes, and ideally it is more useful if it can call VirtualAlloc on Windows, too. It should also handle difference between Linux and MacOS, for example, the presence of MAP_FIXED_NOREPLACE. The resulting function will be something like:

void *
xmmap_anonymous(void *hint, size_t size, rb_mmap_perm_t perm, bool shared, const char *name)
{
#if windows
    // call VirtualAlloc
#else
    // call mmap
# if __linux__
    if (name != NULL) {
        // call prctl
    }
# endif
#endif
}

This is only used for anonymous memory. Memory-mapped files will have a different wrapper.

And we need to consider some special use cases. One example is rb_yjit_reserve_addr_space. It takes advantage of Linux's MAP_FIXED_NOREPLACE to try to allocate the region close to a given address without overwriting existing mmap ranges.

And we need to think about whether we should give C extensions access to this function. Some bundled gems use mmap, too. For example, fiddle. Garbage collector implementations built as .so may need to use mmap, too. The default GC definitely needs mmap. Third-party GC libraries (e.g. MMTk) usually handle mmap and annotations by themselves, so it is not necessary to expose things like xmmap_anonymous to them.

It is a bit complicated, so I think it is better not to introduce the wrapper in this pull request.

Use PR_SET_VMA_ANON_NAME to set human-readable names for anonymous
virtual memory areas mapped by `mmap()` when compiled and run on Linux
5.17 or higher.  This makes it convenient for developers to debug mmap.

wks commented Nov 21, 2024

Copy link
Copy Markdown
Contributor Author

The CI test failed once. The details are here. But it doesn't look like it is related to this PR, and I can't reproduce it locally. I rebased it on the latest master branch and pushed again.

peterzhu2118 commented Nov 21, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

@wks Can you rebase? The compilation issues should be fixed in 6fc8607

Edit: looks like we can just merge master using the GitHub UI.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants


Back | FazBrowse Home | New Git URL