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

src: unify Linux and FreeBSD large pages implem #32534

Conversation

gabrielschulhof
Copy link
Contributor

dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. labels Mar 28, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gabrielschulhof gabrielschulhof added freebsd Issues and PRs related to the FreeBSD platform. linux Issues and PRs related to the Linux platform. labels Mar 28, 2020
@gabrielschulhof gabrielschulhof added the macos Issues and PRs related to the macOS platform / OSX. label Mar 28, 2020
@gabrielschulhof
Copy link
Contributor Author

I added the macos label because this code also simplifies the OSX portion by removing a superfluous memcpy() and mprotect().

src/large_pages/node_large_page.cc Outdated Show resolved Hide resolved
dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on
  * https://www.osboxes.org/freebsd/#freebsd-12-1-vbox
  * https://www.osboxes.org/freebsd/#freebsd-11-vbox
  * test-digitalocean-freebsd11-x64-2
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30154/

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gabrielschulhof gabrielschulhof removed the macos Issues and PRs related to the macOS platform / OSX. label Mar 28, 2020
@gabrielschulhof
Copy link
Contributor Author

I removed the macos label because factoring things out too much seems to be causing a crash there.

@gabrielschulhof
Copy link
Contributor Author

Heh! Forgot the final memcpy() for FreeBSD 😳

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 28, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/30159/ (:heavy_check_mark:)

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 28, 2020
gabrielschulhof pushed a commit that referenced this pull request Mar 30, 2020
dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on
  * https://www.osboxes.org/freebsd/#freebsd-12-1-vbox
  * https://www.osboxes.org/freebsd/#freebsd-11-vbox
  * test-digitalocean-freebsd11-x64-2

Signed-off-by: Gabriel Schulhof <[email protected]>
PR-URL: #32534
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@gabrielschulhof
Copy link
Contributor Author

Landed in 67d5c90.

addaleax pushed a commit that referenced this pull request Mar 30, 2020
dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on
  * https://www.osboxes.org/freebsd/#freebsd-12-1-vbox
  * https://www.osboxes.org/freebsd/#freebsd-11-vbox
  * test-digitalocean-freebsd11-x64-2

Signed-off-by: Gabriel Schulhof <[email protected]>
PR-URL: #32534
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
@gabrielschulhof gabrielschulhof deleted the large-pages-unify-linux-freebsd branch April 21, 2020 02:58
@targos
Copy link
Member

targos commented Apr 22, 2020

Depends on the large pages change to land on v12.x

@targos targos removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-blocked-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on
  * https://www.osboxes.org/freebsd/#freebsd-12-1-vbox
  * https://www.osboxes.org/freebsd/#freebsd-11-vbox
  * test-digitalocean-freebsd11-x64-2

Signed-off-by: Gabriel Schulhof <[email protected]>
PR-URL: nodejs#32534
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
dl_iterate_phdr(3) is also available for FreeBSD. This change adds the
same trimming code for the start and end of the .text section as on
Linux, making it work on FreeBSD, and removing the need for the
additional FreeBSD-specific check.

Manually tested on
  * https://www.osboxes.org/freebsd/#freebsd-12-1-vbox
  * https://www.osboxes.org/freebsd/#freebsd-11-vbox
  * test-digitalocean-freebsd11-x64-2

Signed-off-by: Gabriel Schulhof <[email protected]>
PR-URL: #32534
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: David Carlier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. freebsd Issues and PRs related to the FreeBSD platform. linux Issues and PRs related to the Linux platform.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants