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

Reply via email to