From 7ed39eebe93eab911568f85201f27c44dacfb56e Mon Sep 17 00:00:00 2001 From: Mark Maybee Date: Wed, 24 Feb 2021 21:46:24 +0000 Subject: [PATCH 1/3] Cleanup per-vdev histogram reporting Streamline the per-vdev histogram reporting by directly using the data in vdev_mg->mg_histogram rather than "rolling up" the data from individual metaslab histogram data. --- sdb/commands/zfs/histograms.py | 15 ++++---- sdb/commands/zfs/vdev.py | 34 +++---------------- .../data/regression_output/zfs/spa -mH | 1 + .../data/regression_output/zfs/spa -vH | 1 + .../data/regression_output/zfs/spa -vmH | 1 + 5 files changed, 15 insertions(+), 37 deletions(-) diff --git a/sdb/commands/zfs/histograms.py b/sdb/commands/zfs/histograms.py index b1af719f..e8a5ba4e 100644 --- a/sdb/commands/zfs/histograms.py +++ b/sdb/commands/zfs/histograms.py @@ -136,19 +136,20 @@ def print_histogram(hist: drgn.Object, if max_count < HISTOGRAM_WIDTH_MAX: max_count = HISTOGRAM_WIDTH_MAX - if min_bucket <= max_bucket: - print(f'{" " * indent}seg-size count') - print(f'{" " * indent}{"-" * 8} {"-" * 5}') + if min_bucket > max_bucket: + print(f'{" " * indent}** No histogram data available **') + return + + print(f'{" " * indent}seg-size count') + print(f'{" " * indent}{"-" * 8} {"-" * 5}') for bucket in range(min_bucket, max_bucket + 1): count = int(hist[bucket]) stars = round(count * HISTOGRAM_WIDTH_MAX / max_count) print(f'{" " * indent}{fmt.size_nicenum(2**(bucket+offset)):>8}: ' f'{count:>6} {"*" * stars}') - if min_bucket > max_bucket: - print(f'{" " * indent}** No histogram data available **') - else: - ZFSHistogram.print_histogram_median(hist, offset, indent) + + ZFSHistogram.print_histogram_median(hist, offset, indent) def _call(self, objs: Iterable[drgn.Object]) -> None: for obj in objs: diff --git a/sdb/commands/zfs/vdev.py b/sdb/commands/zfs/vdev.py index a7be8733..33aee994 100644 --- a/sdb/commands/zfs/vdev.py +++ b/sdb/commands/zfs/vdev.py @@ -17,7 +17,7 @@ # pylint: disable=missing-docstring import argparse -from typing import Iterable, List, Tuple, Optional +from typing import Iterable, List, Optional import drgn import sdb @@ -69,31 +69,6 @@ def __init__(self, if self.args.weight: self.arg_list.append("-w") - # - # Iterate over the metaslabs to accumulate histogram data. - # - @staticmethod - def sum_histograms( - metaslabs: Iterable[drgn.Object]) -> Tuple[drgn.Object, int]: - shift = -1 - length = 1 - first_time = True - histsum: List[int] = [] - for msp in metaslabs: - if msp.ms_sm == sdb.get_typed_null(msp.ms_sm.type_): - continue - histogram = msp.ms_sm.sm_phys.smp_histogram - if first_time: - shift = int(msp.ms_sm.sm_shift) - length = len(histogram) - histsum = [0] * length - assert length == len(histogram) - assert shift == int(msp.ms_sm.sm_shift) - for (bucket, value) in enumerate(histogram): - histsum[bucket] += int(value) - first_time = False - return sdb.create_object(f'uint64_t[{length}]', histsum), shift - def pretty_print(self, vdevs: Iterable[drgn.Object], indent: int = 0) -> None: @@ -133,10 +108,9 @@ def pretty_print(self, vdev.vdev_ops.vdev_op_type.string_().decode("utf-8"), ) if self.args.histogram: - metaslabs = sdb.execute_pipeline([vdev], [Metaslab()]) - histsum, shift = self.sum_histograms(metaslabs) - if shift > 0: - ZFSHistogram.print_histogram(histsum, shift, indent + 5) + if vdev.vdev_mg != sdb.get_typed_null(vdev.vdev_mg.type_): + ZFSHistogram.print_histogram(vdev.vdev_mg.mg_histogram, 0, + indent + 5) if self.args.metaslab: metaslabs = sdb.execute_pipeline([vdev], [Metaslab()]) diff --git a/tests/integration/data/regression_output/zfs/spa -mH b/tests/integration/data/regression_output/zfs/spa -mH index 2fa8e0c1..2345bf5f 100644 --- a/tests/integration/data/regression_output/zfs/spa -mH +++ b/tests/integration/data/regression_output/zfs/spa -mH @@ -414,6 +414,7 @@ ADDR NAME 0xffffa08948af8000 HEALTHY NONE /dev/sdb1 0xffffa08949ff8000 HEALTHY NONE /dev/sdc1 0xffffa08949e58000 HEALTHY NONE /dev/sdd1 + ** No histogram data available ** ADDR ID OFFSET FREE FRAG UCMU ----------------------------------------------------------------- 0xffffa0894e631800 0 0x0 512MB 0% 0B diff --git a/tests/integration/data/regression_output/zfs/spa -vH b/tests/integration/data/regression_output/zfs/spa -vH index 31216382..134d3e89 100644 --- a/tests/integration/data/regression_output/zfs/spa -vH +++ b/tests/integration/data/regression_output/zfs/spa -vH @@ -54,6 +54,7 @@ ADDR NAME 0xffffa08948af8000 HEALTHY NONE /dev/sdb1 0xffffa08949ff8000 HEALTHY NONE /dev/sdc1 0xffffa08949e58000 HEALTHY NONE /dev/sdd1 + ** No histogram data available ** 0xffffa089413b8000 meta-domain seg-size count -------- ----- diff --git a/tests/integration/data/regression_output/zfs/spa -vmH b/tests/integration/data/regression_output/zfs/spa -vmH index 2fa8e0c1..2345bf5f 100644 --- a/tests/integration/data/regression_output/zfs/spa -vmH +++ b/tests/integration/data/regression_output/zfs/spa -vmH @@ -414,6 +414,7 @@ ADDR NAME 0xffffa08948af8000 HEALTHY NONE /dev/sdb1 0xffffa08949ff8000 HEALTHY NONE /dev/sdc1 0xffffa08949e58000 HEALTHY NONE /dev/sdd1 + ** No histogram data available ** ADDR ID OFFSET FREE FRAG UCMU ----------------------------------------------------------------- 0xffffa0894e631800 0 0x0 512MB 0% 0B From 47a9f8a67102ac670bea570fa3fb4daf5392833c Mon Sep 17 00:00:00 2001 From: Mark Maybee Date: Thu, 25 Feb 2021 19:55:34 +0000 Subject: [PATCH 2/3] Introduce sdb.is_null() Addresses issue #258 The function used for a null pointer check is sdb.get_typed_null() e.g. if msp.ms_sm == sdb_get_typed_null(msp.ms_sm.type_): This is awkward and confusing. The new sdb.is_null() iterface simplifies the null check: if sdb.is_null(msp.ms_sm): --- sdb/__init__.py | 8 ++++---- sdb/commands/spl/avl.py | 2 +- sdb/commands/zfs/metaslab.py | 4 ++-- sdb/commands/zfs/vdev.py | 2 +- sdb/target.py | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sdb/__init__.py b/sdb/__init__.py index 16a30d81..1d24265b 100644 --- a/sdb/__init__.py +++ b/sdb/__init__.py @@ -32,9 +32,9 @@ CommandInvalidInputError, SymbolNotFoundError, CommandArgumentsError, CommandEvalSyntaxError, ParserError) -from sdb.target import (create_object, get_object, get_prog, get_typed_null, - get_type, get_pointer_type, get_target_flags, - get_symbol, type_canonical_name, type_canonicalize, +from sdb.target import (create_object, get_object, get_prog, get_type, + get_pointer_type, get_target_flags, get_symbol, is_null, + type_canonical_name, type_canonicalize, type_canonicalize_name, type_canonicalize_size, type_equals) from sdb.command import (Address, Cast, Command, InputHandler, Locator, @@ -63,6 +63,7 @@ 'create_object', 'execute_pipeline', 'invoke', + 'is_null', 'get_first_type', 'get_object', 'get_pointer_type', @@ -71,7 +72,6 @@ 'get_symbol', 'get_target_flags', 'get_type', - 'get_typed_null', 'type_canonical_name', 'type_canonicalize', 'type_canonicalize_name', diff --git a/sdb/commands/spl/avl.py b/sdb/commands/spl/avl.py index 8eec654f..61333fb2 100644 --- a/sdb/commands/spl/avl.py +++ b/sdb/commands/spl/avl.py @@ -29,7 +29,7 @@ class Avl(sdb.Walker): input_type = "avl_tree_t *" def _helper(self, node: drgn.Object, offset: int) -> Iterable[drgn.Object]: - if node == sdb.get_typed_null(node.type_): + if sdb.is_null(node): return lchild = node.avl_child[0] diff --git a/sdb/commands/zfs/metaslab.py b/sdb/commands/zfs/metaslab.py index 4be07124..b4e0b626 100644 --- a/sdb/commands/zfs/metaslab.py +++ b/sdb/commands/zfs/metaslab.py @@ -133,7 +133,7 @@ def print_metaslab(msp: drgn.Object, print_header: bool, print("".ljust(indent), "-" * 65) free = msp.ms_size - if spacemap != sdb.get_typed_null(spacemap.type_): + if not sdb.is_null(spacemap): free -= spacemap.sm_phys.smp_alloc ufrees = msp.ms_unflushed_frees.rt_space @@ -169,7 +169,7 @@ def pretty_print(self, Metaslab.print_metaslab(msp, first_time, indent) if self.args.histogram: spacemap = msp.ms_sm - if spacemap != sdb.get_typed_null(spacemap.type_): + if not sdb.is_null(spacemap): histogram = spacemap.sm_phys.smp_histogram ZFSHistogram.print_histogram(histogram, int(spacemap.sm_shift), diff --git a/sdb/commands/zfs/vdev.py b/sdb/commands/zfs/vdev.py index 33aee994..e53ebce6 100644 --- a/sdb/commands/zfs/vdev.py +++ b/sdb/commands/zfs/vdev.py @@ -108,7 +108,7 @@ def pretty_print(self, vdev.vdev_ops.vdev_op_type.string_().decode("utf-8"), ) if self.args.histogram: - if vdev.vdev_mg != sdb.get_typed_null(vdev.vdev_mg.type_): + if not sdb.is_null(vdev.vdev_mg): ZFSHistogram.print_histogram(vdev.vdev_mg.mg_histogram, 0, indent + 5) diff --git a/sdb/target.py b/sdb/target.py index 64db9b39..ed9c11d4 100644 --- a/sdb/target.py +++ b/sdb/target.py @@ -60,9 +60,9 @@ def get_pointer_type(type_name: str) -> drgn.Type: return prog.pointer_type(type_name) -def get_typed_null(type_name: str) -> drgn.Object: +def is_null(obj: drgn.Object) -> bool: global prog - return drgn.NULL(prog, type_name) + return bool(obj == drgn.NULL(prog, obj.type_)) def create_object(type_: Union[str, drgn.Type], val: Any) -> drgn.Object: From 92532a383c95fba743a595cf94afde31573a4425 Mon Sep 17 00:00:00 2001 From: Mark Maybee Date: Thu, 25 Feb 2021 22:42:38 +0000 Subject: [PATCH 3/3] Clean up pylint 2.7.1 warnings: W0221: Parameters differ from overridden 'pretty_print' method (arguments-differ) Occurring at: sdb/commands/zfs/vdev.py:72 sdb/commands/zfs/metaslab.py:163 sdb/commands/zfs/spa.py:68 sdb/commands/zfs/dbuf.py:84 When overriding methods the arguments must match exactly. Introduced print_indented() method for Metaslab and Vdev classes. --- sdb/commands/zfs/dbuf.py | 4 ++-- sdb/commands/zfs/metaslab.py | 9 ++++++--- sdb/commands/zfs/spa.py | 6 +++--- sdb/commands/zfs/vdev.py | 11 +++++++---- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/sdb/commands/zfs/dbuf.py b/sdb/commands/zfs/dbuf.py index cfb3755a..898c3f62 100644 --- a/sdb/commands/zfs/dbuf.py +++ b/sdb/commands/zfs/dbuf.py @@ -81,10 +81,10 @@ def ObjsetName(os: drgn.Object) -> str: os.os_spa.spa_name.string_().decode("utf-8")) return Dbuf.DatasetName(os.os_dsl_dataset) - def pretty_print(self, dbufs: drgn.Object) -> None: + def pretty_print(self, objs: drgn.Object) -> None: print("{:>20} {:>8} {:>4} {:>8} {:>5} {}".format( "addr", "object", "lvl", "blkid", "holds", "os")) - for dbuf in filter(self.argfilter, dbufs): + for dbuf in filter(self.argfilter, objs): print("{:>20} {:>8d} {:>4d} {:>8d} {:>5d} {}".format( hex(dbuf), int(dbuf.db.db_object), int(dbuf.db_level), int(dbuf.db_blkid), int(dbuf.db_holds.rc_count), diff --git a/sdb/commands/zfs/metaslab.py b/sdb/commands/zfs/metaslab.py index b4e0b626..7c7b0465 100644 --- a/sdb/commands/zfs/metaslab.py +++ b/sdb/commands/zfs/metaslab.py @@ -160,9 +160,9 @@ def print_metaslab(msp: drgn.Object, print_header: bool, print((str(int(msp.ms_fragmentation)) + "%").rjust(6), end="") print(nicenum(uchanges_mem).rjust(9)) - def pretty_print(self, - metaslabs: Iterable[drgn.Object], - indent: int = 0) -> None: + def print_indented(self, + metaslabs: Iterable[drgn.Object], + indent: int = 0) -> None: first_time = True for msp in metaslabs: if not self.args.weight: @@ -178,6 +178,9 @@ def pretty_print(self, Metaslab.metaslab_weight_print(msp, first_time, indent) first_time = False + def pretty_print(self, objs: Iterable[drgn.Object]) -> None: + self.print_indented(objs, 0) + @sdb.InputHandler("vdev_t*") def from_vdev(self, vdev: drgn.Object) -> Iterable[drgn.Object]: if self.args.metaslab_ids: diff --git a/sdb/commands/zfs/spa.py b/sdb/commands/zfs/spa.py index ad379487..160e504d 100644 --- a/sdb/commands/zfs/spa.py +++ b/sdb/commands/zfs/spa.py @@ -65,10 +65,10 @@ def __init__(self, if self.args.weight: self.arg_list.append("-w") - def pretty_print(self, spas: Iterable[drgn.Object]) -> None: + def pretty_print(self, objs: Iterable[drgn.Object]) -> None: print("{:18} {}".format("ADDR", "NAME")) print("%s" % ("-" * 60)) - for spa in spas: + for spa in objs: print("{:18} {}".format(hex(spa), spa.spa_name.string_().decode("utf-8"))) if self.args.histogram: @@ -77,7 +77,7 @@ def pretty_print(self, spas: Iterable[drgn.Object]) -> None: if self.args.vdevs or self.args.metaslab: vdevs = sdb.execute_pipeline([spa], [Vdev()]) - Vdev(self.arg_list).pretty_print(vdevs, 5) + Vdev(self.arg_list).print_indented(vdevs, 5) def no_input(self) -> drgn.Object: spas = sdb.execute_pipeline( diff --git a/sdb/commands/zfs/vdev.py b/sdb/commands/zfs/vdev.py index e53ebce6..131ae8da 100644 --- a/sdb/commands/zfs/vdev.py +++ b/sdb/commands/zfs/vdev.py @@ -69,9 +69,9 @@ def __init__(self, if self.args.weight: self.arg_list.append("-w") - def pretty_print(self, - vdevs: Iterable[drgn.Object], - indent: int = 0) -> None: + def print_indented(self, + vdevs: Iterable[drgn.Object], + indent: int = 0) -> None: print( "".ljust(indent), "ADDR".ljust(18), @@ -114,7 +114,10 @@ def pretty_print(self, if self.args.metaslab: metaslabs = sdb.execute_pipeline([vdev], [Metaslab()]) - Metaslab(self.arg_list).pretty_print(metaslabs, indent + 5) + Metaslab(self.arg_list).print_indented(metaslabs, indent + 5) + + def pretty_print(self, objs: Iterable[drgn.Object]) -> None: + self.print_indented(objs, 0) @sdb.InputHandler("spa_t*") def from_spa(self, spa: drgn.Object) -> Iterable[drgn.Object]: