Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meminfo: Add slab_reclaimable to MemAvailable #671

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

asainkujovic
Copy link
Contributor

@asainkujovic asainkujovic commented Jan 5, 2025

This PR proposal is created for the problem explained at issue 670

Slab-Reclaimable memory in any system should not be part of "used" but oposite, should be part of "available" memory.

So like said in issue, I am not an expert about all details of memory values, but hope that someone of lxcfs creators can confirm that this is logical needed change.

Kernel send us 'memory.current' file for the cgroup we are inside, witch we (lxcfs code) use it as base 'memused' value.
For calculating MemAvailable we subtracting memused from memorymax, and already we are adding to it total_active/inactive_file values as reclaimable ones. Slab-reclaimable should be added here too (or we will have unreal growth of memory, like explained in connected issue)

@stgraber stgraber requested review from mihalicyn and hallyn January 5, 2025 16:33
@@ -1389,7 +1398,8 @@ static int proc_meminfo_read(char *buf, size_t size, off_t offset,
snprintf(lbuf, 100, "MemFree: %8" PRIu64 " kB\n", memlimit - memusage);
printme = lbuf;
} else if (startswith(line, "MemAvailable:")) {
snprintf(lbuf, 100, "MemAvailable: %8" PRIu64 " kB\n", memlimit - memusage + (mstat.total_active_file + mstat.total_inactive_file) / 1024);
snprintf(lbuf, 100, "MemAvailable: %8" PRIu64 " kB\n", memlimit - memusage +
(mstat.total_active_file + mstat.total_inactive_file + mstat.slab_reclaimable) / 1024);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@mihalicyn mihalicyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Hi @asainkujovic,

thanks for your work on this one!

Kind regards,
Alex

@asainkujovic
Copy link
Contributor Author

asainkujovic commented Jan 6, 2025

This PR as it is should be better option than current situation. And I still proposing it as it will solve mentioned issue.
However, full story (as all of you already know) in kernel code MemAvail calculation, is that kernel use watermark calclulated from memory zones, and makes estimation where half of reclaimable values (or memory size as watermark value) stays as reserved. So for completeness i would sum up my view of situation by following pseudocode:

    # init variables for MemAvail calculation:
    memlimit        = file_contents(/sys/fs/cgroup/memory.max)
    memusage        = file_contents(/sys/fs/cgroup/memory.current)
    // low_watermark inside lxc container probably can not be read well from somewhere
    // there is memory.peak file exposed as watermark but that is not watermark that we need
    // kernel calculates it by looping through ZONES, we can collect that info too, but it is global value
    // for whole host (not separated per cgroups)
    watermark       = file_contents(some way to get this value, maybe by adding 'grep low /proc/zoneinfo)
    slab_recl       = file_line_val(/sys/fs/cgroup/memory.stat, line:slab_reclaimable)
    kern_recl       = file_line_val(we do not have this value exposed per cgroup, just global one)
    total_recl      = slab_recl + kern_recl
    total_file      = file_line_val(/sys/fs/cgroup/memory.stat, line:active_file)
                     +file_line_val(/sys/fs/cgroup/memory.stat, line:inactive_file)

    estimated_file  = total_file - min(total_file/2, watermark)
    estimated_recl  = slab_recl  - min(total_recl /2, watermark)
    
    // (1) kernel code calculate something like this:
    MemAvail = memlimit - memusage + estimated_file + estimated_recl

    // (2) current LXCFS version calculates MemAvail like this:
    MemAvail = memlimit - memusage + total_file

    // (3) this PR of lxcfs calculates it like this:
    MemAvail = memlimit - memusage + total_file + slab_recl

So solution (3) is still better than (2) and currently best option, so I am still proposing this PR as it is, as a needed solution. We are not including kern_reclaimable but also do not spliting sreclaimable by half, so that estimation good as we do not have all source values that host's kernel has.

Thanks for the review and approval.

@stgraber stgraber merged commit 19e2c99 into lxc:main Jan 7, 2025
21 checks passed
@asainkujovic asainkujovic deleted the excludesrecl branch January 7, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants