Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

YJIT: Use the system page size when the code page size is too small #7267

Merged
merged 1 commit into from Feb 9, 2023

Conversation

XrXr
Copy link
Member

@XrXr XrXr commented Feb 7, 2023

Previously on ARM64 Linux systems that use 64 KiB pages
(CONFIG_ARM64_64K_PAGES=y), YJIT was panicking on boot due to a failed
assertion.

The assertion was making sure that code GC can free the last code page
that YJIT manages without freeing unrelated memory. YJIT prefers picking
16 KiB as the granularity at which to free code memory, but when the
system can only free at 64 KiB granularity, that is not possible.

The fix is to use the system page size as the code page size when the
system page size is 64 KiB. Continue to use 16 KiB as the code page size
on common systems that use 16/4 KiB pages.

Add asserts to code_gc() and free_page() about code GC's assumptions.

Fixes [Bug #19400]

Previously on ARM64 Linux systems that use 64 KiB pages
(`CONFIG_ARM64_64K_PAGES=y`), YJIT was panicking on boot due to a failed
assertion.

The assertion was making sure that code GC can free the last code page
that YJIT manages without freeing unrelated memory. YJIT prefers picking
16 KiB as the granularity at which to free code memory, but when the
system can only free at 64 KiB granularity, that is not possible.

The fix is to use the system page size as the code page size when the
system page size is 64 KiB. Continue to use 16 KiB as the code page size
on common systems that use 16/4 KiB pages.

Add asserts to code_gc() and free_page() about code GC's assumptions.

Fixes [Bug #19400]
@matzbot matzbot requested a review from a team February 7, 2023 23:58
Copy link
Contributor

@maximecb maximecb left a comment

Choose a reason for hiding this comment

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

Well done. Thank you Alan!

@maximecb maximecb merged commit b78f871 into ruby:master Feb 9, 2023
@XrXr XrXr deleted the yjit-64k-pages branch February 9, 2023 16:18
nurse added a commit to nurse/ruby that referenced this pull request Mar 4, 2023
…Backport #19400]

	YJIT: Use the system page size when the code page size is too small
	 (ruby#7267)

	Previously on ARM64 Linux systems that use 64 KiB pages
	(`CONFIG_ARM64_64K_PAGES=y`), YJIT was panicking on boot due to a failed
	assertion.

	The assertion was making sure that code GC can free the last code page
	that YJIT manages without freeing unrelated memory. YJIT prefers picking
	16 KiB as the granularity at which to free code memory, but when the
	system can only free at 64 KiB granularity, that is not possible.

	The fix is to use the system page size as the code page size when the
	system page size is 64 KiB. Continue to use 16 KiB as the code page size
	on common systems that use 16/4 KiB pages.

	Add asserts to code_gc() and free_page() about code GC's assumptions.

	Fixes [Bug #19400]
	---
	 yjit/src/asm/mod.rs    | 78 ++++++++++++++++++++++++++++++++------------------
	 yjit/src/codegen.rs    |  2 --
	 yjit/src/virtualmem.rs | 13 +++++++++
	 3 files changed, 63 insertions(+), 30 deletions(-)

	YJIT: Fix assertion for partially mapped last pages (ruby#7337)

	Follows up [Bug #19400]
	---
	 test/ruby/test_yjit.rb | 19 +++++++++++++++++++
	 yjit/src/asm/mod.rs    |  2 +-
	 yjit/src/virtualmem.rs | 18 +++++++++++++-----
	 3 files changed, 33 insertions(+), 6 deletions(-)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants