[pve-devel] [PATCH common] ProcFSTools: read_proc_stat: add more cpu stats from /proc/stat

[pve-devel] [PATCH common] ProcFSTools: read_proc_stat: add more cpu stats from /proc/stat

[Dominik Csapak]

those fields might be interesting to users. At the moment, this is
only used in the external metrics export.

These fields exist in the kernel since:
* irq - 2.6.0
* softirq - 2.6.0
* steal - 2.6.11
* guest - 2.6.24
* guest_nice - 2.6.33

so they should all exist

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/PVE/ProcFSTools.pm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/PVE/ProcFSTools.pm b/src/PVE/ProcFSTools.pm
index ff30e4b..8309ced 100644
--- a/src/PVE/ProcFSTools.pm
+++ b/src/PVE/ProcFSTools.pm
@@ -168,13 +168,18 @@ sub read_proc_stat {
 
     if (my $fh = IO::File->new ("/proc/stat", "r")) {
 	while (defined (my $line = <$fh>)) {
-	    if ($line =~ m|^cpu\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s|) {
+	    if ($line =~ m|^cpu\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(?:(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+))?|) {
 		$res->{user} = $1;
 		$res->{nice} = $2;
 		$res->{system} = $3;
 		$res->{idle} = $4;
 		$res->{used} = $1+$2+$3;
 		$res->{iowait} = $5;
+		$res->{irq} = $6 if defined($6);
+		$res->{softirq} = $7 if defined($7);
+		$res->{steal} = $8 if defined($8);
+		$res->{guest} = $9 if defined($9);
+		$res->{guest_nice} = $10 if defined($10);
 	    } elsif ($line =~ m|^cpu\d+\s|) {
 		$cpucount++;
 	    }
-- 
2.30.2

Re: [pve-devel] [PATCH common] ProcFSTools: read_proc_stat: add more cpu stats from /proc/stat

[Thomas Lamprecht]

On 27.07.21 09:17, Dominik Csapak wrote:
> those fields might be interesting to users. At the moment, this is
> only used in the external metrics export.
> 
> These fields exist in the kernel since:
> * irq - 2.6.0
> * softirq - 2.6.0
> * steal - 2.6.11
> * guest - 2.6.24
> * guest_nice - 2.6.33
> 
> so they should all exist

Why the define check then? We're strictly 5.4+ kernel support requirement wise
in all stable supported releases.

But yeah, actually that was planned since a bit; IIRC I talked with wobu when I
added those to PBS[0], not sure what happened then though, probably just lost track
of that.

[0]: https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox/src/sys/linux/procfs/mod.rs;h=5784e0e885d2c5b32d9acb6f63b7785cb8031995;hb=refs/heads/master#l1891

but def. makes sense, the `total` stat one should include some (adding the `guest*`
ones could be wrong in general) of those too though, the PBS one[1] can be used here too.

[1]: https://git.proxmox.com/?p=proxmox.git;a=blob;f=proxmox/src/sys/linux/procfs/mod.rs;h=5784e0e885d2c5b32d9acb6f63b7785cb8031995;hb=refs/heads/master#l337


> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/PVE/ProcFSTools.pm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/ProcFSTools.pm b/src/PVE/ProcFSTools.pm
> index ff30e4b..8309ced 100644
> --- a/src/PVE/ProcFSTools.pm
> +++ b/src/PVE/ProcFSTools.pm
> @@ -168,13 +168,18 @@ sub read_proc_stat {
>  
>      if (my $fh = IO::File->new ("/proc/stat", "r")) {
>  	while (defined (my $line = <$fh>)) {
> -	    if ($line =~ m|^cpu\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s|) {
> +	    if ($line =~ m|^cpu\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(?:(\d+)\s+(\d+)\s+(\d+)\s+(\d+)\s+(\d+))?|) {
>  		$res->{user} = $1;
>  		$res->{nice} = $2;
>  		$res->{system} = $3;
>  		$res->{idle} = $4;
>  		$res->{used} = $1+$2+$3;
>  		$res->{iowait} = $5;
> +		$res->{irq} = $6 if defined($6);
> +		$res->{softirq} = $7 if defined($7);
> +		$res->{steal} = $8 if defined($8);
> +		$res->{guest} = $9 if defined($9);
> +		$res->{guest_nice} = $10 if defined($10);
>  	    } elsif ($line =~ m|^cpu\d+\s|) {
>  		$cpucount++;
>  	    }
>