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

L2ARC: Cleanup buffer re-compression #16038

Merged
merged 1 commit into from
Apr 23, 2024
Merged

L2ARC: Cleanup buffer re-compression #16038

merged 1 commit into from
Apr 23, 2024

Conversation

amotin
Copy link
Member

@amotin amotin commented Mar 28, 2024

When compressed ARC is disabled, we may have to re-compress when writing into L2ARC. If doing so we can't fit it into the original physical size, we should just fail immediately, since even if it may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places without using abd_return_buf_copy().

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:

@amotin amotin requested a review from allanjude March 28, 2024 21:56
@amotin amotin added the Status: Code Review Needed Ready for review and testing label Mar 28, 2024
@amotin amotin force-pushed the l2arc_transform branch 2 times, most recently from b345274 to f0b1728 Compare March 29, 2024 14:43
@gmelikov
Copy link
Member

gmelikov commented Apr 1, 2024

Off-topic, but I want to highlight for reviewers related PR #9416 where we may partially improve re-compression consistency.

behlendorf
behlendorf approved these changes Apr 22, 2024
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Makes sense. Can you just rebase this.

module/zfs/arc.c Outdated Show resolved Hide resolved
@amotin
L2ARC: Cleanup buffer re-compression
5a95bc3 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
@amotin
Copy link
Member Author

amotin commented Apr 22, 2024

@behlendorf Rebased.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 22, 2024
@behlendorf behlendorf merged commit 1f940de into openzfs:master Apr 23, 2024
23 of 25 checks passed
@amotin amotin deleted the l2arc_transform branch April 23, 2024 17:03
amotin added a commit to amotin/zfs that referenced this pull request May 23, 2024
@amotin
L2ARC: Cleanup buffer re-compression
4542ce5 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16038
ixhamza pushed a commit to truenas/zfs that referenced this pull request May 23, 2024
@amotin @ixhamza
L2ARC: Cleanup buffer re-compression
5389e02 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16038
ixhamza pushed a commit to truenas/zfs that referenced this pull request May 23, 2024
@amotin @ixhamza
L2ARC: Cleanup buffer re-compression
f51168c 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16038
behlendorf pushed a commit that referenced this pull request May 29, 2024
@amotin @behlendorf
L2ARC: Cleanup buffer re-compression
0f1e8ba 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes #16038
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
@amotin @lundman
L2ARC: Cleanup buffer re-compression
c1c5504 
When compressed ARC is disabled, we may have to re-compress when
writing into L2ARC.  If doing so we can't fit it into the original
physical size, we should just fail immediately, since even if it
may still fit into allocation size, its checksum will never match.

While there, refactor the code similar to other compression places
without using abd_return_buf_copy().

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16038
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@allanjude allanjude Awaiting requested review from allanjude

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
@amotin @gmelikov @behlendorf