Skip to content

ZIL: restore some things lost in "ZIL-crash" review (#17398)#17622

Closed
robn wants to merge 5 commits intoopenzfs:masterfrom
robn:zil-crash-safety
Closed

ZIL: restore some things lost in "ZIL-crash" review (#17398)#17622
robn wants to merge 5 commits intoopenzfs:masterfrom
robn:zil-crash-safety

Conversation

@robn
Copy link
Copy Markdown
Member

@robn robn commented Aug 12, 2025

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

Motivation and Context

I was reviewing another PR and noticed that the lwb_flags field wasn't there. Looking closer, and I realised that when I rebased in #17398 (comment), I did it on an older work branch that didn't have all of more recent changes from review. This PR restores those.

Description

This was done by reading the diff between ef66c53 (the last position of #17398 before my screw-up) and master and carefully reapplying the changes.

Review comments in #17398 have more context.

Refactor/cleanup:

  • add lwb_flags, make lwb_slog and lwb_slim be flags
  • change zil_commit_waiter_skip() to be zil_commit_waiter_done() and take an error, and use it

Functional:

  • add a flag LWB_FLAG_CRASHED to in-flight LWBs in zil_crash(), and use it to know to take no further action. This is mainly to avoid calling list_next(&zl_lwb_list, lwb) when lwb is on zl_lwb_crash_list via the same node, but being able to clearly identify an LWB as "dead" is generally useful.
  • if the pool suspends during stall fallback in zil_process_commit_list(), stop trying, and get an error back to unassigned waiters
  • don't leak the suspend cookie if the pool suspended while we were waiting for the zil to suspend

How Has This Been Tested?

zil and failmode test tags succeeded, though none of them really exercise these fairly extreme edge cases.

This stuff was all approved in #17398 though so hopefully its not too onerous (provided I didn't screw it up again).

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:

robn and others added 5 commits August 12, 2025 12:57
@robn
ZIL: use a bitfield for LWB "slog" and "slim" state flags
de082a8 
I'm soon about to need another LWB flag, and boolean_t is just so big
for only storing a single bit. Changing to a bitfield is far less
wasteful.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
@robn
ZIL: flag crashed LWBs so we know not to process them
99beee0 
If the ZIL crashed, any outstanding LWBs are no longer interesting, so
if they return, we need to just clean them up and return, not try to do
any work on them. This is true even if they return success, as that may
be long after the pool suspended and resumed, depending on when/if the
kernel decides to return the IO to us. In particular, we must not try to
get the "next" LWB from zl_lwb_list, since they're no longer on that
list.

So, we put a flag on in-flight LWBs in zil_crash() when we move them
from zl_lwb_list to zl_lwb_crash_list, so we know what's going on when
they return.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
@robn
ZIL: single zil_commit_waiter_done() function to complete a waiter
da02b57 
Just making it easier to not get the locking and broadcast wrong.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
@robn
zil_process_commit_list: fail better if the pool suspends in stall
716cc0f 
Make sure we properly inform the nolwb waiters of the error, and don't
keep trying.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
@robn
zil_suspend: fix cookie leak if ZIL crashes during wait
cdf1f53 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
@robn robn changed the title ZIL: restore some things lost in #17398 review ZIL: restore some things lost in "ZIL-crash" review (#17398) Aug 12, 2025
amotin
amotin approved these changes Aug 12, 2025
View reviewed changes
Copy link
Copy Markdown
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 did have feeling we have too many concurrent things going on there...

@amotin amotin added the Status: Accepted Ready to integrate (reviewed, tested) label Aug 12, 2025
behlendorf pushed a commit that referenced this pull request Aug 12, 2025
@robn @behlendorf
ZIL: flag crashed LWBs so we know not to process them
92da3e1 
If the ZIL crashed, any outstanding LWBs are no longer interesting, so
if they return, we need to just clean them up and return, not try to do
any work on them. This is true even if they return success, as that may
be long after the pool suspended and resumed, depending on when/if the
kernel decides to return the IO to us. In particular, we must not try to
get the "next" LWB from zl_lwb_list, since they're no longer on that
list.

So, we put a flag on in-flight LWBs in zil_crash() when we move them
from zl_lwb_list to zl_lwb_crash_list, so we know what's going on when
they return.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #17622
behlendorf pushed a commit that referenced this pull request Aug 12, 2025
@robn @behlendorf
ZIL: single zil_commit_waiter_done() function to complete a waiter
f562e0f 
Just making it easier to not get the locking and broadcast wrong.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #17622
behlendorf pushed a commit that referenced this pull request Aug 12, 2025
@robn @behlendorf
zil_process_commit_list: fail better if the pool suspends in stall
7c9adc6 
Make sure we properly inform the nolwb waiters of the error, and don't
keep trying.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #17622
behlendorf pushed a commit that referenced this pull request Aug 12, 2025
@robn @behlendorf
zil_suspend: fix cookie leak if ZIL crashes during wait
531568f 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes #17622
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
@robn @spauka
ZIL: use a bitfield for LWB "slog" and "slim" state flags
8213e05 
I'm soon about to need another LWB flag, and boolean_t is just so big
for only storing a single bit. Changing to a bitfield is far less
wasteful.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
@robn @spauka
ZIL: flag crashed LWBs so we know not to process them
41e0cd1 
If the ZIL crashed, any outstanding LWBs are no longer interesting, so
if they return, we need to just clean them up and return, not try to do
any work on them. This is true even if they return success, as that may
be long after the pool suspended and resumed, depending on when/if the
kernel decides to return the IO to us. In particular, we must not try to
get the "next" LWB from zl_lwb_list, since they're no longer on that
list.

So, we put a flag on in-flight LWBs in zil_crash() when we move them
from zl_lwb_list to zl_lwb_crash_list, so we know what's going on when
they return.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
@robn @spauka
ZIL: single zil_commit_waiter_done() function to complete a waiter
08b5de7 
Just making it easier to not get the locking and broadcast wrong.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
@robn @spauka
zil_process_commit_list: fail better if the pool suspends in stall
8a8f24b 
Make sure we properly inform the nolwb waiters of the error, and don't
keep trying.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
spauka pushed a commit to spauka/zfs that referenced this pull request Aug 30, 2025
@robn @spauka
zil_suspend: fix cookie leak if ZIL crashes during wait
167b422 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
@robn robn deleted the zil-crash-safety branch January 9, 2026 02:37
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
@robn @lundman
ZIL: use a bitfield for LWB "slog" and "slim" state flags
7d564e0 
I'm soon about to need another LWB flag, and boolean_t is just so big
for only storing a single bit. Changing to a bitfield is far less
wasteful.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
@robn @lundman
ZIL: flag crashed LWBs so we know not to process them
9579fe7 
If the ZIL crashed, any outstanding LWBs are no longer interesting, so
if they return, we need to just clean them up and return, not try to do
any work on them. This is true even if they return success, as that may
be long after the pool suspended and resumed, depending on when/if the
kernel decides to return the IO to us. In particular, we must not try to
get the "next" LWB from zl_lwb_list, since they're no longer on that
list.

So, we put a flag on in-flight LWBs in zil_crash() when we move them
from zl_lwb_list to zl_lwb_crash_list, so we know what's going on when
they return.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
@robn @lundman
ZIL: single zil_commit_waiter_done() function to complete a waiter
0a0a416 
Just making it easier to not get the locking and broadcast wrong.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
@robn @lundman
zil_process_commit_list: fail better if the pool suspends in stall
946fcd4 
Make sure we properly inform the nolwb waiters of the error, and don't
keep trying.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonosx/openzfs-fork that referenced this pull request Jan 30, 2026
@robn @lundman
zil_suspend: fix cookie leak if ZIL crashes during wait
0b9b309 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Feb 15, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
d764055 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: codex-5.3-xhigh
Porting-notes: clean cherry-pick.
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Feb 15, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
1f99f75 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: codex-5.3-xhigh
Porting-notes: clean cherry-pick.
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Feb 16, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
b74a05b 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: codex-5.3-xhigh
Porting-notes: clean cherry-pick.
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
@robn @lundman
ZIL: use a bitfield for LWB "slog" and "slim" state flags
4165535 
I'm soon about to need another LWB flag, and boolean_t is just so big
for only storing a single bit. Changing to a bitfield is far less
wasteful.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
@robn @lundman
ZIL: flag crashed LWBs so we know not to process them
b105538 
If the ZIL crashed, any outstanding LWBs are no longer interesting, so
if they return, we need to just clean them up and return, not try to do
any work on them. This is true even if they return success, as that may
be long after the pool suspended and resumed, depending on when/if the
kernel decides to return the IO to us. In particular, we must not try to
get the "next" LWB from zl_lwb_list, since they're no longer on that
list.

So, we put a flag on in-flight LWBs in zil_crash() when we move them
from zl_lwb_list to zl_lwb_crash_list, so we know what's going on when
they return.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
@robn @lundman
ZIL: single zil_commit_waiter_done() function to complete a waiter
70f0448 
Just making it easier to not get the locking and broadcast wrong.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
@robn @lundman
zil_process_commit_list: fail better if the pool suspends in stall
098f884 
Make sure we properly inform the nolwb waiters of the error, and don't
keep trying.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Feb 23, 2026
@robn @lundman
zil_suspend: fix cookie leak if ZIL crashes during wait
b129d1d 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Mar 1, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
0bf8f4e 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: codex-5.3-xhigh
Porting-notes: clean cherry-pick.
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Apr 28, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
1749cec 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: Pavel Snajdr <snajpa@snajpa.net>
Porting-notes: clean cherry-pick.
snajpa pushed a commit to vpsfreecz/zfs that referenced this pull request Apr 30, 2026
@robn @snajpa
zil_suspend: fix cookie leak if ZIL crashes during wait
0c59f4b 
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin@TrueNAS.com>
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Closes openzfs#17622
Upstream: 531568f
Ported-by: Pavel Snajdr <snajpa@snajpa.net>
Porting-notes: clean cherry-pick.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@behlendorf behlendorf behlendorf approved these changes

@amotin amotin amotin approved these changes

Assignees

No one assigned

Labels

Status: Accepted Ready to integrate (reviewed, tested)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@robn @behlendorf @amotin