255 looks good but any reason why not using MAXPATHLEN ?

Could define HEADER_FORMAT="0::" and strlen

Adding a comment that on cgroupv2 system it will return something like '/sys/fs/cgroup/system.slice/system-dirsrv.slice/dirsrv@standalone1.service'

Not expert at all on vgroup. I noticed in my tests that memory.max and memory.high were 'max' (not uint)

In case cg_mem* are unchanged because it is neither cgroup V1/V2. Is there fallback values ?

Is this possible ? If current vsz overpass the process limit, I would expect it exits at the next malloc.

Not expert at all on vgroup. I noticed in my tests that memory.max and memory.high were 'max' (not uint)

If these are max, then the _spal_uint64_t_file_get() call to sscanf will fail, causing dest to be left as 0, indicating no limit.

In case cg_mem* are unchanged because it is neither cgroup V1/V2. Is there fallback values ?

Yes, it remains at 0, so we bypass those checks.

Is this possible ? If current vsz overpass the process limit, I would expect it exits at the next malloc.

This is saying "if the softlimit has room given our current usage". So if the soft or hardlimit is say 1GB and we are using 256mb, then only 768mb is available, rather than the full gb. So these are actually checks to make sure that we don't underflow to a huge value of memory available, or over allocate based on the current cgroup usage.

So the logic is correct, and the final } else { here catches when usage > soft/hard limits, to give a warning about the condition.

CHanges to your other comments have been made :) thanks for the thorough review mate

"..now two files.." -> "..now three files.."

Something like this would be stronger check we are in CGv2
if ((strcmp(s, CG2_HEADER_FORMAT, CG2_HEADER_LEN) && (strlen(s) >= CG2_HEADER_LEN))...

It is now 20%

Not expert at all on vgroup. I noticed in my tests that memory.max and memory.high were 'max' (not uint)

If these are max, then the _spal_uint64_t_file_get() call to sscanf will fail, causing dest to be left as 0, indicating no limit.

In case cg_mem* are unchanged because it is neither cgroup V1/V2. Is there fallback values ?

Yes, it remains at 0, so we bypass those checks.

Is this possible ? If current vsz overpass the process limit, I would expect it exits at the next malloc.

This is saying "if the softlimit has room given our current usage". So if the soft or hardlimit is say 1GB and we are using 256mb, then only 768mb is available, rather than the full gb. So these are actually checks to make sure that we don't underflow to a huge value of memory available, or over allocate based on the current cgroup usage.
So the logic is correct, and the final } else { here catches when usage > soft/hard limits, to give a warning about the condition.

I agree the logic is correct. My remark was a question, how is it possible to have memory.usage > memory.soft_limit. I would expect some alloc failure if usage>soft_limit

If max/soft limit are 'max', then cg_mem_soft = cg_mem_hard = 0.
So we will get this wrong alarming message in error log.

@firstyear, beside those cosmetic comments the patch looks valid to me. You have my ACK

Something like this would be stronger check we are in CGv2
if ((strcmp(s, CG2_HEADER_FORMAT, CG2_HEADER_LEN) && (strlen(s) >= CG2_HEADER_LEN))...

True, that would be a better check, because I don't know if that header ever changes, it seems like the whole format is a bit undocumented :(

If max/soft limit are 'max', then cg_mem_soft = cg_mem_hard = 0.
So we will get this wrong alarming message in error log.

Yep, corrected by checking that soft or hard are non zero now.

I'll ask you to review again to be 100% sure given I changed a bit of logic in a few places @tbordaz

Thanks @firstyear , the patch looks good to me. Ack

Thanks for your very thorough review @tbordaz I really appreciate it!

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3938

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.