<div>
Bengt,
</div><div><br></div><div>Good catch! The fix looks good to me.</div><div><br></div><div><div>igor</div><div><br></div></div>
<p style="color: #A0A0A8;">On Friday, March 23, 2012 at 5:56 AM, Bengt Rutisson wrote:</p>
<blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;">
<span><div><div><div><br></div><div>Hi all,</div><div><br></div><div>Could I have a couple of reviews for this change? This is on the </div><div>critical-watch list for hs23, so it would be great if I could get some </div><div>reviews already today.</div><div><br></div><div><a href="http://cr.openjdk.java.net/~brutisso/7103665/webrev.00/">http://cr.openjdk.java.net/~brutisso/7103665/webrev.00/</a></div><div><br></div><div>Some background:</div><div><br></div><div>There are two problems that this fix solves:</div><div><br></div><div>(1) When we run with -XX:+UseNUMA we have to make the heap parsable </div><div>since we leave some holes in the heap. We do this by faking Integer </div><div>arrays in those places. However, on a large heap those holes can be </div><div>larger than what can be covered by a single Integer array. The fix is to </div><div>allocate several arrays in those cases.</div><div><br></div><div>The easy fix would have been to call CollectedHeap::fill_with_objects() </div><div>instead of CollectedHeap::fill_with_object(). Note the extra "s" in the </div><div>method name. But MutableNUMASpace::ensure_parsability(), where the </div><div>change is needed, need to know about all the objects that are allocated </div><div>so I saw no better solution than to implement my own loop. Any ideas on </div><div>how I could re-use fill_with_objects() instead are welcome.</div><div><br></div><div>(2) CollectedHeap::_filler_array_max_size should be the maximum size </div><div>that can be covered by a fake Integer array. This value is in words, but </div><div>since the word size is different on different platforms but the Integer </div><div>size is always the same we need to convert between word and sizeof(jint) </div><div>at some point. Unfortunately we do the conversion in the wrong </div><div>direction, which means that on 64 bit platforms we end up with a max </div><div>size that is 4 times larger than intended. This in turn makes us </div><div>overflow an int when we convert from a word size to the length of the </div><div>array that we are setting up.</div><div><br></div><div>Here is the code that overflows:</div><div><br></div><div> 328 void</div><div> 329 CollectedHeap::fill_with_array(HeapWord* start, size_t words, bool </div><div>zap)</div><div> 330 {</div><div> 331 assert(words >= filler_array_min_size(), "too small for an array");</div><div> 332 assert(words <= filler_array_max_size(), "too big for a single </div><div>object");</div><div> 333</div><div> 334 const size_t payload_size = words - filler_array_hdr_size();</div><div> 335 const size_t len = payload_size * HeapWordSize / sizeof(jint);</div><div> 336</div><div> 337 // Set the length first for concurrent GC.</div><div> 338 ((arrayOop)start)->set_length((int)len);</div><div> 339 post_allocation_setup_common(Universe::intArrayKlassObj(), </div><div>start, words);</div><div> 340 DEBUG_ONLY(zap_filler_array(start, words, zap);)</div><div> 341 }</div><div><br></div><div>If filler_array_max_size() on line 332, which returns </div><div>CollectedHeap::_filler_array_max_size, has a too large value we will </div><div>overflow the int conversation on line 338.</div><div><br></div><div>Testing:</div><div>This fix solves the issue that was found in the reproducer that I could </div><div>set up on a Linux x64 machine. I have asked the one who initially </div><div>reported the issue to run on their Solaris amd64 setup. I will also try </div><div>to set up a reproducer on a Solaris machine.</div><div><br></div><div>I also ran the fix through JPRT. It did not fail, but there are no NUMA </div><div>tests in there as far as I know. But the change for issue (2) was </div><div>hopefully tested.</div><div><br></div><div>Thanks,</div><div>Bengt</div></div></div></span>
</blockquote>
<div>
<br>
</div>