Re: [pve-devel] [PATCH v2 common 1/1] fix #1454: meminfo: also return arcsize

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

Am 28/03/2023 um 14:49 schrieb Matthias Heiserer:
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
> Changes from v1:
> None
> 
>  src/PVE/ProcFSTools.pm | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/PVE/ProcFSTools.pm b/src/PVE/ProcFSTools.pm
> index eb4b0f5..0686c34 100644
> --- a/src/PVE/ProcFSTools.pm
> +++ b/src/PVE/ProcFSTools.pm
> @@ -283,6 +283,7 @@ sub read_meminfo {
>  	swaptotal => 0,
>  	swapfree => 0,
>  	swapused => 0,
> +	arcsize => 0,
>      };
>  
>      my $fh = IO::File->new ("/proc/meminfo", "r");
> @@ -307,6 +308,11 @@ sub read_meminfo {
>      my $spages = PVE::Tools::file_read_firstline("/sys/kernel/mm/ksm/pages_sharing") // 0 ;
>      $res->{memshared} = int($spages) * 4096;
>  
> +    my $arcstats = PVE::Tools::file_get_contents("/proc/spl/kstat/zfs/arcstats");

file_get_contents dies if the file isn't there, which might happen e.g., on PMG or if
one disables ZFS – why ever that would be.

so rather do:

my $arcstats = eval { PVE::Tools::file_get_contents("/proc/spl/kstat/zfs/arcstats") };

(just ignore the error, alternatively we could set a flag on ENOENT to avoid querying that
the next time, but that would be just an optimization).

> +    if ($arcstats && $arcstats =~ m/size\s+\d+\s+(\d+)/m) {

this match is rather brittle as there are multiple lines with size, e.g:

grep -P 'size\s+\d+\s+(\d+)' /proc/spl/kstat/zfs/arcstats
size                            4    5356816
compressed_size                 4    221696
uncompressed_size               4    5079552
overhead_size                   4    4947968
hdr_size                        4    29520
data_size                       4    0
metadata_size                   4    5169664
dbuf_size                       4    38016
dnode_size                      4    93248
bonus_size                      4    14080
anon_size                       4    0
mru_size                        4    2696704
mru_ghost_size                  4    0
mfu_size                        4    2472960
mfu_ghost_size                  4    0
l2_prefetch_asize               4    0
l2_mru_asize                    4    0
l2_mfu_asize                    4    0
l2_bufc_data_asize              4    0
l2_bufc_metadata_asize          4    0
l2_size                         4    0
l2_asize                        4    0
l2_hdr_size                     4    0
l2_log_blk_avg_asize            4    0
l2_log_blk_asize                4    0
l2_rebuild_size                 4    0
l2_rebuild_asize                4    0
arc_raw_size                    4    0
abd_chunk_waste_size            4    12288

so it hard depends on ZFS never changing the existing order or adding another field that
matches before the actual size one. So please add start/end anchors, e.g.:

if ($arcstats && $arcstats =~ m/^size\s+\d+\s+(\d+)$/m) {


> +	$res->{arcsize} = int ($1);

nit: extra whitespace after `int` and before the parenthesis.

> +    }
> +
>      return $res;
>  }
>