Re: [pve-devel] [PATCH qemu-server v4 2/4] vmstatus: add memhost for host view of vm mem consumption

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 29.07.25 um 14:50 schrieb Lukas Wagner:
> On Sat Jul 26, 2025 at 3:06 AM CEST, Aaron Lauterer wrote:
>> +        my $fh = IO::File->new("/sys/fs/cgroup/qemu.slice/${vmid}.scope/cgroup.procs", "r");
>> +        if ($fh) {
>> +            while (my $childPid = <$fh>) {
>> +                chomp($childPid);
> 
> nit: should be snake_case

+1 and go into it's own method, I moved it in a follow-up

> 
>> +                open(my $SMAPS_FH, '<', "/proc/$childPid/smaps_rollup")
>> +                    or die "failed to open PSS memory-stat from process - $!\n";
>> +
>> +                while (my $line = <$SMAPS_FH>) {
>> +                    if ($line =~ m/^Pss:\s+([0-9]+) kB$/) {
>> +                        $d->{memhost} = $d->{memhost} + int($1) * 1024;
> 
> Why do you sum up $d->{memhost} with the thing you just read from /proc?
> As far as I can tell memhost should always be zero at this point....

it's a += and there are two loops, basically it sums up all Pss stats for
every process in a cgroup to get a somewhat correct total memory consumption
of that cgroup.

> 
> 
>> +                        last;
> 
> ... and also you break here from the loop, so even if there were two Pss
> lines, you wouldn't add them up.
> 
> Am I missing something? :)

This is smaps_rollup, not smaps, i.e. the former is a a summed up variant
of all separate mappings from the latter, so there is always only one Pss
entry in there.

See (need to search twice for "smaps_rollup", there is no better link anchor
in the vicinity):

https://docs.kernel.org/filesystems/proc.html#process-specific-subdirectories

> 
>> +                    }
>> +                }
>> +                close $SMAPS_FH;
>> +            }
>> +        }
>> +        close($fh);
>> +
>>          my $pressures = PVE::ProcFSTools::read_cgroup_pressure("qemu.slice/${vmid}.scope");
>>          $d->{pressurecpusome} = $pressures->{cpu}->{some}->{avg10} * 1;
>>          $d->{pressurecpufull} = $pressures->{cpu}->{full}->{avg10} * 1;
>> @@ -2707,7 +2732,6 @@ sub vmstatus {
>>          } else {
>>              $d->{cpu} = $old->{cpu};
>>          }
>> -
>>      }
>>  
>>      return $res if !$full;
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel