Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libzstd: also build with LIBZPOOL_CPPFLAGS #16489

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

robn
Copy link
Member

@robn robn commented Aug 30, 2024

[Sponsors: Klara, Inc., Wasabi Technology, Inc.]

Motivation and Context

A user reported an abd_verify() crash through libzstd, and wondered if it was the same thing as #16477. And it was!

Description

libzstd now also allocates its own abd_t, and so has the same issue as zstream did, so this applies the same workaround: compile it with ZFS_DEBUG. See 92fca1c.

This looks weird, because libzstd doesn't appear to look related to the ZFS kernel, but there is already a cross-dependency there: zstd needs zfs_lz4_compress, and zfs needs zfs_zstd_compress (and others), so the two can never really be separated without more work. Another job for another time.

A quick glance suggests zstd isn't using ZFS_DEBUG or DEBUG for anything else, so I wouldn't expect this to make any meaningful difference elsewhere.

How Has This Been Tested?

Compile checked only.

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:

libzstd now also allocates its own abd_t, and so has the same issue as
zstream did, so this applies the same workaround: compile it with
ZFS_DEBUG. See 92fca1c.

This looks weird, because libzstd doesn't appear to look related to the
ZFS kernel, but there is already a cross-dependency there: zstd needs
zfs_lz4_compress, and zfs needs zfs_zstd_compress (and others), so the
two can never really be separated without more work. Another job for
another time.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
@behlendorf behlendorf merged commit 5c67820 into openzfs:master Sep 9, 2024
21 checks passed
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Sep 9, 2024
pfannc pushed a commit to pfannc/zfs that referenced this pull request Nov 5, 2024
libzstd now also allocates its own abd_t, and so has the same issue as
zstream did, so this applies the same workaround: compile it with
ZFS_DEBUG. See 92fca1c.

This looks weird, because libzstd doesn't appear to look related to the
ZFS kernel, but there is already a cross-dependency there: zstd needs
zfs_lz4_compress, and zfs needs zfs_zstd_compress (and others), so the
two can never really be separated without more work. Another job for
another time.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Mark Maybee <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16489
pfannc pushed a commit to pfannc/zfs that referenced this pull request Nov 7, 2024
libzstd now also allocates its own abd_t, and so has the same issue as
zstream did, so this applies the same workaround: compile it with
ZFS_DEBUG. See 92fca1c.

This looks weird, because libzstd doesn't appear to look related to the
ZFS kernel, but there is already a cross-dependency there: zstd needs
zfs_lz4_compress, and zfs needs zfs_zstd_compress (and others), so the
two can never really be separated without more work. Another job for
another time.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Mark Maybee <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16489
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants