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

Workaround issue of Linux vdev_disk.c #16678

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Workaround issue of Linux vdev_disk.c #16678

merged 1 commit into from
Oct 23, 2024

Conversation

amotin
Copy link
Member

@amotin amotin commented Oct 22, 2024

in some cases not linearizing buffers with disk sector crossing a page boundary. It is fine for hardware, but somehow required by LUKS. It is not typical for ZFS to produce such buffers, but it may happen if 6KB block is compressed to 4KB, while still having 2KB alignment. Banning the 6KB buffers helps vdevs with ashift=12.

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:

in some cases not linearizing buffers with disk sector crossing a
page boundary. It is fine for hardware, but somehow required by LUKS.
It is not typical for ZFS to produce such buffers, but it may happen
if 6KB block is compressed to 4KB, while still having 2KB alignment.
Banning the 6KB buffers helps vdevs with ashifh=12.

Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
@tonyhutter tonyhutter added the Status: Code Review Needed Ready for review and testing label Oct 23, 2024
@tonyhutter tonyhutter merged commit aefc2da into openzfs:master Oct 23, 2024
18 of 20 checks passed
@amotin amotin deleted the 6kb branch October 23, 2024 17:29
@mabod
Copy link

mabod commented Oct 24, 2024

There is a typo in the comment:

ashifh=12 should read ashift=12

@mcmilk
Copy link
Contributor

mcmilk commented Oct 24, 2024

There is a typo in the comment:

ashifh=12 should read ashift=12

You are right, within the comment :-(

@amotin
Copy link
Member Author

amotin commented Oct 24, 2024

There is a typo in the comment:
ashifh=12 should read ashift=12

You are right, within the comment :-(

Another reason to remove all the workaround sooner. ;)

robn added a commit to robn/zfs that referenced this pull request Oct 25, 2024
Now that we can handle these different alignments, we don't this
workaround.

This reverts commit aefc2da.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
robn added a commit to robn/zfs that referenced this pull request Oct 26, 2024
Now that we can handle these different alignments, we don't this
workaround.

This reverts commit aefc2da.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
robn added a commit to robn/zfs that referenced this pull request Oct 26, 2024
Now that we can handle these different alignments, we don't this
workaround.

This reverts commit aefc2da.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
robn added a commit to robn/zfs that referenced this pull request Oct 27, 2024
Now that we can handle these different alignments, we don't this
workaround.

This reverts commit aefc2da.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
behlendorf pushed a commit that referenced this pull request Nov 1, 2024
Now that we can handle these different alignments, we don't this
workaround.

This reverts commit aefc2da.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes #16687
@tonyhutter tonyhutter mentioned this pull request Nov 6, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants