Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v12]

2022-05-03 Thread Severin Gehwolf
On Tue, 3 May 2022 09:21:11 GMT, xpbob  wrote:

>> set memory.swappiness to 0,swap space will not be used 
>> determine the value of memory.swappiness
>> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
>> 
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 100.00M
>> Maximum Processes Limit: 4194305 
>> 
>> =>
>> 
>> Memory Limit: 50.00M
>> Memory Soft Limit: Unlimited
>> Memory & Swap Limit: 50.00M
>> Maximum Processes Limit: 4194305
>
> xpbob has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   remove no use copy

To be honest, the original test change was closer to what I was expecting to 
see. Could you clarify why we'd need to use `cgcreate` et.al. for this test? As 
far as I'm aware `--memory-swappiness` should work just as well.

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 150:

> 148: GET_CONTAINER_INFO(julong, _memory->controller(), 
> "/memory.swappiness",
> 149:"Swappiness is: " JULONG_FORMAT, JULONG_FORMAT, 
> swappiness);
> 150: if(swappiness == 0) {

Style: Space before `(`

src/hotspot/os/linux/cgroupV1Subsystem_linux.cpp line 152:

> 150: if(swappiness == 0) {
> 151:   jlong memlimit = read_memory_limit_in_bytes();
> 152:   log_trace(os, container)("Memory and Swap Limit has been reset to 
> " JULONG_FORMAT " because of Swappiness is 0", memlimit);

`s/because of Swappiness is 0/because swappiness is 0/g`

test/hotspot/jtreg/containers/docker/TestCgroupV1Memory.java line 58:

> 56: try {
> 57: testMemoryLimit();
> 58: testMemoryLimitWithSwappiness();

Only `testMemoryLimitWithSwappiness()` should be `cgroupv1` conditional.

test/lib/jdk/test/lib/containers/cgroup/CgroupV1TestUtils.java line 34:

> 32: 
> 33: 
> 34: public class CgroupV1TestUtils {

I'm not sure what the point of this class is. Wouldn't we be able to test the 
same thing with the existing docker infrastructure using 
`--memory-swappiness=0` as docker option? As far as I can see this utility adds 
a dependency in `libcgroup` and related tools, which isn't necessarily 
installed on test systems. Docker dep, on the other hand, is enforced via 
`@require` tag.

-

Changes requested by sgehwolf (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8285


Re: RFR: 8284950: CgroupV1 detection code should consider memory.swappiness [v12]

2022-05-03 Thread xpbob
> set memory.swappiness to 0,swap space will not be used 
> determine the value of memory.swappiness
> https://www.kernel.org/doc/Documentation/cgroup-v1/memory.txt
> 
> 
> Memory Limit: 50.00M
> Memory Soft Limit: Unlimited
> Memory & Swap Limit: 100.00M
> Maximum Processes Limit: 4194305 
> 
> =>
> 
> Memory Limit: 50.00M
> Memory Soft Limit: Unlimited
> Memory & Swap Limit: 50.00M
> Maximum Processes Limit: 4194305

xpbob has updated the pull request incrementally with one additional commit 
since the last revision:

  remove no use copy

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8285/files
  - new: https://git.openjdk.java.net/jdk/pull/8285/files/97a547f4..ed2f2855

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8285=11
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8285=10-11

  Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8285.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8285/head:pull/8285

PR: https://git.openjdk.java.net/jdk/pull/8285