Skip to content

Conversation

@tonyhutter
Copy link
Contributor

Motivation and Context

Break out range_tree/btree/highbit64/lowbit64 code for #17864

Description

Break out the range_tree, btree, and highbit64/lowbit64 code from kernel space into shared kernel and userspace code. This is needed for the updated zpool status -vv error byte range reporting that will be coming in a future commit. That commit needs the range_tree code in kernel and userspace.

How Has This Been Tested?

Test built

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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • 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:

@tonyhutter
Copy link
Contributor Author

Note - I kept highbit64() and lowbit64() as inline functions rather than macros, since some parts of our code expected them to return an int specifically.

Break out the range_tree, btree, and highbit64/lowbit64 code from kernel
space into shared kernel and userspace code.  This is needed for the
updated `zpool status -vv` error byte range reporting that will be
coming in a future commit.  That commit needs the range_tree code in
kernel and userspace.

Signed-off-by: Tony Hutter <[email protected]>
uint_t num_logs(nvlist_t *nv);
uint64_t array64_max(uint64_t array[], unsigned int len);
int highbit64(uint64_t i);
int lowbit64(uint64_t i);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lowbit64 too

libnvpair.la
libnvpair.la \
librange_tree.la \
libbtree.la
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not using any of this yet so this hunk should get moved to the "error ranges" PR.

libnvpair.la

libnvpair.la \
librange_tree.la
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zhack shouldn't need this yet either.

#include <sys/btree.h>
#include <sys/bitops.h>
#include <sys/zfs_context.h>
#include <sys/sysmacros.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When compiling without ZFS_DEBUG there are a bunch of warning: unused parameter ‘xxx’ [-Wunused-parameter] messages which were seem to have been suppressed before. One way to tackle this would be to add some no-op #define macros for the !ZFS_DEBUG case.

module/zcommon/zprop_common.c
module/zcommon/zprop_common.c \
module/zfs/btree.c \
module/zfs/range_tree.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These aren't needed by libzfs and can be removed. We'll want to add in whatever dependencies are needed as part of the "error ranges" changes.

# | | \ \ | | / / | \ \
# libicp --/ | \ \ | | / / | \ \
# | \ librange_tree / | \ \
# libzstd ---/ \ libbtree / | \ \--------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be where we end up, but libzpool should be our only consumer of librange_tree and libbtree in this PR.

libzpool.la \
libzfs_core.la

libbtree.la
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested but I'm pretty sure we should be able to prune this down to what we actually use. Something like:

 %C%_btree_test_LDADD = \
       libavl.la \
       libbtree.la

@@ -0,0 +1,6 @@
# SPDX-License-Identifier: CDDL-1.0
libbtree_la_CFLAGS = $(AM_CFLAGS) $(KERNEL_CFLAGS) $(LIBRARY_CFLAGS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For both libbtree and librange_tree we should set -fvisibility=hidden by default in the Makefile.am and then define a wrapper macro to make visible the symbols we need. Both libnvpair and libavl are good examples. For example:

libbtree_la_CFLAGS += -fvisibility=hidden

and

#ifndef _BTREE_H
#define _BTREE_H extern __attribute__((visibility("default")))

_BTREE_H void zfs_btree_init(void);
_BTREE_H void zfs_btree_fini(void);
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants