RFR: 8255917: runtime/cds/SharedBaseAddress.java failed "assert(reserved_rgn != 0LL) failed: No reserved region" [v2]

[Ioi Lam]

On Mon, 7 Dec 2020 18:05:21 GMT, Yumin Qi <minqi at openjdk.org> wrote:

>> Hi, Please review
>>   Windows mapping for file into memory could not happen to reserved memory. In mapping CDS archive we first reserve enough memory then before mapping, release them. For cds archive and using class space, need split the whole space into two, that is, release the whole reserved space and do reservation to the two split spaces again, which is problematic that there is possibility other thread or system can kick in to take the released space.
>>   The fix is the first step of two steps:
>>    1) Do not split reserved memory;
>>    2) Remove splitting memory.
>>    This fix is first step, for Windows and use requested mapping address, reserved for cds archive and ccs on a contiguous space separately, so there is no need to call split. If any reservation failed, release them, go to other way, but do not do the 'real' split either. For Windows (and using class space), the reserved space will be released anyway. 
>> 
>> Tests:tier1-5,tier7
>
> Yumin Qi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add comment for reserve archive space and ccs on windows when mapping on required address

Changes requested by iklam (Reviewer).

src/hotspot/share/memory/metaspaceShared.cpp line 1655:

> 1653:     archive_space_rs = total_rs.first_part(ccs_begin_offset,
> 1654:                                            (size_t)os::vm_allocation_granularity(),
> 1655:                                            /*split=*/false);

Since `/*split=*/false` is passed,  the region is no longer split using `os::split_reserved_memory()`. This means on Windows, we  cannot use `os::release_memory()` to free the individual regions of `archive_space_rs` and `class_space_rs`.

The Windows API of [VirtualFree](https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-virtualfree) says:

> Releases the specified region of pages, or placeholder [...], `dwSize` must be 0 (zero), and `lpAddress` must point to the base address returned by the `VirtualAlloc` function when the region is reserved. The function fails if either of these conditions is not met.

I suggest we do this:

- Add an extra `ReservedSpace& total_rs` parameter to `MetaspaceShared::reserve_address_space_for_archives()`. Return the `total_rs` when we go through this path of the code.
- Also pass `total_rs` to `MetaspaceShared::release_reserved_spaces`. If `total_rs.is_reserverd()` is true, release `total_rs` instead of the two smaller spaces.

To make sure this PR is correct, we should add something like the following in `os::release_memory()`, and check for this log in test/hotspot/jtreg/runtime/cds/SharedBaseAddress.java:

if (!res) {
  log_info(os)("os::release_memory(" PTR_FORMAT ", " SIZE_FORMAT ") failed", p2i(addr), bytes);
}

Perhaps, in a separate RFE, we should add an assert in `os::release_memory()`, or at least change the `log_info` to `log_warning`.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1657