<html>
  <head>
    <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Tao,<br>
    <br>
    This looks OK to me.<br>
    <br>
    I can see what Thomas is saying though. All of the checks involve
    something like:<br>
    <br>
    total += some_value;<br>
    if (total < some_value) {<br>
      // We must have overflowed<br>
      vm_exit(...);<br>
    }<br>
    <br>
    So a function in CollectedHeap (the base class of SharedHeap and
    ParallelScavengeHeap) might make sense. For example:<br>
    <br>
    total_reserved = 0;<br>
    total_reserved = add_and_verify_no_overflow(total_reserved,
    max_heap_size);<br>
    total_reserved = add_and_verify_no_overflow(total_reserved,
    max_perm_size);<br>
    <br>
    Where the function is:<br>
    <br>
    size_t add_and_verify_no_overflow(size_t x, size_t y) {<br>
      const char* msg = "...";<br>
      x += y;<br>
      if (x < y) {<br>
        vm_exit(msg);<br>
      }<br>
      return x;<br>
    }<br>
    <br>
    But I can live with your current changes.<br>
    <br>
    JohnC<br>
    <br>
    <div class="moz-cite-prefix">On 4/10/2013 9:52 AM, Tao Mao wrote:<br>
    </div>
    <blockquote cite="mid:516598D3.1020809@oracle.com" type="cite">
      <meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
      Hi Bengt,<br>
      <br>
      Thank you for reviewing. A new webrev is updated.<br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Etamao/6761744/webrev.01/">http://cr.openjdk.java.net/~tamao/6761744/webrev.01/</a><br>
      <br>
      Cheers,<br>
      Tao<br>
      <br>
      On 4/10/13 1:54 AM, Bengt Rutisson wrote:
      <blockquote cite="mid:516528C2.8080709@oracle.com" type="cite">
        <meta content="text/html; charset=UTF-8"
          http-equiv="Content-Type">
        <div class="moz-cite-prefix"><br>
          Hi Tao,<br>
          <br>
          This change looks good. Thanks for adding the JTReg test, it
          looks good!<br>
          <br>
          One minor nit:<br>
          <br>
          In src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp I
          would suggest to
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          store "(size_t) align_size_up(pgs->max_size(),
          HeapRegion::GrainBytes)" in a local variable rather than
          duplicating the calculation.<br>
          <br>
          Thanks,<br>
          Bengt<br>
          <br>
          On 4/8/13 7:22 AM, Tao Mao wrote:<br>
        </div>
        <blockquote cite="mid:51625424.4050209@oracle.com" type="cite">
          <meta http-equiv="content-type" content="text/html;
            charset=UTF-8">
          6761744 Hotspot crashes if process size limit is exceeded<br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="https://jbs.oracle.com/bugs/browse/JDK-6761744">https://jbs.oracle.com/bugs/browse/JDK-6761744</a><br>
          <br>
          webrev:<br>
          <a moz-do-not-send="true" class="moz-txt-link-freetext"
            href="http://cr.openjdk.java.net/%7Etamao/6761744/webrev.00/">http://cr.openjdk.java.net/~tamao/6761744/webrev.00/</a><br>
          <br>
          changeset:<br>
          The fix only needs to go to hsx24 since there's no perm gen in
          hotspot-25. Thus, the webrev is based on hsx24 repo.<br>
          <br>
          It provides for 32-bit builds a preventive check of the size
          of "the object heap + perm gen" before reserving VM memory.
          The total size should not exceed 4096MB (i.e. 4GB) for 32-bit
          builds; otherwise, the total doesn't make sense and, what's
          worse, overflow occurs. It will consequentially trigger anther
          error of memory access violation, which was not protected<span
            style="color: rgb(0, 0, 0); font-family: Arial, FreeSans,
            Helvetica, sans-serif; font-size: 12.727272033691406px;
            font-style: normal; font-variant: normal; font-weight:
            normal; letter-spacing: normal; line-height:
            15.454545021057129px; orphans: auto; text-align: start;
            text-indent: 0px; text-transform: none; white-space: normal;
            widows: auto; word-spacing: 0px; -webkit-text-size-adjust:
            auto; -webkit-text-stroke-width: 0px; background-color:
            rgb(240, 240, 240); display: inline !important; float:
            none;"></span>.<br>
          <br>
          jtreg testing java code is also written, checking both 32-bit
          and (trivially) 64-bit builds.<br>
          <br>
          testing:<br>
          check jtreg tests with flags -XX:+UseParallelGC, -XX:+UseG1GC,
          -XX:+UseParNewGC, -XX:+UseConcMarkSweepGC, -XX:+UseSerialGC
          and builds of 32-bit and 64-bit. All passed.<br>
          <br>
          Needs JPRT test when pushing.<br>
        </blockquote>
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>