SPDX: fix incorrect license tags
These already had SPDX tags and so were misidentifed as CDDL-1.0 in
eb9098ed47. Return them to what they were supposed to be.
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18914
spdxcheck: detect multiple tags within a file
Since a single tag can express multiple options, enforce that only a
single tag be present.
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18914
license: fix CDDL license headers identified by spdxcheck
All missed in 4b05f927dc by virtue of being "close enough".
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18914
spdxcheck: check CDDL-1.0 tags for correct license boilerplate
Now that we have a preferred, canonical form, we can enforce it.
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18914
mmp: tell a failed uberblock claim apart from remote activity
When the claim could not write to every device the config expects
present, spa_activity_check_claim() replaced the error from
mmp_claim_uberblock() with EREMOTEIO, so an operator whose peer died
together with its mirror legs was told another host holds the pool,
which sends them looking for a host that is not there.
Report the cause instead. A shortfall has two causes worth telling
apart, so mmp_claim_uberblock() now counts the writes it issues
alongside the ones that succeed. A leaf the config expects present but
which cannot be written is never issued one, so too few issued means a
device is absent, which persists across retries and is what
"zhack mmp reclaim" recovers; that returns ENODEV. Enough issued but
too few good means the writes reached present devices and failed, which
a retry may clear; that stays EIO. The issued count is gated exactly as
the good count is so the two describe the same set of leaves.
Both get a case in spa_ld_activity_result() and both still return
[28 lines not shown]
ZTS: add coverage for zhack mmp reclaim
Six scenarios: a stranded pool is recovered and the claim then accepts
it, a live host sharing a leg is still refused, both top-level vdevs
are counted after a recovery, a pool without multihost is left alone,
a log vdev leg is not touched, and a raidz member is not touched.
Every recovery assertion re-imports as a third hostid. zhack exports
cleanly under its own hostid, so importing again as the same host
takes the exported-and-matching-hostid path, skips the activity check
entirely, and would leave the claim unexercised and the test vacuous.
The assertions read req_writes and good_writes from the claim's own
dbgmsg line, which 20176224e added. That is the only observable of
the claim arithmetic, at the cost of coupling the test to a debug
message this change does not control.
mmp_pool_destroy() used a bare "pgrep zhack", which matches any
process whose name merely contains zhack. A ksh script named
[6 lines not shown]
zhack: add "mmp reclaim" to recover a pool stranded by MMP
When a host fails together with the mirror legs attached to it, the
surviving labels still describe those legs as present, so the MMP
uberblock claim keeps demanding a write to every one of them and no
later import can satisfy it. The pool cannot be imported by any host
again.
Add "zhack mmp reclaim", which imports once with the claim's required
write count relaxed for the mirror legs this host cannot open, marks
those leaves offline so that the ordinary imports which follow
succeed, and exports.
The relaxation is confined to userspace. mmp_claim_relaxed is
declared under #ifndef _KERNEL, and module/Kbuild.in builds the module
with -D_KERNEL, so the flag cannot exist in a kernel module. libzpool
does not define _KERNEL and so gets the check. This follows the
zfeature_checks_disable pattern zhack already uses around the same
import, and is stronger, since that flag does exist in the kernel.
[20 lines not shown]
license: update URL in all headers
These all used the short-form header already, but with the old URL.
Update them to the new, to fully match the standard form.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <robn at despairlabs.com>
Closes #18904
license: convert zed/LLNL CDDL headers to a standard form
This was a slightly different structure, but the same information.
Switch to the standard header, but leave the additional copyright
notice.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <robn at despairlabs.com>
Closes #18904
license: convert all "fenced" CDDL headers to standard form
These all used the older, more verbose version of the header. The more
recent short form is clearer and altogether nicer. Use it!
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <robn at despairlabs.com>
Closes #18904
zstream: zstream_queue bug fixes and simplifications
This PR makes several changes to `zstream_queue.c` aimed at
bulletproofing and simplification.
### Remove thread pool spindown
This PR removes code that decomissioned worker threads once the last
remaining queue had completed. The interlock between this operation and
the creation of new queues complicated the locking system significantly
and is known to have introduced at least two subtle locking bugs. With
this change, the thread pool will be created once and retained until
the process exits.
### Remove lock-free queue scoring
The code is designed to work correctly even without scoring threads
holding the queue mutexes of the queues they're examining. However,
I've confirmed through performance testing that lock-free operation
[39 lines not shown]
Fix negative time overflows in DDT pruning
- Account DDT entries created after prune start to histogram bin 0
(now) instead of the last one (long ago) due to negative overflow.
- Return error when requested to prune for more days than passed
since the epoch.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #18886
nvpair: i_get_value_size() string array handling tweak
The strnlen() function needs to be given the length of the remaining
region to behave as intended, but it was given the length of the total
region on packed strings.
Reported-by: Grok 4.5 Build Beta
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Alek Pinchuk <Alek.Pinchuk at connectwise.com>
Signed-off-by: Richard Yao <richard at ryao.dev>
Closes #18877
nvpair: Improve native handling of unterminated strings
This continues the work done in 59dc88602e23a436440e4164c6d9401da8f0dff2
and parallels what is already done for XDR encoding.
Reported-by: Grok 4.5 Build Beta
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Alek Pinchuk <Alek.Pinchuk at connectwise.com>
Signed-off-by: Richard Yao <richard at ryao.dev>
Closes #18876
CodeQL: Flag implicit compare-then-assign in branch conditions
Implicit compare-then-assign in branch conditions is buggy since
developers often mean assign-then-compare, but sometimes actually mean
compare-then-assign. GCC's -Wparentheses was originally meant to catch
assignment in place of comparison, requiring an extra set of parentheses
to turn this off. This had the happy coincidence of making developers
explicit about assign-then-compare vs compare-then-assign.
An outer level of extra parentheses will inhibit -Wparentheses warnings.
This often results in assign-then-compare being made explicit, but
instead of turning `if (x = foo() < 0)` into `if ((x = foo()) < 0)`, a
developer might write `if ((x = foo() < 0))`, which turns off the
warning, without fixing the problem. This happened in openzfs/zfs#18874.
There are other potential variations, such as `if ((x = (foo()) < 0))`,
which also suppresses GCC's warning, but fails to actually do anything
since the intended explicit parentheses to specify compare-then-assign
are around the right operand of the boolean operator, rather than around
the boolean operator, yet we have the additional parentheses needed to
[43 lines not shown]
Parallelize brt_pending_apply() for multiple vdevs
Unlike DDT, sorted by the checksum prefix, BRT uses salted hash.
It means that we can't pre-sort the accesses for better caching,
and with BRT size getting bigger that dbuf cache, there is a high
chance that each lookup will require a 8KB block decompression.
It is not as bad as for 32KB DDT block, but still.
Since each vdev in BRT is completely independent, running several
brt_pending_apply_vdev() instances in parallel should increase both
CPU and (in case of ARC miss) pool utilization.
Reviewed-by: Rob Norris <rob.norris at truenas.com>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #18860
Parallelize cloning of deduplicated blocks
When block cloning is used on deduplicated blocks, the additional
references are accounted in DDT instead of BRT. Previously it was
done by brt_pending_apply_vdev(), calling ddt_addref() for each
block with DEDUP flag in the block pointer. The process was single-
threaded, and caused a bottleneck in the sync context, since with
large DDT ddt_addref() needs to decompress 32KB ZAP leaf for each
added block.
This patch changes how pending blocks with DEDUP flag are handled.
Inspead of storing them in per-vdev bv_pending_tree, sorted by
offset (which sorting makes no sense for later DDT updates), store
them in one of 16 AVL trees, sharded and sorted by their checksum
prefix. This way we can simultaneously reduce lock contention on
pools with small number of vdevs, process the shards by separate
sync threads accessing/decompressing different ZAP leaves, and
access ZAP leaves in hash order to increase chances of decompressed
dbuf cache hits.
[9 lines not shown]
arc: add a few invariant checks in release builds
Convert a couple ASSERTs invariants to VERIFYs to enforce them
in release builds to be able to root-case #18782 kernel panic,
whenever it happens again.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Andriy Tkachuk <atkachuk at wasabi.com>
Closes #18840
(cherry picked from commit 023d44b9ef68f1eff6b97d98c6a16cdb4b93cf76)
arc: fix race between arc_release() and arc_read_done()
Consider the following scenario:
1. arc_release() is called on hdr with one buf, but which has
IO_IN_PROGRESS (reading more raw data in encypted pool while
keeping decrypted data in buf).
2. arc_release() moves hdr to anon state and discards its identity.
3. arc_read_done() is called, adds the 2nd buf to hdr, increasing
b_refcnt to 2.
Now we have hdr in anon state with two bufs and without identity.
Or here's a racing scenario:
1. arc_release() checked that hdr is not in anon state, but before
taking hash_lock
2. arc_read_done() takes hash_lock, moves hdr to anon state, in
case of an error.
[25 lines not shown]
Linux: rewrite snapshot automount facility
This commit has a near-total rewrite of the snapshot mount/expire
facility, to try and modernise it, fix a lot of structural and
implementation issues and improve performance.
Because its a change in architecture, not just implementation, it's not
really been possible to incrementally move the implementation from the
old to the new. So, the source commentary is intended as a standalone
description of how it all works, while this commit message describes
what's changed for readers who know the previous version.
The main conceptual difference is that the previous version assumed that
there would only ever be one mount per snapshot dataset at most, and
that that mount would be on the matching `.zfs/snapshot/<snapname>`
mountpoint, and that this subsystem would be the one responsible for
unmounting. Those limitations just don't exist, as Linux mounts can be
duplicated, moved around, attached to other namespaces, even deleted,
without actually removing access to the filesystem and all without the
[100 lines not shown]
Linux 5.19/6.17: handle differences in how to flush delay workqueue
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
Linux 6.3: follow_down() gains flags arg
We need the flags arg to trigger the snapshot mount. For earlier
kernels, we can emulate it with vfs_path_lookup()
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
Linux 6.18 compat: vfs_parse_fs_string() takes 3 args
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: test that a foreign mount on the snapdir can be detached
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: test that ongoing access doesn't prevent snapdir detach
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: test multiple snapshot automounts via multiple base dataset mounts
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: test snapdir shutdown behaviour when automount is moved elsewhere
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: test snapdir shutdown behaviour when automount is bound elsewhere
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: snapdir_admin_create_destroy: use makedir helper
The rust-coreutils (uutils) mkdir calls llistxattr() immediately after
calling mkdir(), which triggers the automount, and so prevents rmdir
from seeing and deleting the mountpoint dir.
Switching over to our helper that we know only calls mkdir() fixes that
up.
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847
ZTS: makedir helper command
The system mkdir command may do other things besides the mkdir()
syscall, which is a problem when we're trying to test the syscall
specifically. This helper only does the mkdir() syscall.
Sponsored-by: TrueNAS
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Rob Norris <rob.norris at truenas.com>
Closes #18847:w
