zfs-2.1.6 patchset

[tonyhutter]

Motivation and Context

New release mainly to support the 5.19 kernel.

Description

Fedora 36 is now running the 5.19.8 kernel, and we need to support it.

How Has This Been Tested?

Buildbot will test

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:

• [ ] My code follows the OpenZFS code style requirements.
• [ ] I have updated the documentation accordingly.
• [ ] I have read the contributing document.
• [ ] I have added tests to cover my changes.
• [ ] I have run the ZFS Test Suite with this change applied.
• [ ] All commit messages are properly formatted and contain Signed-off-by.

[ryao]

This use-after-free fix probably should be included:

https://github.com/openzfs/zfs/commit/13f2b8fb92c23090b9f6e701c8471aef6b8e917b

Plenty of bad things are possible through that on non-debug builds. The issue has been detected in the wild through debug builds:

https://github.com/openzfs/zfs/issues/10989

[tonyhutter]

@ryao thanks, included

[ryao]

One more to add:

https://github.com/openzfs/zfs/commit/e5327e7f9790ed7e884a7f8d9fa412632506b826

It is unlikely to be hit, but it is a very simple fix.

[tonyhutter]

@ryao included in my latest push

[ryao]

https://github.com/openzfs/zfs/commit/b24d1c77f7fc53d26ee915b5203a139f13fd9791

That was just added to master. There are a few issues in the issue tracker where I would like to be able to ask end users to turn that on, so it would be useful if we could get that into 2.1.6.

[ryao]

A few more patches were merged to master today that might be good to put into 2.1.6.

This fixes both races that can cause assertions to incorrectly go off on amd64/x86 and races that can cause bad behavior on ARM and PowerPC:

https://github.com/openzfs/zfs/commit/e949d36040e5e79fe0dfda6a33451111cc5a0476

This fixes an issue where we overfill a queue when doing redacted send/recv:

https://github.com/openzfs/zfs/commit/ddb1fd91c0dbf64847235ee65e50e87c43257b05

[ryao]

This deadlock fix probably should be added:

https://github.com/openzfs/zfs/commit/577d41d3b2e4b37f51270c399c85b2708e21238a

[ryao]

This patch probably should be added so that distributions do not ship incomplete builds in the event something goes wrong that would be uncaught without this:

https://github.com/openzfs/zfs/commit/6a2dda8f05d9fb3c5b7d81c8c6762cd43be07dd7

[ryao]

37f6845c6f86b1d04593e55d94318326006f4b5d probably should be added. We had #13917 filed because our handling of drives that report large physical block sizes is currently poor. That patch improves things.

[ixhamza]

@tonyhutter It would be nice to have these commits as well: https://github.com/openzfs/zfs/commit/a7bd20e309a4b45b18b1da8e379f5826debe4870 https://github.com/openzfs/zfs/commit/c50b3f14d33cd469af47e16f0c6c76f2b4b5158e https://github.com/openzfs/zfs/commit/577d41d3b2e4b37f51270c399c85b2708e21238a. Do let me know in case of any issues or conflicts.

[ghost]

Along with a7bd20e309a4b45b18b1da8e379f5826debe4870 we'll also want the other commit from that PR: 48ab427e9dbed2a98ac97e1d62851ee70701b2d5

[tonyhutter]

@freqlabs it appears https://github.com/openzfs/zfs/commit/48ab427e9dbed2a98ac97e1d62851ee70701b2d5 was squashed into https://github.com/openzfs/zfs/commit/a7bd20e309a4b45b18b1da8e379f5826debe4870 at the time of merge.

I'll try to pull all requested commits in.

[sand-bit]

Not quite sure where else to ask. Since 2.1.6 has been tagged, are we still waiting on https://github.com/openzfs/zfs/commit/b24d1c77f7fc53d26ee915b5203a139f13fd9791 [Add zfs_btree_verify_intensity kernel module parameter] https://github.com/openzfs/zfs/commit/e949d36040e5e79fe0dfda6a33451111cc5a0476 [Fix assertions in crypto reference helpers] https://github.com/openzfs/zfs/commit/ddb1fd91c0dbf64847235ee65e50e87c43257b05 [Fix incorrect size given to bqueue_enqueue() call in dmu_redact.c] Or could the pull request be considered a near final release?

[ryao]

This closes #13622.

[tonyhutter]

@sand-bit I pulled these two in: https://github.com/openzfs/zfs/commit/b24d1c77f7fc53d26ee915b5203a139f13fd9791 [Add zfs_btree_verify_intensity kernel module parameter] https://github.com/openzfs/zfs/commit/ddb1fd91c0dbf64847235ee65e50e87c43257b05 [Fix incorrect size given to bqueue_enqueue() call in dmu_redact.c]

This one is a little scary looking and just fixes asserts (which aren't enabled by default) so I left it out: https://github.com/openzfs/zfs/commit/e949d36040e5e79fe0dfda6a33451111cc5a0476 [Fix assertions in crypto reference helpers]

[ryao]

@tonyhutter The original intention was to just fix assertions, but it was pointed out during review that the memory barriers were not properly used in the original code and could cause race conditions on other architectures like ARM and PPC. This is because membar_exit() was disabled via a preprocessor definition. Fixes for that were then added to the patch, but I forgot to update the commit message to talk about it. Without that patch, it is theoretically possible to have use-after-free issues from out of order execution on a number of RISC architectures. It probably should be included.

If you still find it scary, it is possible to split out the fix for ARM, PPC, etcetera by doing a single line change:

#define membar_exit()

Would become:

#define membar_exit() member_producer()

That is sufficient to fix the problems for ARM, PPC, etcetera.

[behlendorf]

@ryao since e949d360 was only recently merged, and comparatively lightly tested, let's hold off on including it in this release. We can consider adding it to a future point release.

[pedrohlc-mindlab]

I wasn't having any issues with zfs-2.1.6-staging or this branch a few commits ago (been daily updating it), now with 62686bc I'm getting some "Pool zroot has encountered an uncorrectable" and call trace:

__schedule
? get_nohz_timer_target
? raw_spin_unlock_irqrestore
? __mod_timer
schedule
schedule_timeout
? timer_migration_handler
io_schedule_timeout
__cv_timedwait_common [spl]
? cpuacct_percpu_seq_show
__cv_timedwait_io [spl]
zio_wait [zfs]
dmu_buf_hold_array_by_dnode [zfs]
dmu_read_uio_dnode [zfs]
? preempt_count_add
? raw_spin_lock
? raw_spin_unlock
dmu_read_ui_dbuf [zfs]
zfs_read [zfs]
vfs_read
ksys_read
do_syscall
entry_SYSCALL_64_after_hwframe

Wish I could be more precise, but it's complicated to bisect in a production machine, and save dumps/logs when the root filesystem in the machine is dead. :rofl:

I can try with some commit reverted if someone can point me a possible culprit or open an issue if it will be more productive.

EDIT: Unable to reproduce in previous e4d970c.

[sand-bit]

@pedrohlc-mindlab Could you clarify which platform you're on? I'm assuming a BSD platform since you're using ZFS as the boot partition (at least ubuntu doesn't recommend mixing system/self-built zfs bits).

[PedroHLC]

@pedrohlc-mindlab Could you clarify which platform you're on? I'm assuming a BSD platform since you're using ZFS as the boot partition (at least ubuntu doesn't recommend mixing system/self-built zfs bits).

Sorry, I wrote that in a hurry during work hour, and I even posted with the wrong account.

The system is a single zpool over two identical NVMEs (2x1TB ADATA SX8100NP, created from this bash script), "desktop" output from this NixOS configuration -- kernel is 5.19.10-lqx1.

The times it crashed had these things in common: I was running a nextjs hot-reloadable ts-node server for development, browser open, sublime + TabNine, while hearing music on Spotify.

I have some spare time now, I'll try to reproduce and dump+log the error to /boot

[ixhamza]

@tonyhutter We would also want to add https://github.com/openzfs/zfs/pull/13476/commits/f163a44d78b5b098e8cab800709d2423530f8cd7 as well, which is an improvement over https://github.com/openzfs/zfs/commit/a7bd20e309a4b45b18b1da8e379f5826debe4870. Thanks!

[gdevenyi]

When this release happens I hope #13755 and #13612 are addressed with the release notes

[tonyhutter]

@PedroHLC @pedrohlc-mindlab I removed this from the 2.1.6 patchlist:

505df8d13 Dynamically size dbuf hash mutex array

Can you give it a try and let us know if you still see the issue?

[ryao]

I had a message from a distribution developer regarding an issue one of his his end users had with 2.1.6 on the weekend. It appears to be the same issue @pedrohlc-mindlab is having. I will try passing a message back to try the revised patch set.

[ryao]

I was just told that person is @PedroHLC. My mistake.

[tonyhutter]

@ixhamza f163a44 would be great, but I'd like to stabilize and wrap up this release soon. I'd recommend we defer f163a44 to either the next point release or major release.

[ixhamza]

@tonyhutter https://github.com/openzfs/zfs/commit/f163a44d78b5b098e8cab800709d2423530f8cd7 fixes huge delays and spikes during writes that were introduced by the https://github.com/openzfs/zfs/commit/a7bd20e309a4b45b18b1da8e379f5826debe4870, i.e., https://github.com/openzfs/zfs/commit/f163a44d78b5b098e8cab800709d2423530f8cd7 is the correct version of https://github.com/openzfs/zfs/commit/a7bd20e309a4b45b18b1da8e379f5826debe4870. Sorry for the delayed request but I have checked and it should be straightforward to apply this.

[ryao]

There are two moderately serious issues that I very recently discovered in the code whose patches I feel should be added to this after they are merged to master:

https://github.com/openzfs/zfs/pull/13944 https://github.com/openzfs/zfs/pull/13954

I am trying to exercise restraint in making requests for things to be added. These two are the only additional two that I plan to ask to be included at this point since they fix potentially serious undefined behavior inside the kernel. I do not plan to ask about any others unless they are either similarly serious or more serious. That said, they still need to be merged to master before they could be included.

[PedroHLC]

@PedroHLC @pedrohlc-mindlab I removed this from the 2.1.6 patchlist:

505df8d Dynamically size dbuf hash mutex array

Can you give it a try and let us know if you still see the issue?

Sure, I wasn't able to reproduce it this weekend, but I can try today since I'll be resuming Friday's work.

[tonyhutter]

163a44 fixes huge delays and spikes during writes that were introduced by the a7bd20e, i.e., f163a44 is the correct version of a7bd20e.

@ixhamza thanks for the info. I didn't make the connection that those two commit were related. I pulled in f163a44