range_tree: Add zfs_recover_rt parameter and extra debug info

[ihoro]

Motivation and Context

There are production cases when loading of a metaslab leads to a ZFS panic due to unexpected entries in its spacemap (presumably). The assertions in zfs_range_tree_add_impl() and zfs_range_tree_remove_impl() fail due to overlapping or missing segments, etc. A business would like to go ahead with such pools while the root cause is being investigated.

Description

The idea is to allow loading such metaslabs with a potential space leak as a trade-off instead of a potential data loss.

We already have zfs_recover module parameter to mitigate various issues, including some range tree cases, and this patch adds zfs_recover_ms parameter to localize the recovery behavior to the metaslab loading process only.

The following diagrams are expected to help with the details:

How Has This Been Tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[ihoro]

The update includes:

• Re-work into zfs_recover_rt parameter instead of zfs_recover_ms. It covers all range trees.
• Add range tree name as an extra debug info resulting into a message like zfs: rt_instance=vdev_obsolete_segments: ....
• The metaslab related trees provide even more details like zfs: rt_instance={spa=p1 vdev_guid=4127788562752866619 ms_id=0 ms_allocatable}: ....
• Update zfs.4 man page respectively

[tonyhutter]

Looks like there's a build issue:

    CC       module/zfs/libzpool_la-dsl_bookmark.lo
    CC       module/zfs/libzpool_la-dsl_crypt.lo
  module/zfs/dnode.c: In function 'dnode_free_range':
  module/zfs/dnode.c:2441:59: error: passing argument 7 of 'zfs_range_tree_create_usecase' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers]
   2441 |                             ZFS_RANGE_TREE_UC_FREE_SPACE, "dn_free_ranges");
        |                                                           ^~~~~~~~~~~~~~~~
  In file included from ./include/sys/space_map.h:34,
                   from ./include/sys/spa.h:46,
                   from ./include/sys/dbuf.h:32,
                   from module/zfs/dnode.c:28:
  ./include/sys/range_tree.h:294:45: note: expected 'char *' but argument is of type 'const char *'
    294 |     zfs_range_tree_usecase_t usecase, char *instance);
        |                                       ~~~~~~^~~~~~~~
    CC       module/zfs/libzpool_la-dsl_dataset.lo
    CC       module/zfs/libzpool_la-dsl_deadlist.lo
  cc1: all warnings being treated as errors
  make[4]: *** [Makefile:10313: module/zfs/libzpool_la-dnode.lo] Error 1

[ihoro]

Looks like there's a build issue:

Indeed, there should have been one more dev iteration on my side. It should be fixed now.

[tonyhutter]

For some reason a bunch of the runners are failing. I'm going to manually restart then.

[tonyhutter]

Please rebase on master; that will pull in the new .github/workflow/* files and allow the tests to complete.

[ihoro]

For some reason a bunch of the runners are failing. I'm going to manually restart then.

Thank you.

Please rebase on master; that will pull in the new .github/workflow/* files and allow the tests to complete.

Sure, it's been moved 3 commits up, over the recent workflow changes, it should work now.

[amotin]

I see plenty of places where use case is not defined. We could look better. Also I think in many cases we could still log pool and vdev, even if we have no metaslab number to report.

[ihoro]

I see plenty of places where use case is not defined. We could look better.

It seems it's time to rename the _UC_UNKNOWN flag to the more appropriate one _UC_GENERIC. It depicts the intention better -- a range tree without special treatment where zfs_recover_rt simply does not panic upon unexpected additions/removal.

The unknown (now generic) flag is intentionally used for the range tree instances where special treatment is not expected. Sometimes it's not about allocated/free space or it's a temporary tree which is based on already "recovered" other ones. Anyway, I think I could review the instances once again, probably someone should not be GENERIC.

Also I think in many cases we could still log pool and vdev, even if we have no metaslab number to report.

Yes, it's worth the extra code to be maximum useful. It will come with the next iteration of the patch.

[amotin]

I don't think "GENERIC" is really meaningful. Easier and cleaner I think would be just to pass 0 if we can't say anything better (we really should).