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

fix panic on debug build #8736

Closed
wants to merge 1 commit into from
+6 −1
Closed

fix panic on debug build #8736

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion module/zfs/arc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,8 @@ arc_buf_try_copy_decompressed_data(arc_buf_t *buf)
{
arc_buf_hdr_t *hdr = buf->b_hdr;
boolean_t copied = B_FALSE;
int cnt = 0;
zio_cksum_t *b_freeze_cksum = NULL;

ASSERT(HDR_HAS_L1HDR(hdr));
ASSERT3P(buf->b_data, !=, NULL);
Expand All @@ -1866,13 +1868,16 @@ arc_buf_try_copy_decompressed_data(arc_buf_t *buf)
copied = B_TRUE;
break;
}
b_freeze_cksum = from->b_hdr->b_l1hdr.b_freeze_cksum;
cnt++;
}

/*
* There were no decompressed bufs, so there should not be a
* checksum on the hdr either.
*/
EQUIV(!copied, hdr->b_l1hdr.b_freeze_cksum == NULL);
if (cnt > 0)
EQUIV(!copied, b_freeze_cksum == NULL);
Copy link
Contributor

Choose a reason for hiding this comment

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

@ikozhukhov given that b_l1hdr.b_freeze_cksum can only be set when ZFS_DEBUG_MODIFY is enabled (see arc_cksum_compute(). I think this EQUIV is only valid when ZFS_DEBUG_MODIFY is set, and we should instead wrap it with a conditional that checks for the flag. For the same reason, the ASSERT after the call to arc_buf_try_copy_decompressed_data() in arc_buf_fill() looks wrong to me as well.

My suspicion is we've never seen this in testing because by default for debug builds we enable ZFS_DEBUG_MODIFY. It would be great to get @grwilson's thoughts on this, since I may be misunderstanding the intent of b_freeze_cksum here.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, b_freeze_cksum should only have data when ZFS_DEBUG_MODIFY is enabled. So if I understand the original problem then wrapping the EQUIV with if (zfs_flags & ZFS_DEBUG_MODIFY) would fix the problem.


return (copied);
}
Expand Down