cgroups: change memory usage calculation to match Docker #2455
Conversation
This changes the memory usage calculation to follow Docker's approach: - For cgroup v2: memory.current - memory.stat['inactive_file'] - For cgroup v1: memory.usage_in_bytes - memory.stat['total_inactive_file'] The previous calculation used anonymous memory only, but Docker's calculation provides more accurate memory usage by excluding inactive file cache. Closes: containers#2454 Signed-off-by: Giuseppe Scrivano <[email protected]>
Reviewer's GuideThis PR refactors the cgroups memory usage calculation to align with Docker’s methodology by subtracting inactive file cache from total memory usage in both cgroup v2 and cgroup v1, replacing the previous anonymous‐only approach. Sequence diagram: cgroup v2 memory data retrievalsequenceDiagram
participant Handler as linuxMemHandler
participant FS as FileSystem
Handler->>FS: Read "memory.current"
FS-->>Handler: current_value
Handler->>FS: Read "inactive_file" from "memory.stat"
FS-->>Handler: inactive_file_value
Sequence diagram: cgroup v1 memory data retrievalsequenceDiagram
participant Handler as linuxMemHandler
participant FS as FileSystem
Handler->>FS: Read "memory.usage_in_bytes"
FS-->>Handler: usage_in_bytes_value
Handler->>FS: Read "total_inactive_file" from "memory.stat"
FS-->>Handler: total_inactive_file_value
Class diagram for modified linuxMemHandlerclassDiagram
class linuxMemHandler {
+Stat(ctr *CgroupControl, m *cgroups.Stats) error
}
Flow diagram for updated memory usage calculation logicgraph TD
A[Start: Stat function] --> B{Is ctr.cgroup2?};
B -- Yes --> C[memoryRoot = cgroup v2 path];
C --> D[Read 'memory.current' --> current_val];
D --> E[Read 'inactive_file' from 'memory.stat' --> inactiveFile_val];
E --> F{inactiveFile_val < current_val?};
F -- Yes --> G[memUsage.Usage.Usage = current_val - inactiveFile_val];
F -- No --> H[memUsage.Usage.Usage = 0];
G --> Z[Update m.Stats];
H --> Z;
B -- No --> I[memoryRoot = cgroup v1 path];
I --> J[Read 'memory.usage_in_bytes' --> usageInBytes_val];
J --> K[Read 'total_inactive_file' from 'memory.stat' --> totalInactiveFile_val];
K --> L{totalInactiveFile_val < usageInBytes_val?};
L -- Yes --> M[memUsage.Usage.Usage = usageInBytes_val - totalInactiveFile_val];
L -- No --> N[memUsage.Usage.Usage = 0];
M --> Z;
N --> Z;
Z --> End[Return error status];
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @giuseppe - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return err | ||
| } | ||
|
|
||
| // Docker calculation: memory.current - memory.stat['inactive_file'] |
There was a problem hiding this comment.
suggestion: Extract common memory usage logic into a helper
This calculation is repeated in both cgroup v1 and v2. Refactoring it into a shared utility function would improve maintainability and consistency.
Suggested implementation:
// Docker calculation: memory.current - memory.stat['inactive_file']
memUsage.Usage.Usage = calculateDockerMemUsage(current, inactiveFile) // Read memory.usage_in_bytes
usageInBytes, err := readFileAsUint64(filepath.Join(memoryRoot, "memory.usage_in_bytes"))func calculateDockerMemUsage(usage, inactiveFile uint64) uint64 {
if inactiveFile < usage {
return usage - inactiveFile
}
return 0
}You should also update the cgroup v1 code path to use calculateDockerMemUsage instead of duplicating the logic.
If the v1 code path also does memUsage.Usage.Usage = 0; if inactiveFile < usageInBytes { memUsage.Usage.Usage = usageInBytes - inactiveFile }, replace it with memUsage.Usage.Usage = calculateDockerMemUsage(usageInBytes, inactiveFile).
|
the difference with Docker is that when inactive_file is bigger than memory.current then 0 is returned instead of |
|
@containers/podman-maintainers PTAL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99, sourcery-ai[bot] The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM |
|
/lgtm |
3 participants
This changes the memory usage calculation to follow Docker's approach:
The previous calculation used anonymous memory only, but Docker's calculation provides more accurate memory usage by excluding inactive file cache.
Closes: #2454
Summary by Sourcery
Align cgroup memory usage calculation with Docker by subtracting inactive file caches for both v1 and v2
Enhancements: