RFR: 8300645: Revert julong changes from 8194232 after 8292083

Ioi Lam iklam at openjdk.org
Mon Feb 13 18:29:32 UTC 2023


On Mon, 13 Feb 2023 17:47:39 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Surely it can happen because `subsystem_file_line_contents` may successfully parse `-1` into a `jlong`? Assuming that the file can contain a negative number. I don't think I understand why we have the safeguard there otherwise.
>
> The files won't contain negative numbers, AFAIK. Only some random large numbers or the actual limit. `-2` is being returned by the macro if the file doesn't exist or some other error occurs (basically this case). Since this repeatedly causes confusion for me when I revisit this code, I've added the assert for clarity. When we reach line 94, we can be sure that we get a positive value. If you want the assert removed, that's fine with me as well. To me, having the assert there is clearer, though.

I think the `assert(memswlimit > 0, "invariant");` doesn't give a clear explanation why this is the case. And seeing multiple of this kind of assert in the same file is problematic.

Is it possible for `/memory.limit_in_bytes` to use a number that's larger than `max_jlong`? If so, `GET_CONTAINER_INFO(jlong, ... JLONG_FORMAT)` will fail and this function will return `OSCONTAINER_ERROR`. Previously, this function would return -1.

(the fact that `OSCONTAINER_ERROR` (-2) can be returned by this function is also problematic).

I actually think the old code is better. It can be further tightened by changing the blind (and potentially unsafe) cast


 return (jlong)hier_memswlimit;


to


 return as_positive_memory_limit(hier_memswlimit);


as_positive_memory_limit will assert that the value is not greater than `max_jlong`, which should be guaranteed by the above calculation. More explanations can be added as comments in `as_positive_memory_limit`.

-------------

PR: https://git.openjdk.org/jdk/pull/12166


More information about the hotspot-runtime-dev mailing list