Goetz,
On 14/12/2015 16:11, Lindenmaier, Goetz wrote:
Hi Frederic,
thanks for your fast reply.
From reading the code I guessed about what you explained, now
I understood more details, thanks!
I will not fiddle with handling the zones. My change is only about
handling larger pages, i.e. the sizes of the zones, so there is no
danger I will break the mechanism you implemented.
The take over is that the VM code should be able to manage
the reserved zone alone or the reserved zone and the yellow
zone together.
If you want to change it for a better name, more explicit,
I'm fine with that.
Yes, I would like to change it to 'yellow_reserved' wherever
both are handled at the same time.
There's places where red+yellow+reserved are handled as
one, here I will use 'guard_zone'. That's aliged with
create_stack_guard_pages() which guards all (red, yellow,
reserved) of them.
I believe that red+yellow+reserved are handled as one only
during stack initialization. Once configured, the only
moment when the red zone protection is changed is when the
VM is about to generate a crash dump.
Anyway, 'yellow_reserved' sounds good to me.
stack_yellow_zone_enabled() -> stack_guards_enabled()
Here, "guards" refers to the two guard zones: reserved
+ yellow.
Actually, it also includes that the red page is enabled, right?
One question, in os_linux.cpp, you meant AMD64, not Itanium, right?
27.18+#if defined(IA32) || defined(IA64)
I'll move that functionality to another place, so no need to fix it.
You're right, I meant AMD64.
Thanks,
Fred
-----Original Message-----
From: Frederic Parain [mailto:frederic.par...@oracle.com]
Sent: Montag, 14. Dezember 2015 15:44
To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; Karen Kinnear
<karen.kinn...@oracle.com>
Cc: hotspot-...@openjdk.java.net; core-libs-dev <core-libs-
d...@openjdk.java.net>
Subject: Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for Critical
Sections
Goetz,
The reserved zone is sometime considered as a subpart of
the yellow zone, and sometime as an independent entity.
Technically the reserved zone is placed just before the
yellow zone, but the way it is managed depends on the
context.
In most cases, there's no specially annotated methods on
the thread's call stack. In this configuration the
reserved zone is considered as being part of the yellow
zone. This means that the protection of the reserved
zone is always the same as the protection of the yellow
zone.
In some cases, a thread could have one or more methods
on its call stack with the ReservedStackAccess annotation.
In this configuration, the reserved zone is managed as
a separate entity. Initially protected like the yellow
zone, access to the reserved zone can be temporary granted
for the execution of some critical sections. This means
that the protection of the reserved zone can be removed
while the yellow zone is still protected.
The take over is that the VM code should be able to manage
the reserved zone alone or the reserved zone and the yellow
zone together. I've already renamed a method because of
this change:
stack_yellow_zone_enabled() -> stack_guards_enabled()
Here, "guards" refers to the two guard zones: reserved
+ yellow.
If you want to change it for a better name, more explicit,
I'm fine with that. We just have to preserve the different
operations required for stack overflow management.
Let's say R(x) and Y(x) represent the protection of
the reserved zone and the yellow zone respectively.
And let's say that x = P means "zone protected"
and x = G means "access granted"
The different configurations are:
(1) R(P) Y(P) => initial configuration
(2) R(G) Y(P) => first stack overflow with annotated
method on stack
(3) R(G) Y(G) => stack overflow without annotated
method on stack, or second stack
overflow with annotated method
on stack
And the transitions are:
(1) -> (3) -> (1)
or
(1) -> (2) -> (1)
or
(1) -> (2) -> (3) -> (1)
I hope this would clarify the semantic of the reserved zone.
If it doesn't, let me know, I'll try to explain it differently.
Thanks,
Fred
On 14/12/2015 14:59, Lindenmaier, Goetz wrote:
Hi Frederic,
I'm now again working on my change "8139864: Improve handling of stack
protection zones."
Coleen had asked me to wait until this change of yours is submitted.
You changed the stack_yellow_zone accessor functions in thread.hpp to
handle both zones, yellow and reserved.
Therefore, reading the code, the reserved zone seems to be part of the
yellow zone.
In the description of the bug, it says "The new zone defined by the
proposed
solution is placed just before the yellow zone." This reads as if the zones
are
separate.
Would you mind if I rename the stack_yellow_zone accessor functions to
stack_yellow_reserved_zone? This would make clear in the code that this
are two separate zones.
I anyways have to adapt most of the calls to these accessors.
Best regards,
Goetz.
-----Original Message-----
From: hotspot-dev [mailto:hotspot-dev-boun...@openjdk.java.net] On
Behalf Of Frederic Parain
Sent: Montag, 14. Dezember 2015 14:24
To: Karen Kinnear <karen.kinn...@oracle.com>
Cc: hotspot-...@openjdk.java.net; core-libs-dev <core-libs-
d...@openjdk.java.net>
Subject: Re: RFR(L): JDK-8046936 : JEP 270: Reserved Stack Areas for
Critical
Sections
Karen,
Thank you for your review.
Fred
On 23/11/2015 20:10, Karen Kinnear wrote:
Frederic,
Looks good.
Many thanks.
Karen
On Nov 23, 2015, at 12:44 PM, Frederic Parain
<frederic.par...@oracle.com <mailto:frederic.par...@oracle.com>>
wrote:
Karen,
Thank you for your review, my answers are in-lined below.
New Webrevs (including some fixes suggested by Paul Sandoz):
http://cr.openjdk.java.net/~fparain/8046936/webrev.01/hotspot/
http://cr.openjdk.java.net/~fparain/8046936/webrev.01/jdk/
On 20/11/2015 19:44, Karen Kinnear wrote:
Frederic,
Code review for web revs you sent out.
Code looks good. I am not as familiar with the compiler code.
I realize you need to check in at least a subset of the
java.util.concurrent changes for
the test to work, so maybe I should not have asked Doug about his
preference there.
Sorry.
I am impressed you found a way to make a reproducible test!
Comments/questions:
1. src/cpu/sparc/vm/interp_masm_sparc.cpp line 1144 “is” -> “if”
Fixed
2. IIRC, due to another bug with windows handling of our guard pages,
this
is disabled for Windows. Would it be worth putting a comment in the
bug , 8067946, that once this is fixed, the reserved stack logic on
windows
will need testing before enabling?
More than testing, the implementation would have to be completed
on
Windows. I've added a comment to JDK-8067946.
3. In get_frame_at_stack_banging_point on Linux, BSD and
solaris_x86 if
this is in interpreter code, you explicitly return the Java sender
of the current frame. I was expecting to see that on Solaris_sparc
and Windows
as well? I do see the assertion in caller that you do have a java frame.
It doesn't make sense to check the current frame (no bytecode has
been
executed yet, so risk of partially executed critical section). I did the
change but not for all platforms. This is now fixed for Solaris_SPARC
and Windows too.
4. test line 83 “writtent” -> “written”
Fixed
5. I like the way you set up the preallocated exception and then set
the message - we may
try that for default methods in future.
6. I had a memory that you had found a bug in safe_for_sender - did
you already check that in?
I've fixed x86 platforms in JDK-8068655.
I've piggybacked the SPARC fix to this webrev (frame_sparc.cpp:635),
I had hoped it would have been silently accepted :-)
7. I see the change in trace.xml, and I see an include added to
SharedRuntime.cpp,
but I didn’t see where it was used - did I just miss it?
trace.xml changes define a new event.
This event is created at sharedRuntime.cpp::3006 and it is used
in the next 3 lines.
Thanks. I must have mistyped when I searched for it.
Thanks,
Fred
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email:frederic.par...@oracle.com
<mailto:frederic.par...@oracle.com>
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
