RFR(S) 8241004: NMT tests fail on unalinged thread size with debug build

[Zhengyu Gu]

Hi Boris,

I could not find a system to reproduce, I am not sure how it works.

So, I have a few questions:

> 
-  assert(is_aligned(start, page_sz), "Start address must be page aligned");
-  assert(is_aligned(size, page_sz), "Size must be page aligned");
+  address aligned_start = align_down(start, page_sz);
+  address aligned_end   = align_up(start + size, page_sz);
+  int     page_num      = (aligned_end - aligned_start) / page_sz;

Below start address should be stack guard pages. IIRC, stack guards are 
page aligned, right? so how can start address not page aligned?

Align down start address seems can collide guard pages?


> I'm not sure I got the idea.VirtualMemoryTracker is already doing what 
> you said: "scanning reserved regions for committed regions":
> - snapshot_thread_stacks() iterates over reserved regions
> - for each mtThreadStack region SnapshotThreadStackWalker iterates over 
> its committed ranges
> - committed ranges of a given region are provided by linux kernel and 
> summed up by ReservedMemoryRegion::add_committed_region

If unaligned stack is due to unaligned base (stack top), would aligning 
up stack size in SnapshotThreadStackWalker::do_allocation_site() solve 
the problem?

Thanks,

-Zhengyu

> 
> Yes, it is unaligned thread stack issue reproduced with 
> -XX:NativeMemoryTracking option.
> 
>> -Zhengyu
>>
>> [1] 
>> http://hg.openjdk.java.net/jdk/jdk/file/a44396903fd8/src/hotspot/share/services/virtualMemoryTracker.cpp#l531
>>
>>>
>>> Thanks, Thomas
>>>
>>>
>>>
>>> On Wed, May 13, 2020 at 6:29 PM Boris Ulasevich 
>>> <boris.ulasevich at bell-sw.com>
>>> wrote:
>>>
>>>> Hi Thomas,
>>>>
>>>> Good point! Thanks.
>>>> I reworked the function to ease both restrictions.
>>>> http://cr.openjdk.java.net/~bulasevich/8241004/webrev.01
>>>>
>>>> regards,
>>>> Boris
>>>>
>>>> On 13.05.2020 08:59, Thomas Stüfe wrote:
>>>>
>>>> Hi Boris,
>>>>
>>>> Note that we have unaligned stack boundaries on AIX too, not so 
>>>> strange :)
>>>>
>>>> What I do not like about the solution is that it loosens the 
>>>> restriction
>>>> on the input size while keeping the restriction on the start 
>>>> address. I'd
>>>> rather see this function either in its current form (requiring the 
>>>> input
>>>> ranges to be page aligned) or the input range to be completely
>>>> unrestricted. This function is currently only used by NMT but it is 
>>>> useful
>>>> and may be used in other places too.
>>>>
>>>> Can you make it work with arbitrary input ranges? I do not think this
>>>> would require much changes from your patch, you do most of the work 
>>>> already.
>>>>
>>>> Thanks!
>>>>
>>>> ..Thomas
>>>>
>>>>
>>>>
>>>> On Thu, May 7, 2020 at 2:56 PM boris <boris.ulasevich at bell-sw.com> 
>>>> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Please review the following change.
>>>>>
>>>>> http://cr.openjdk.java.net/~bulasevich/8241004/webrev.00
>>>>> http://bugs.openjdk.java.net/browse/JDK-8241004
>>>>>
>>>>> The stack size created by the implementation of the pthread library is
>>>>> not necessarily page aligned. It is weird, but it is not forbidden: 
>>>>> the
>>>>> pthread library may add some gap on it own. My change removes stack 
>>>>> size
>>>>> alignment assert checks and tunes NMT committed range calculation for
>>>>> the case of unaligned stack size.
>>>>>
>>>>> thanks,
>>>>> Boris
>>>>>
>>>>
>>>>
>>>
>>
>