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

[Boris Ulasevich]

Hi Zhengyu,

On 18.05.2020 19:00, Zhengyu Gu wrote:
> 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?

Yes, Hotspot guard page is just below start address, and start address 
is page aligned in my case.

The start address change was done in webrev.01 to make the input range 
completely unrestricted. The original webrev.00 had the modification for 
size only (see this mail thread).

>> 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?

Yes, with a fix for "Not contain this region" assertion [1] it can be 
done this way:
http://cr.openjdk.java.net/~bulasevich/8241004/webrev.02b

[1] 
http://hg.openjdk.java.net/jdk/jdk/file/a44396903fd8/src/hotspot/share/services/virtualMemoryTracker.cpp#l112

> 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
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>