Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8280940: gtest os.release_multi_mappings_vm is racy
Reviewed-by: dcubed, sjohanss
  • Loading branch information
tstuefe committed Feb 14, 2022
1 parent 9d0a4c3 commit f07b816
Showing 1 changed file with 35 additions and 12 deletions.
47 changes: 35 additions & 12 deletions test/hotspot/gtest/runtime/test_os.cpp
Expand Up @@ -441,39 +441,62 @@ struct NUMASwitcher {
#endif

#ifndef _AIX // JDK-8257041
TEST_VM(os, release_multi_mappings) {
TEST_VM(os, release_multi_mappings) {

// With NMT enabled, this will trigger JDK-8263464. For now disable the test if NMT=on.
if (MemTracker::tracking_level() > NMT_off) {
return;
}

// Test that we can release an area created with multiple reservation calls
const size_t stripe_len = 4 * M;
const int num_stripes = 4;
// What we do:
// A) we reserve 6 small segments (stripes) adjacent to each other. We commit
// them with alternating permissions to prevent the kernel from folding them into
// a single segment.
// -stripe-stripe-stripe-stripe-stripe-stripe-
// B) we release the middle four stripes with a single os::release_memory call. This
// tests that os::release_memory indeed works across multiple segments created with
// multiple os::reserve calls.
// -stripe-___________________________-stripe-
// C) Into the now vacated address range between the first and the last stripe, we
// re-reserve a new memory range. We expect this to work as a proof that the address
// range was really released by the single release call (B).
//
// Note that this is inherently racy. Between (B) and (C), some other thread may have
// reserved something into the hole in the meantime. Therefore we keep that range small and
// entrenched between the first and last stripe, which reduces the chance of some concurrent
// thread grabbing that memory.

const size_t stripe_len = os::vm_allocation_granularity();
const int num_stripes = 6;
const size_t total_range_len = stripe_len * num_stripes;

// reserve address space...
address p = reserve_multiple(num_stripes, stripe_len);
ASSERT_NE(p, (address)NULL);
PRINT_MAPPINGS("A");

// .. release it...
// .. release the middle stripes...
address p_middle_stripes = p + stripe_len;
const size_t middle_stripe_len = (num_stripes - 2) * stripe_len;
{
// On Windows, use UseNUMAInterleaving=1 which makes
// os::release_memory accept multi-map-ranges.
// Otherwise we would assert (see below for death test).
// On Windows, temporarily switch on UseNUMAInterleaving to allow release_memory to release
// multiple mappings in one go (otherwise we assert, which we test too, see death test below).
WINDOWS_ONLY(NUMASwitcher b(true);)
ASSERT_TRUE(os::release_memory((char*)p, total_range_len));
ASSERT_TRUE(os::release_memory((char*)p_middle_stripes, middle_stripe_len));
}
PRINT_MAPPINGS("B");

// re-reserve it. This should work unless release failed.
address p2 = (address)os::attempt_reserve_memory_at((char*)p, total_range_len);
ASSERT_EQ(p2, p);
// ...re-reserve the middle stripes. This should work unless release silently failed.
address p2 = (address)os::attempt_reserve_memory_at((char*)p_middle_stripes, middle_stripe_len);
ASSERT_EQ(p2, p_middle_stripes);
PRINT_MAPPINGS("C");

ASSERT_TRUE(os::release_memory((char*)p, total_range_len));
// Clean up. Release all mappings.
{
WINDOWS_ONLY(NUMASwitcher b(true);) // allow release_memory to release multiple regions
ASSERT_TRUE(os::release_memory((char*)p, total_range_len));
}
}
#endif // !AIX

Expand Down

3 comments on commit f07b816

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@tstuefe
Copy link
Member Author

@tstuefe tstuefe commented on f07b816 Apr 1, 2022

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on f07b816 Apr 1, 2022

Choose a reason for hiding this comment

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

@tstuefe Could not automatically backport f07b8165 to openjdk/jdk17u-dev due to conflicts in the following files:

  • test/hotspot/gtest/runtime/test_os.cpp

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk17u-dev:

$ git checkout -b tstuefe-backport-f07b8165
$ git fetch --no-tags https://git.openjdk.java.net/jdk f07b8165231799383303e5c0755d07afd2feb7fd
$ git cherry-pick --no-commit f07b8165231799383303e5c0755d07afd2feb7fd
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport f07b8165231799383303e5c0755d07afd2feb7fd'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport f07b8165231799383303e5c0755d07afd2feb7fd.

Please sign in to comment.