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.

Sign up for GitHub

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

Jump to bottom

Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional #8979

Merged
merged 1 commit into from
Jul 3, 2019
Merged

Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional #8979

merged 1 commit into from
Jul 3, 2019

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

Alternate fix for #8736. @ikozhukhov could you verify this change
resolves the issue on dilos as well.

Description

The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set. Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() is unsafe.
However, since it's largely redundant it has simply been removed.

How Has This Been Tested?

Without this change applied, a debug build of ZFS (--enable_debug) with
the zfs_flags ZFS_DEBUG_MODIFY bit clear would consistently fail when
running the send_mixed_raw.ksh test case. With the patch applied, the
test case passes regardless of how ZFS_DEBUG_MODIFY is set.

Note: This issue went unnoticed since by default ZFS_DEBUG_MODIFY is enabled for all debug builds. It must be explicitly cleared to reproduce the issue.

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)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
49961e3 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 1, 2019
ikozhukhov
ikozhukhov approved these changes Jul 3, 2019
View reviewed changes
Copy link
Contributor

@ikozhukhov ikozhukhov left a comment

Choose a reason for hiding this comment

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

confirm: PASS on DilOS

allanjude
allanjude approved these changes Jul 3, 2019
View reviewed changes
Copy link
Contributor

@allanjude allanjude left a comment

Choose a reason for hiding this comment

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

I was running into this while trying to debug stuff on Linux. Thanks for the fix.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jul 3, 2019
@behlendorf behlendorf merged commit 46db9d6 into openzfs:master Jul 3, 2019
TulsiJain pushed a commit to TulsiJain/zfs that referenced this pull request Jul 20, 2019
@behlendorf
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
5ec07d6 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
TulsiJain pushed a commit to TulsiJain/zfs that referenced this pull request Jul 20, 2019
@behlendorf
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
07bf776 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 13, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
ceb1d9c 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 22, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
1b67a61 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Aug 23, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
cb5d761 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 17, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
140e21b 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Sep 18, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
925ae8d 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#8979
tonyhutter pushed a commit that referenced this pull request Sep 26, 2019
@behlendorf @tonyhutter
Check b_freeze_cksum under ZFS_DEBUG_MODIFY conditional
7a03d7c 
The b_freeze_cksum field can only have data when ZFS_DEBUG_MODIFY
is set.  Therefore, the EQUIV check must be wrapped accordingly.
For the same reason the ASSERT in arc_buf_fill() in unsafe.
However, since it's largely redundant it has simply been removed.

Reviewed-by: George Wilson <gwilson@delphix.com>
Reviewed-by: Allan Jude <allanjude@freebsd.org>
Reviewed-by: Igor Kozhukhov <igor@dilos.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #8979
This was referenced Sep 8, 2020
Closed
Closed
@behlendorf behlendorf deleted the issue-8736 branch April 19, 2021 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allanjude allanjude allanjude approved these changes

@ikozhukhov ikozhukhov ikozhukhov approved these changes

@grwilson grwilson Awaiting requested review from grwilson

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@behlendorf @ikozhukhov @allanjude