RFR(s): 8078513: [linux] Clean up code relevant to LinuxThreads implementation
Coleen Phillimore
coleen.phillimore at oracle.com
Tue May 12 17:14:33 UTC 2015
Hi Thomas, I like this change too, thank you!
Can you add the cleanups that David suggested and send it back out for
review? Except supports_variable_stack_size() change could be a follow
on RFR.
Thanks,
Coleen
On 5/10/15, 8:32 PM, David Holmes wrote:
> Hi Thomas,
>
> First thanks for taking on this cleanup task - it has been a looong
> time coming.
>
> This looks good. I don't see anything wrong but a few opportunities
> for additional clean-up.
>
> On 5/05/2015 11:16 PM, Thomas Stüfe wrote:
>> Hi all,
>>
>> This fix is a follow-up from my earlier question:
>> http://mail.openjdk.java.net/pipermail/hotspot-dev/2015-April/018186.html
>>
>>
>> webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8078513/webrev.00/webrev/
>> bug: https://bugs.openjdk.java.net/browse/JDK-8078513
>
> os_linux.cpp:
>
> The whole MAP_GROWSDOWN discussion and code also seems to be no longer
> relevant.
>
> The gcc 2.x workarounds can also be removed I think.
>
> I wonder if we can also eradicate WorkAroundNPTLTimedWaitHang :)
>
> Aside: The createThread_lock (now unused) seems to have been copied
> into src/os/aix/vm/os_aix.hpp as part of the AIX port and should be
> removed at some point.
>
> ---
>
> src/os/linux/vm/os_linux.hpp
>
> Copyright needs updating.
>
> ---
>
> src/os_cpu/linux_x86/vm/os_linux_x86.cpp
>
> Copyright needs updating.
>
> os::Linux::supports_variable_stack_size() is now dead code (value is
> true for all arch's AFAICS) so can be removed from all
> src/os_cpu/linux_XXX/os_linux_XXX.cpp.
>
> I think supports_variable_stack_size() is also always true on other
> platforms (AIX, BSD) and so could be cleaned up in that code too
> (separate clean-up CR okay for that).
>
> ---
>
> src/os_cpu/linux_x86/vm/threadLS_linux_x86.cpp
>
> Copyright needs updating.
>
> ---
>
> I'm happy to sponsor this for you.
>
> Thanks,
> David
> -----
>
>
>>
>> This change removes (I hope all) remnants of code dealing with
>> LinuxThreads
>> from the hotspot.
>>
>> Discussion of the changes:
>>
>> 1) on LinuxThreads, each thread had an own pid, so getpid() would
>> return a
>> unique pid for every thread it was invoked in. On NPTL, this is not an
>> issue anymore; getpid() works as expected returning a global pid.
>> For LinuxThreads, there was a workaround where at VM startup the pid was
>> captured (on the thread invoking CreateJavaVM) and this pid was
>> stored in
>> static variable _initial_pid and returned in os::current_process_id().
>> Later another workaround was added because CreateJavaVM was not
>> invoked on
>> the primordial thread anymore, so the pid was handed in via a define
>> (see
>> Arguments::sun_java_launcher_pid()).
>>
>> Both workaround were removed and os::current_process_id() was changed to
>> just return getpid().
>
> We should remove the code on the JDK side as well - possibly as a
> follow up clean up (but via the same forest as this will be pushed):
>
> ./java.base/unix/native/libjli/java_md_solinux.c
>
> void SetJavaLauncherPlatformProps() {
> /* Linux only */
> #ifdef __linux__
> const char *substr = "-Dsun.java.launcher.pid=";
> char *pid_prop_str = (char *)JLI_MemAlloc(JLI_StrLen(substr) +
> MAX_PID_STR_SZ + 1);
> sprintf(pid_prop_str, "%s%d", substr, getpid());
> AddOption(pid_prop_str, NULL);
> #endif /* __linux__ */
> }
>
>
>
>> 2) os::Linux::gettid(): Linux uses the kernel thread id for OSThread
>> thread
>> id - I did not change that. But by now the system call gettid should be
>> available in every Linux kernel, so I removed the fallback handling.
>>
>> 3) A number of changes dealt with the way LinuxThreads allocated thread
>> stacks (with mmap and the MAP_GROWSDOWN flag). On LinuxThreads, it
>> was not
>> possible to change the size of thread stacks (to start a new thread
>> with a
>> different stack size). NPTL can do this and all the places where we
>> dealt
>> with fixed stacks I changed.
>>
>> 4) On LinuxThreads, there is the problem that the thread stacks - which
>> grow down and are allocated with MAP_FIXED - may at some point trash the
>> java heap. To prevent this, every allocation done with
>> os::reserve_memory()
>> and friends recorded a "_highest_vm_reserved_address". There was a
>> function
>> called "_thread_safety_check" which prevented start of new threads if
>> thread stacks came dangerously close to the highest reserved address + a
>> gap; latter gap could be set via parameter ThreadSafetyMargin.
>>
>> All this coding was removed. Note that this coding probably was already
>> broken since a long time, because there are many allocations which
>> were not
>> tracked.
>>
>> 5) Recognition of glibc version and pthread library version were very
>> elaborate to deal with recognition of LinuxThreads implementation. Those
>> were dumbed down and now just assert in the highly unlikely case that we
>> encounter a LinuxThreads implementation.
>>
>> The rest of the stuff is more straight-forward.
>>
>> ---
>>
>> I built the changes on Linux x64, x86 and ppc64 and ran jtreg tests
>> (hotspot/test) on these platforms too. I did not see any issues, but I'd
>> like to get a couple of reviews for this.
>>
>> Kind Regards,
>> Thomas
>>
More information about the hotspot-runtime-dev
mailing list