<font size=2 face="sans-serif">Hi, Thomas, and David,</font><br><br><font size=2 face="sans-serif">Thank you for your comments.</font><br><br><font size=2 face="sans-serif">> I think Hiroshi thinks that since
the work stealing itself does a CAS</font><br><font size=2 face="sans-serif">> with barrier after obtaining "new_obj"
in the other thread, it should</font><br><font size=2 face="sans-serif">> be safe (for other threads consuming
an object on the task queue).</font><br><br><font size=2 face="sans-serif">Thank you. What Thomas thankfully explain
is that I wanted to mention why relaxed CAS is available for copy_to_survivor.</font><br><br><font size=2 face="sans-serif">> I also do not think it is safe
as is - for example, at least</font><br><font size=2 face="sans-serif">> PSPromotionManager::copy_and_push_safe_barrier()
reads data from the</font><br><font size=2 face="sans-serif">> returned new_obj (in another log
message :)) regardless of failure.</font><br><font size=2 face="sans-serif">> </font><br><font size=2 face="sans-serif">> That method also reads the forwardee
if forwarded, and then again uses</font><br><font size=2 face="sans-serif">> object information in that same
log message. A quick look did not show</font><br><font size=2 face="sans-serif">> other issues, but don't count this
as a review.</font><br><br><font size=2 face="sans-serif">Thank you for your comments.</font><br><br><font size=2 face="sans-serif">As Carsten suggested, I guess, size
may not be necessary for logging when CAS is failed (the size will be logged
by the other thread that successfully operates the CAS). By reducing printing
a size of new_obj, relaxing CAS for forwarding pointers becomes safe, I
believe.</font><br><br><font size=2 face="sans-serif">In my understanding, PSPromotionManager::copy_and_push_safe_barrier()
updates a card table for new_obj. However, this new_obj will not be used
fro card tables in the same GC as a root of GC because all of entries in
card tables were registered as tasks before any calls of copy_and_push_safe_barrier.</font><br><br><font size=2 face="sans-serif">I created a new webrev that reduces
print formats when CAS is failed. Could you review this and give comments
on it?</font><br><a href=http://cr.openjdk.java.net/~horii/8154736/webrev.00/><font size=2 color=blue face="sans-serif">http://cr.openjdk.java.net/~horii/8154736/webrev.00/</font></a><br><br><font size=2 face="sans-serif">Regards,<br>Hiroshi<br>-----------------------<br>Hiroshi Horii, Ph.D.<br>IBM Research - Tokyo<br></font><br><br><tt><font size=2>Thomas Schatzl <thomas.schatzl@oracle.com> wrote
on 09/30/2016 21:02:31:<br><br>> From: Thomas Schatzl <thomas.schatzl@oracle.com></font></tt><br><tt><font size=2>> To: David Holmes <david.holmes@oracle.com>,
Hiroshi H Horii/Japan/IBM@IBMJP</font></tt><br><tt><font size=2>> Cc: hotspot-compiler-dev <hotspot-compiler-dev@openjdk.java.net>,
<br>> Tim Ellison <Tim_Ellison@uk.ibm.com>, Michihiro Horie/Japan/<br>> IBM@IBMJP, "ppc-aix-port-dev@openjdk.java.net" <ppc-aix-port-<br>> dev@openjdk.java.net>, "hotspot-gc-dev@openjdk.java.net"
<hotspot-<br>> gc-dev@openjdk.java.net>, "hotspot-runtime-dev@openjdk.java.net"
<br>> <hotspot-runtime-dev@openjdk.java.net></font></tt><br><tt><font size=2>> Date: 09/30/2016 21:04</font></tt><br><tt><font size=2>> Subject: Re: RFR(M): 8154736: enhancement of
cmpxchg and <br>> copy_to_survivor for ppc64</font></tt><br><tt><font size=2>> <br>> Hi,<br>> <br>> On Fri, 2016-09-30 at 21:12 +1000, David Holmes wrote:<br>> > On 30/09/2016 8:17 PM, Hiroshi H Horii wrote:<br>> > > <br>> > > Dear David, and Dan,<br>> > > <br>> > > Thank you for your comments.<br>> > > <br>> > > > <br>> > > > In<br>> > > > hotspot/src/share/vm/gc/parallel/psPromotionManager.inline.hpp:<br>> > > > 266 the log line reads data from the forwardee even
when the CAS<br>> > > > fails. I believe those reads will be unsafe without
barriers<br>> > > > after<br>> > > > the copy of the content of the object.<br>> > > > hotspot/src/share/vm/gc/parallel/psPromotionManager.inline.hpp:28<br>> > > > 8<br>> > > > same problem as in line 266<br>> > > Can we use o->size() or new_obj_size instead of new_obj->size()?<br>> <br>> They are not equivalent. Parallel GC and other collectors creatively<br>> reuse the "length" field of objArrays to indicate progress
in the<br>> scanning them during GC.<br>> <br>> new_obj_size is the result of a call to o->size() (and the compiler
may<br>> redo computations at any point), so has the same issue.<br>> <br>> > > > If you feel that the use of new_obj->size() is potentially
unsafe<br>> > > > then<br>> > > > the fact we return new_obj means that any use of new_obj
by the<br>> > > > caller<br>> > > > may also potentially be unsafe.<br>> > > In my understanding, while copying objects to a survivor
space, if<br>> > > a thread creates a new_obj and sets a pointer with CAS,
the other<br>> > > threads can touch the new_obj after the thread calls<br>> > > push_contents(new_obj) (Line: 239). In push_contents,<br>> > > OrderAccess::release_store is called before pushing the
object as a<br>> > > task into a deque of workstealing (taskqueue.inline.hpp).
If the<br>> > > other thread reads the task, all of copy for new_obj is
safe.<br>> > I'm not familiar with the larger picture of the GC protocols
here,<br>> > but just looking at this code fragment in isolation if the CAS
fails<br>> > we read o->forwardee() to set new_obj. That in itself is fine
because<br>> > we're reading the field that we were testing with the CAS. But
we<br>> > could then deference new_obj before the thread that won the CAS
calls<br>> > push_contents; and even if it is after push_contents we have
not done<br>> > an acquire to pair with the release-store in push_contents.<br>> <br>> I think Hiroshi thinks that since the work stealing itself does a
CAS<br>> with barrier after obtaining "new_obj" in the other thread,
it should<br>> be safe (for other threads consuming an object on the task queue).<br>> <br>> > So I'm really not seeing how we can use a barrier-less CAS here.<br>> <br>> I also do not think it is safe as is - for example, at least<br>> PSPromotionManager::copy_and_push_safe_barrier() reads data from the<br>> returned new_obj (in another log message :)) regardless of failure.<br>> <br>> That method also reads the forwardee if forwarded, and then again
uses<br>> object information in that same log message. A quick look did not
show<br>> other issues, but don't count this as a review.<br>> <br>> Thanks,<br>>   Thomas<br>> <br></font></tt><BR>