Skip to content

Conversation

behlendorf
Copy link
Contributor

Motivation and Context

https://www.illumos.org/issues/14003 and #13479. I was only able to identify the one reported instance of this in our tracker, but this race definitely looks possible and should be fixed.

Description

From the illumos issue:

It's a race condition; the dtrace just widens the window (by inserting delays at the appropriate point). My theory is that zfs send is reading a blkptr that has a hdr in l1arc, but with no cached raw buffer, so it issues a zio_read for that data, then drops the hash_lock to wait (the prefetch code can do that, too, although it uses nowait()). At the same time, a different thread issues a dbuf_hold() for the same blkptr, and finds a dbuf pointing to an arc_buf with the same header in the dbuf cache. before the send's read completes, that same thread calls arc_release() (e.g. via dbuf_dirty() when adding or removing ZAP entries for objects in a directory). Since arc_release() only checks for existing buffers, and not outstanding i/o that may add a buffer later, it moves the hdr to state arc_anon. When the send's read finishes and send destroys the buffer, there are now two buffers (each with its own reference) on the hdr, which trips the verify.

How Has This Been Tested?

A reproducer was written which leveraged a new dtrace probe that allowed to race to be more easily triggered. It was still difficult to hit, but at least possible. Additional testing notes can be found at https://www.illumos.org/issues/14003#note-29.

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)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • 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:

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Aug 23, 2025
@behlendorf behlendorf marked this pull request as draft August 23, 2025 02:12
@github-actions github-actions bot added Status: Work in Progress Not yet ready for general review and removed Status: Code Review Needed Ready for review and testing labels Aug 23, 2025
@behlendorf behlendorf force-pushed the illumos-14003 branch 2 times, most recently from 5234e54 to ff23e28 Compare August 27, 2025 21:43
Authored by: Paul Zuchowski <[email protected]>
Reviewed by: Jerry Jelinek <[email protected]>
Reviewed by: Gordon Ross <[email protected]>
Reviewed by: Toomas Soome <[email protected]>
Approved by: Robert Mustacchi <[email protected]>

Illumos-issue: https://www.illumos.org/issues/14003
Illumos-commit: illumos/illumos-gate@b541cf35

Porting Notes:

The b_cv was removed in openzfs#15340 preventing us from adopting this
change without modification.  Instead, we adopt the approach taken
in openzfs#15340 to wait for any in progress IO.

Additionally, update trace_arc.h on Linux to include the new dtrace
probe.  While not required I opted to keep it.

Signed-off-by: Brian Behlendorf <[email protected]>
Issue openzfs#13479
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Work in Progress Not yet ready for general review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants