Skip to content

Enable zero-elimination if dedup is activated #17291

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

allanjude
Copy link
Contributor

Motivation and Context

Optimize dedup's handling of blocks of all-zeros.

Description

Creating high-refcount dedup entries for checksums of each record size is inefficient when the record could be recorded as a hole instead.

Normally, zero-elimination is disabled if compression is not enabled, but in the dedup case, it is expected that the write will be skipped anyway, we are just optimizing the way the write is skipped.

How Has This Been Tested?

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:

Creating high-refcount dedup entries for checksums of each record size
is inefficient when the record could be recorded as a hole instead.

Normally, zero-elimination is disabled if compression is not enabled,
but in the dedup case, it is expected that the write will be skipped
anyway, we are just optimizing the way the write is skipped.

Signed-off-by: Allan Jude <[email protected]>
@amotin
Copy link
Member

amotin commented May 1, 2025

@allanjude There seem to be some edge cases:

  [ 7192.631153] ZTS run /usr/share/zfs/zfs-tests/tests/functional/rsend/send-wR_encrypted_zvol
...
  [ 7195.123099] VERIFY3U(off + size, <=, abd->abd_size) failed (16384 <= 1024)
  [ 7195.125098] PANIC at abd.c:772:abd_iterate_func()
  [ 7195.126829] Showing stack for process 614070
  [ 7195.128264] CPU: 1 PID: 614070 Comm: z_wr_iss Tainted: P           OE      6.1.0-34-amd64 #1  Debian 6.1.135-1
  [ 7195.132048] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
  [ 7195.135981] Call Trace:
  [ 7195.138731]  <TASK>
  [ 7195.141046]  dump_stack_lvl+0x44/0x5c
  [ 7195.143620]  spl_panic+0xf0/0x108 [spl]
  [ 7195.146237]  ? vdev_disk_io_rw+0xb9/0x1e0 [zfs]
  [ 7195.158061]  abd_iterate_func+0x163/0x190 [zfs]
  [ 7195.161238]  ? abd_get_from_buf_impl+0x70/0x70 [zfs]
  [ 7195.162996]  zio_write_compress+0x9c9/0x11b0 [zfs]
  [ 7195.165429]  ? srso_alias_return_thunk+0x5/0x7f
  [ 7195.167039]  zio_execute+0xea/0x200 [zfs]
  [ 7195.168533]  taskq_thread+0x27d/0x5a0 [spl]
  [ 7195.169926]  ? wake_up_q+0x90/0x90
  [ 7195.171443]  ? zio_execute_stack_check.constprop.0+0x10/0x10 [zfs]
  [ 7195.174046]  ? taskq_lowest_id+0xc0/0xc0 [spl]
  [ 7195.175578]  kthread+0xda/0x100
  [ 7195.176777]  ? kthread_complete_and_exit+0x20/0x20
  [ 7195.178362]  ret_from_fork+0x22/0x30
  [ 7195.179891]  </TASK>

@amotin amotin added the Status: Revision Needed Changes are required for the PR to be accepted label May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Revision Needed Changes are required for the PR to be accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants