Re: [pve-devel] [PATCH zfsonlinux 2/2] update arc_summary arcstat patch with new introduced values

[Stoiko Ivanov <s.ivanov@proxmox.com>]

10 minutes after sending this - I saw a report about pvereport ending in a
Python stacktrace - took me a while to see that a similar issue is present
between 2.1 and 2.2 - will send the series again with those changes also
added (this time the method was changing the source until no more
stacktraces were present with the current userspace and kernel 6.2 (with
ZFS 2.1) running).

Not sure if dropping the whole patch or alternatively cleaning it up once
every major PVE release would also be an option (although tbh - I expect
quite a few monitoring tools to collect data from these utils - and having
that throw exceptions will probably cause some discomfort to our users...)


On Tue,  7 May 2024 15:38:36 +0200
Stoiko Ivanov <s.ivanov@proxmox.com> wrote:

> ZFS 2.2.4 added new kstats for speculative prefetch in:
> 026fe796465e3da7b27d06ef5338634ee6dd30d8
> 
> Adapt our patch introduced with ZFS 2.1 (for the then added MFU/MRU
> stats), to also deal with the now introduced values not being present
> (because an old kernel-module does not offer them).
> 
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
>  ...guard-access-to-freshly-introduced-.patch} | 79 ++++++++++++++++---
>  debian/patches/series                         |  2 +-
>  2 files changed, 69 insertions(+), 12 deletions(-)
>  rename debian/patches/{0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch => 0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch} (61%)
> 
> diff --git a/debian/patches/0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch b/debian/patches/0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
> similarity index 61%
> rename from debian/patches/0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
> rename to debian/patches/0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
> index 2e7c207d..a0768923 100644
> --- a/debian/patches/0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
> +++ b/debian/patches/0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
> @@ -1,7 +1,10 @@
>  From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
>  From: Thomas Lamprecht <t.lamprecht@proxmox.com>
>  Date: Wed, 10 Nov 2021 09:29:47 +0100
> -Subject: [PATCH] arc stat/summary: guard access to l2arc MFU/MRU stats
> +Subject: [PATCH] arc stat/summary: guard access to freshly introduced stats
> +
> +l2arc MFU/MRU and zfetch past future and stride stats were introduced
> +in 2.1 and 2.2.4 respectively:
>  
>  commit 085321621e79a75bea41c2b6511da6ebfbf2ba0a added printing MFU
>  and MRU stats for 2.1 user space tools, but those keys are not
> @@ -14,20 +17,24 @@ Move those two keys to a .get accessor with `0` as fallback, as it
>  should be better to show some possible wrong data for new stat-keys
>  than throwing an exception.
>  
> -Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> -
>  also move l2_mfu_asize  l2_mru_asize l2_prefetch_asize
>  l2_bufc_data_asize l2_bufc_metadata_asize to .get accessor
>  (these are only present with a cache device in the pool)
> +
> +guard access to zfetch past future stride stats introduced in
> +026fe796465e3da7b27d06ef5338634ee6dd30d8
> +
> +These are present in the current kernel, but lead to an exception, if
> +running the new user-space with an old kernel module.
> +
>  Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> -Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
>  ---
> - cmd/arc_summary | 28 ++++++++++++++--------------
> - cmd/arcstat.in  | 14 +++++++-------
> - 2 files changed, 21 insertions(+), 21 deletions(-)
> + cmd/arc_summary | 40 ++++++++++++++++++++--------------------
> + cmd/arcstat.in  | 26 +++++++++++++-------------
> + 2 files changed, 33 insertions(+), 33 deletions(-)
>  
>  diff --git a/cmd/arc_summary b/cmd/arc_summary
> -index 100fb1987..86b2260a1 100755
> +index 100fb1987..5fb2cdbbc 100755
>  --- a/cmd/arc_summary
>  +++ b/cmd/arc_summary
>  @@ -655,13 +655,13 @@ def section_arc(kstats_dict):
> @@ -48,6 +55,39 @@ index 100fb1987..86b2260a1 100755
>       prt_i1('L2 ineligible evictions:',
>              f_bytes(arc_stats['evict_l2_ineligible']))
>       print()
> +@@ -794,26 +794,26 @@ def section_dmu(kstats_dict):
> +     zfetch_stats = isolate_section('zfetchstats', kstats_dict)
> + 
> +     zfetch_access_total = int(zfetch_stats['hits']) +\
> +-        int(zfetch_stats['future']) + int(zfetch_stats['stride']) +\
> +-        int(zfetch_stats['past']) + int(zfetch_stats['misses'])
> ++        int(zfetch_stats.get('future', 0)) + int(zfetch_stats.get('stride', 0)) +\
> ++        int(zfetch_stats.get('past', 0)) + int(zfetch_stats['misses'])
> + 
> +     prt_1('DMU predictive prefetcher calls:', f_hits(zfetch_access_total))
> +     prt_i2('Stream hits:',
> +            f_perc(zfetch_stats['hits'], zfetch_access_total),
> +            f_hits(zfetch_stats['hits']))
> +-    future = int(zfetch_stats['future']) + int(zfetch_stats['stride'])
> ++    future = int(zfetch_stats.get('future', 0)) + int(zfetch_stats.get('stride', 0))
> +     prt_i2('Hits ahead of stream:', f_perc(future, zfetch_access_total),
> +            f_hits(future))
> +     prt_i2('Hits behind stream:',
> +-           f_perc(zfetch_stats['past'], zfetch_access_total),
> +-           f_hits(zfetch_stats['past']))
> ++           f_perc(zfetch_stats.get('past', 0), zfetch_access_total),
> ++           f_hits(zfetch_stats.get('past', 0)))
> +     prt_i2('Stream misses:',
> +            f_perc(zfetch_stats['misses'], zfetch_access_total),
> +            f_hits(zfetch_stats['misses']))
> +     prt_i2('Streams limit reached:',
> +            f_perc(zfetch_stats['max_streams'], zfetch_stats['misses']),
> +            f_hits(zfetch_stats['max_streams']))
> +-    prt_i1('Stream strides:', f_hits(zfetch_stats['stride']))
> ++    prt_i1('Stream strides:', f_hits(zfetch_stats.get('stride', 0)))
> +     prt_i1('Prefetches issued', f_hits(zfetch_stats['io_issued']))
> +     print()
> + 
>  @@ -860,20 +860,20 @@ def section_l2arc(kstats_dict):
>              f_perc(arc_stats['l2_hdr_size'], arc_stats['l2_size']),
>              f_bytes(arc_stats['l2_hdr_size']))
> @@ -80,10 +120,10 @@ index 100fb1987..86b2260a1 100755
>       print()
>       prt_1('L2ARC breakdown:', f_hits(l2_access_total))
>  diff --git a/cmd/arcstat.in b/cmd/arcstat.in
> -index c4f10a1d6..c570dca88 100755
> +index c4f10a1d6..3a716981b 100755
>  --- a/cmd/arcstat.in
>  +++ b/cmd/arcstat.in
> -@@ -597,8 +597,8 @@ def calculate():
> +@@ -597,19 +597,19 @@ def calculate():
>       v["el2skip"] = d["evict_l2_skip"] // sint
>       v["el2cach"] = d["evict_l2_cached"] // sint
>       v["el2el"] = d["evict_l2_eligible"] // sint
> @@ -93,7 +133,24 @@ index c4f10a1d6..c570dca88 100755
>  +    v["el2mru"] = d.get("evict_l2_eligible_mru", 0) // sint
>       v["el2inel"] = d["evict_l2_ineligible"] // sint
>       v["mtxmis"] = d["mutex_miss"] // sint
> -     v["ztotal"] = (d["zfetch_hits"] + d["zfetch_future"] + d["zfetch_stride"] +
> +-    v["ztotal"] = (d["zfetch_hits"] + d["zfetch_future"] + d["zfetch_stride"] +
> +-                   d["zfetch_past"] + d["zfetch_misses"]) // sint
> ++    v["ztotal"] = (d["zfetch_hits"] + d.get("zfetch_future", 0) + d.get("zfetch_stride", 0) +
> ++                   d.get("zfetch_past", 0) + d["zfetch_misses"]) // sint
> +     v["zhits"] = d["zfetch_hits"] // sint
> +-    v["zahead"] = (d["zfetch_future"] + d["zfetch_stride"]) // sint
> +-    v["zpast"] = d["zfetch_past"] // sint
> ++    v["zahead"] = (d.get("zfetch_future", 0) + d.get("zfetch_stride", 0)) // sint
> ++    v["zpast"] = d.get("zfetch_past", 0) // sint
> +     v["zmisses"] = d["zfetch_misses"] // sint
> +     v["zmax"] = d["zfetch_max_streams"] // sint
> +-    v["zfuture"] = d["zfetch_future"] // sint
> +-    v["zstride"] = d["zfetch_stride"] // sint
> ++    v["zfuture"] = d.get("zfetch_future", 0) // sint
> ++    v["zstride"] = d.get("zfetch_stride", 0) // sint
> +     v["zissued"] = d["zfetch_io_issued"] // sint
> +     v["zactive"] = d["zfetch_io_active"] // sint
> + 
>  @@ -624,11 +624,11 @@ def calculate():
>           v["l2size"] = cur["l2_size"]
>           v["l2bytes"] = d["l2_read_bytes"] // sint
> diff --git a/debian/patches/series b/debian/patches/series
> index 35f81d13..229027ff 100644
> --- a/debian/patches/series
> +++ b/debian/patches/series
> @@ -6,6 +6,6 @@
>  0006-dont-symlink-zed-scripts.patch
>  0007-Add-systemd-unit-for-importing-specific-pools.patch
>  0008-Patch-move-manpage-arcstat-1-to-arcstat-8.patch
> -0009-arc-stat-summary-guard-access-to-l2arc-MFU-MRU-stats.patch
> +0009-arc-stat-summary-guard-access-to-freshly-introduced-.patch
>  0010-Fix-nfs_truncate_shares-without-etc-exports.d.patch
>  0011-zpool-status-tighten-bounds-for-noalloc-stat-availab.patch



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