Re: 8242480: Negative value may be returned by getFreeSwapSpaceSize() in the docker(Internet mail)

[傅杰]

Hi Severin,

Thanks for your review and very nice suggestions.

Updated: http://cr.openjdk.java.net/~jiefu/8242480/webrev.01/

test/hotspot/jtreg/containers/docker/TestGetFreeSwapSpaceSize.java is added to 
reproduce the bug.

Thanks a lot.
Best regards,
Jie


On 2020/4/16, 4:40 PM, "Severin Gehwolf" <sgehw...@redhat.com> wrote:

    Hi Jie,
    
    On Fri, 2020-04-10 at 01:49 +0000, jiefu(傅杰) wrote:
    > Hi all,
    >  
    > JBS:    https://bugs.openjdk.java.net/browse/JDK-8242480
    > Webrev: http://cr.openjdk.java.net/~jiefu/8242480/webrev.00/
    >  
    > Negative values were returned by getFreeSwapSpaceSize() in our docker 
testing.
    > The reason is that current implementation doesn't consider the situation 
when memory.limit_in_bytes == memory.memsw.limit_in_bytes, which means do not 
use the swap space at all.
    >  
    > In 
src/jdk.management/unix/classes/com/sun/management/internal/OperatingSystemImpl.java,
 let's see
    > ------------------------------------------------
    >  71     public long getFreeSwapSpaceSize() {
    >  72         if (containerMetrics != null) {
    >  73             long memSwapLimit = 
containerMetrics.getMemoryAndSwapLimit();
    >  74             long memLimit = containerMetrics.getMemoryLimit();
    >  75             if (memSwapLimit >= 0 && memLimit >= 0) {
    >  76                 for (int attempt = 0; attempt < MAX_ATTEMPTS_NUMBER; 
attempt++) {
    >  77                     long memSwapUsage = 
containerMetrics.getMemoryAndSwapUsage();
    >  78                     long memUsage = containerMetrics.getMemoryUsage();
    >  79                     if (memSwapUsage > 0 && memUsage > 0) {
    >  80                         // We read "memory usage" and "memory and 
swap usage" not atomically,
    >  81                         // and it's possible to get the negative 
value when subtracting these two.
    >  82                         // If this happens just retry the loop for a 
few iterations.
    >  83                         if ((memSwapUsage - memUsage) >= 0) {
    >  84                             return memSwapLimit - memLimit - 
(memSwapUsage - memUsage);
    >  85                         }
    >  86                     }
    >  87                 }
    >  88             }
    >  89         }
    >  90         return getFreeSwapSpaceSize0();
    >  91     }
    > ------------------------------------------------
    > If memSwapLimit (@line 73) equals memLimit (@line 74), then 
getFreeSwapSpaceSize() may return a negative value @line 84.
    >  
    > It would be better to fix it.
    > Could you please review it and give me some advice?
    
    Would this be reproducible via a test? There is
    test/hotspot/jtreg/containers/docker/TestMemoryAwareness.java which
    contains testOperatingSystemMXBeanAwareness() tests.
    
    It would be good to capture this in a test somehow.
    
    Thanks,
    Severin