Skip to content

txg: generalise txg_wait_synced_sig() to txg_wait_synced_flags() #17284

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

Merged
merged 1 commit into from
May 2, 2025

Conversation

robn
Copy link
Member

@robn robn commented Apr 28, 2025

[Sponsors: Klara, Inc., Wasabi Technology, Inc.]

Motivation and Context

We have work in progress that will require breaking out of a txg_wait_synced() call in some situations. These are still in development, and may change, but are:

  • To make fsync() return error when the pool suspends, txg_wait_synced() needs to return when the pool suspends
  • A variation of forced export (a new update in Support forced export of ZFS pools #11082) may want to signal a condvar to throw out everything blocked in txg_wait_synced().

This is the supporting API change to allow these kind of things to be added in the future.

Description

txg_wait_synced_sig() is "wait for txg, unless a signal arrives". We expect that future development will require similar "wait unless X" behaviour.

This generalises the API as txg_wait_synced_flags(), where the provided flags describe the events that should cause the call to return.

Instead of a boolean, the return is now an error code, which the caller can use to know which event caused the call to return.

The existing call to txg_wait_synced_sig() is now txg_wait_synced_flags(TXG_WAIT_SIGNAL).

How Has This Been Tested?

Compile checked on Linux and FreeBSD. Basic sanity checks done.

(Tested a lot on internal projects too).

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:

Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

I have no objections, primarily a worry whether we'll be able to watch for few things same time. They'd need to have similar locking. But that's later. Meanwhile just couple things to consider.

@amotin amotin added the Status: Code Review Needed Ready for review and testing label Apr 29, 2025
@robn robn force-pushed the txg-wait-flags branch from 7e9c2cd to bda2484 Compare May 2, 2025 03:42
txg_wait_synced_sig() is "wait for txg, unless a signal arrives". We
expect that future development will require similar "wait unless X"
behaviour.

This generalises the API as txg_wait_synced_flags(), where the provided
flags describe the events that should cause the call to return.

Instead of a boolean, the return is now an error code, which the caller
can use to know which event caused the call to return.

The existing call to txg_wait_synced_sig() is now
txg_wait_synced_flags(TXG_WAIT_SIGNAL).

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <robn@despairlabs.com>
@robn robn force-pushed the txg-wait-flags branch from bda2484 to 69f2824 Compare May 2, 2025 03:53
@tonyhutter tonyhutter added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 2, 2025
@tonyhutter tonyhutter merged commit a7de203 into openzfs:master May 2, 2025
22 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants