Skip to content

Initialize vr_last_txg for rebuild - #18482

Merged
behlendorf merged 1 commit into
openzfs:masterfrom
behlendorf:pr-18473-followup
May 2, 2026
Merged

behlendorf merged 1 commit into
openzfs:masterfrom
behlendorf:pr-18473-followup

Conversation

@behlendorf

@behlendorf behlendorf commented May 1, 2026 •

Copy link
Copy Markdown
Contributor

Motivation and Context

Minor optimization as a followup to #18473.

Description

Only call txg_wait_synced() when rebuild IOs were issued for this metaslab. This is a small optimization since in practice the first metaslab is very likely to have allocations and cause vr_last_txg to be initialized. After this point when processing empty metaslabs txg_wait_synced() is called but with an already committed txg so it will not wait. Still it's better not to call txg_wait_synced() at all when it's not needed.

How Has This Been Tested?

Compiled, will be tested by the CI along with additional local testing.

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)

Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Copilot AI review requested due to automatic review settings May 1, 2026 20:55
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label May 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR aims to avoid unnecessary txg_wait_synced() calls during vdev rebuild by only waiting when rebuild I/Os have actually set vr_last_txg.

Changes:

  • Initialize vr_last_txg to 0 when starting the rebuild thread.
  • Guard txg_wait_synced() so it’s only called when vr_last_txg has been set.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread module/zfs/vdev_rebuild.c Resolved
Comment thread module/zfs/vdev_rebuild.c Resolved
Comment thread module/zfs/vdev_rebuild.c Resolved
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 2, 2026
@behlendorf
behlendorf merged commit d5099c3 into openzfs:master May 2, 2026
42 of 48 checks passed
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 4, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 4, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request May 6, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
kithrup pushed a commit to KlaraSystems/zfs that referenced this pull request May 7, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
Gality369 pushed a commit to Gality369/zfs that referenced this pull request May 12, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jul 30, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
pull Bot pushed a commit to A-Archives-and-Forks/openzfs that referenced this pull request Sep 26, 2026
Only call txg_wait_synced() when rebuild IOs were issued for this
metaslab.  This is a small optimization since in practice the first
metaslab is very likely to have allocations and cause vr_last_txg
to be initialized.  After this point when processing empty metaslabs
txg_wait_synced() is called but with an already committed txg so it
will not wait.  Still it's better not to call txg_wait_synced() at
all when it's not needed.

Reviewed-by: Andriy Tkachuk <atkachuk@wasabi.com>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#18482
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