ZIO: Batch vdev children completions
A vdev child ZIO's life after return from the block layer is three cheap
pipeline stages, yet each child costs its own taskq dispatch and context
switch to get there, only to decrement the parent's child count and die.
Even with increased number of taskqueues this can create a huge lock
contention and scheduler overheads, especially on large systems.
To avoid that extra cost collect the leaf children of a RAIDZ, dRAID or
mirror parents as they come back, and only once the last one is in,
process all of their completions, followed by the parent's one, all on
a single thread.
This optimization is mutually exclusive with I/O scheduler, since
delayed completions there may lead to a deadlock, but for that case I
have somewhat alike optimization idea later.
With this change my tests of 32KB block writes to 3x 5-wide NVMe RAIDZ1
on 64-core system show throughput increase from 13 GiB/s to 17 GiB/s,
[4 lines not shown]
zfs-tests: add zhack mos leak/reclaim coverage
- Add a new cli_root zhack test for the mos leak/scan/reclaim flow.
- Verify dry-run leak/reclaim do not change reclaimable leak counts.
- Verify write-mode leak adds the expected reclaimable clone and
space map objects, and write-mode reclaim removes them.
- Verify reclaim idempotency by running write reclaim twice and
asserting the second pass reports "nothing to reclaim".
- Export/import the pool around write-mode operations to match the
zhack mos command import requirements.
- Register the new test in tests/zfs-tests/tests/Makefile.am and
tests/runfiles/common.run.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Martin Minkus <martin.minkus at gmail.com>
Closes #18907
zhack: add mos leak subcommand for local repro
- Add `zhack mos leak` to create synthetic leaked MOS objects for
testing scan/reclaim behavior on disposable pools, mirroring the
existing `zhack metaslab leak`.
- Support `-c` and `-s` counts for leaked DSL clone maps and
leaked SPA space maps, with dry-run default and `-w` apply.
- Print the created object IDs so test scripts can assert expected
candidates and post-reclaim cleanup.
- Document the new subcommand in zhack.1.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Martin Minkus <martin.minkus at gmail.com>
Closes #18907
zhack: add mos scan/reclaim for leaked MOS objects
- Add `zhack mos scan` to enumerate reclaim candidates for
unreferenced empty DSL clone maps and zeroed space maps.
- Add `zhack mos reclaim` with dry-run by default; `-w` applies
frees in sync context using `zap_destroy` and `space_map_free_obj`.
- Enable readable spacemaps during readonly analysis to avoid
false positives when counting referenced metaslab space maps.
- Document the new subcommands in zhack.1.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Martin Minkus <martin.minkus at gmail.com>
Closes #18907
zpool: things get messy with 0 columns
zpool iostat with terminal reporting 0 columns will print out
a very long line. Use columns 80 when query reports 0.
Reviewed-by: Tony Hutter <hutter2 at llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Toomas Soome <tsoome at me.com>
Closes #19051
ZTS: wait for the zhack pids mmp_concurrent_import started
verify_zhack() looks up the processes to wait for by name:
ZHACKPIDS=$(pgrep zhack)
for pid in $ZHACKPIDS; do
wait $pid
so it only waits for the zhacks that pgrep happens to see. One that has
not reached its exec() yet is still named after the shell that forked
it, and one that has already exited is gone; either way the test
continues while a zhack may still hold the pool imported, and the
import_activity_check() that follows then finds activity where it
requires mmp_result: 0. A zhack missed this way is also not counted,
so IMPORT_COUNT can silently disagree with what actually happened.
A CI run failed this way. Two zhacks were started, only one was
reported, the other was still running at the end and was killed by
cleanup:
[21 lines not shown]
ZTS: let the zvol settle before alloc_class_016_pos destroys the pool
The test writes 10M to a zvol and then destroys the pool a few tens of
milliseconds later:
cannot destroy 'testpool': pool is busy
ERROR: zpool destroy -f testpool exited 1
Closing the zvol makes the kernel emit a change uevent, udev opens the
device to scan it, and a destroy issued while that scan is in flight
gets EBUSY. The test already waits for udev after creating the volume
but not after writing to it, and it destroys the pool with log_must
rather than log_must_busy.
Wait for udev again after the write, and destroy the pool the way
destroy_pool() does, retrying while it reports "busy".
Verified in a VM. The race does not reproduce on an idle machine, so
udev was imitated by holding the zvol open for three seconds before the
[7 lines not shown]
zcp: support bookmarks in zfs.sync.destroy
Teach the channel-program synctask destroy path to recognize bookmark
targets and route them through the existing bookmark-destroy
implementation. This enables zfs.sync.destroy() to remove bookmarks from
Lua channel programs, matching the behavior already available for
datasets and snapshots. A functional regression test was added to cover
bookmark destruction from a channel program.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Stephen Warren <swarren at wwwdotorg.org>
Closes #18992
Revert "Linux: avoid prefaulting under the ZFS range lock"
This reverts commit 43bb1614d50fee71b8ada14ca7b3a0d968a26263.
Signed-off-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Issue #18872
Issue #19046
Do not complete a device removal that hit IO errors
spa_vdev_remove_thread() checks vca_read_error_bytes and
vca_write_error_bytes at the end of each metaslab it copies and sets
svr_thread_exit, so that the removal is cancelled instead of completed.
Both counters are incremented from the copy zio callbacks in
spa_vdev_copy_segment_read_done() and
spa_vdev_copy_segment_write_done(), so the errors of the segments
copied last can arrive after that check has already run. The loop then
ends with svr_thread_exit still B_FALSE and the thread calls
vdev_remove_complete(), dropping the vdev even though part of its data
was never written to the new location.
Write errors are the way to hit this. A write error is only known once
the write completes, while a read error is recorded before the write it
feeds is even issued, so read errors are almost always seen in time.
With enough data to copy, the errors of one metaslab are noticed while
the next one is being copied, which is why this mostly goes unnoticed;
it is the errors of the metaslab copied last that are missed.
[15 lines not shown]
Increasing ZTS timeout as some PRs exceed the limit
Follow up work to better balance the work between the available
CI VMs is being evaluated to speed things up, but for the moment
increase the limit to prevent these timeout failures.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: tiehexue <tiehexue at hotmail.com>
Closes #18997
Wait for the initialize and trim threads to exit
An initializing thread marks the vdev VDEV_INITIALIZE_COMPLETE, drops
vdev_initialize_lock, syncs out the new state with txg_wait_synced()
and only then clears vdev_initialize_thread. spa_vdev_activity_in_
progress_impl() looks at the state alone:
boolean_t in_progress = (activity == ZPOOL_WAIT_INITIALIZE) ?
(vd->vdev_initialize_state == VDEV_INITIALIZE_ACTIVE) :
(vd->vdev_trim_state == VDEV_TRIM_ACTIVE);
so "zpool wait -t initialize" and "zpool initialize -w" return while
the thread is still in that txg wait. A command issued right after
them then hits the vdev_initialize_thread != NULL checks in
spa_vdev_initialize_impl() and fails with EBUSY, both for uninit and
for starting another initialization:
cannot initialize '/var/tmp/zpool_disk2.dat': currently initializing
[16 lines not shown]
ZTS: cover change-key on a dataset whose key does not match its root
An incremental raw receive onto a dataset that was rewrapped locally
with 'zfs change-key -i' leaves it pointing at the local encryption root
while carrying the sending root's key material. Until the previous
commit, running 'zfs change-key' on such a dataset, or on its encryption
root, panicked in syncing context.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Michael Heller <75820586+mkhllr at users.noreply.github.com>
Closes #17425
Closes #18969
dsl_crypt: validate every key change-key will rewrap
spa_keystore_change_key_sync_impl() recurses through a dataset and its
children and VERIFY0()s spa_keystore_dsl_key_hold_dd() on each one,
while spa_keystore_change_key_check() only checks that the wrapping key
of the target's encryption root is loaded. A dataset whose DSL Crypto
Key can no longer be unwrapped with that wrapping key therefore turns a
'zfs change-key' into a panic in syncing context, which leaves txg_sync
blocked and the pool unusable until the machine is rebooted:
PANIC at dsl_crypt.c:1475:spa_keystore_change_key_sync_impl()
Walk the same dsl dirs in the check function and hold every DSL Crypto
Key the sync function will rewrap, so an unusable key is reported to
the caller as EACCES instead.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Michael Heller <75820586+mkhllr at users.noreply.github.com>
Closes #17425
Closes #18969
ZTS: bound ctime_001_pos by the time that actually elapsed
For each operation ctime.c reads the timestamp, sleeps two seconds,
runs the operation, reads the timestamp again and requires the
difference to be between 2 and 10 seconds. The upper bound is a
statement about how long the operation may take, not about the
timestamp: when the machine is loaded enough for a creat() to take ten
seconds, the timestamp is updated correctly and the test still fails.
That bound has already been raised once, from 4 to 10 seconds, in
cc210862d ("ZTS: ctime_001_pos increase tolerance") for exactly these
false positives. It is now being hit again at 12 seconds:
st_mtime: BAD time change: t1(1788433492), t2(1788433504)
Take the wall clock before the sleep and after the operation, and
require the new timestamp to fall in that window instead. The
operation cannot have run earlier than two seconds after the first
reading (the sleep) nor later than the second one, so the check no
[11 lines not shown]
Linux: avoid prefaulting under the ZFS range lock
zfs_write() prefaults only the first transaction-sized chunk before
taking the file range lock. A larger write faults each later chunk
while that lock is held. If its source maps the same ZFS file, the
fault enters zfs_getpage() and waits forever for the lock held by the
writing thread.
Keep prefaulting bounded to one transaction-sized chunk and make Linux
iterator copies honor uio_fault_disable. Both the normal DMU path and
the full-block ARC buffer path now copy without page faults while the
range lock is held. If a later source page is not resident, preserve
any completed data and report a Linux short write instead of faulting or
retrying under the lock.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: nexicturbo <turbonexic at gmail.com>
Closes #18135
Closes #18872
log_spacemap: make zfs_log_sm_blksz tunable
Make the block size used for the log space map feature's space maps
settable at runtime.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Reviewed-by: Alexander Motin <alexander.motin at TrueNAS.com>
Signed-off-by: Christos Longros <chris.longros at gmail.com>
Closes #18974
Don't fail a release because a deferred snapshot is still busy
dsl_dataset_user_release_check_one() fails the whole release with
EBUSY when the last hold comes off a snapshot that is marked for
deferred destruction and is still long held. Mark one while it is
idle, with "zfs hold t pool/fs at snap" and then "zfs destroy -d
pool/fs at snap", mount it afterwards by listing .zfs/snapshot, and
"zfs release t pool/fs at snap" reports "dataset is busy" and leaves the
tag where it was. The caller is now sitting on a tag they cannot
drop until the mount goes, and the best-effort unmount the release
path already does cannot take a mount away from an open file.
The check was there because the release is what destroys the
snapshot, and destroying one that something still holds is not on.
Where an owner is what holds it, that is no longer the only way it
gets destroyed: the owner ends at dsl_dataset_disown(), which asks
for the sweep, so the release can drop the tag and leave the mark to
be collected in the ordinary way. Every other long hold keeps
failing the release as before. Nothing would come back for the mark
[20 lines not shown]
Defer destruction of a snapshot that a mount is holding open
On Linux, "zfs destroy -d" on a snapshot automounted under
.zfs/snapshot with a file still open fails with EBUSY, exactly the
way the plain destroy does, and leaves defer_destroy off. The -d
option is documented as marking whatever it cannot destroy right
away, so there is currently no way to say "get rid of it when you
can" about a snapshot someone is reading. Issue #16339.
dsl_destroy_snapshot_check_impl() turns away any long-held snapshot
before it looks at the defer flag, and a snapshot is long held for as
long as it is mounted. Taking the mount away instead is not on the
table: zfsctl_snapshot_unmount() invalidates the snapdir dentry,
which detaches the mount, but the dataset stays owned until the last
open file goes, and the 20ms it then waits is not enough for a file
someone is still reading. FreeBSD never lands here, since its
zfsctl_snapshot_unmount() goes through dounmount() with MS_FORCE.
So let the snapshot be marked, and destroy it once whatever was
[44 lines not shown]
ZTS: cover a meta-dnode range colliding with a redaction entry
The collision needs a redaction entry for an object whose number is an
exact multiple of DNODES_PER_BLOCK, at block zero, while every object
in that dnode block is free in the sending snapshot. The reporter's
script reaches it by chance, because objects are allocated in chunks
which are dnode block aligned, and takes a few attempts.
Build it deliberately instead: create enough files that some aligned
dnode block is filled entirely by them, search for one, redact the
file owning the first object in it, then free the whole block before
taking the sending snapshot. Both orderings of the tie are covered,
one with the redaction list on --redact and one with it reached from
the bookmark on an incremental.
The dataset is created with dnodesize=legacy. The search wants an
aligned run of 32 consecutive object numbers, which holds only while a
dnode occupies one slot; with larger dnodes they are spaced two, four
or eight apart and no such run exists.
[8 lines not shown]
dmu_send: a meta-dnode range does not start where an object does
send_range_start_compare() positions a meta-dnode range, whose blkids
count dnode blocks, at the object its first dnode block covers:
objequiv becomes start_blkid * DNODES_PER_BLOCK and l0equiv is forced
to zero. That places it correctly against the objects it covers, but
it also makes it compare equal to the first block of the object at
that exact boundary, because both sides then have the same objequiv
and an l0equiv of zero. The two do not start at the same place; their
blkids are not even in the same units.
find_next_range() relies on that comparison meaning what it says. The
loop which computes first_change skips DMU_META_DNODE_OBJECT, so a
meta-dnode range never lowers it, while the loop which then advances
every range that starts alongside the one being returned does not skip
it, and applies an object-relative blkid to a range counting dnode
blocks. On a debug build that trips
VERIFY3U(first_change, >, ranges[i]->start_blkid) failed (1 > 4)
[51 lines not shown]
ZTS: drop stale expected-failure masks
The zts-report 'maybe' list suppresses results for tests that are
expected to fail or skip, so a real regression in any of them is
reported as expected and never fails CI. Thirty-eight of those entries
no longer describe anything observable. Each test below was run ten
consecutive times on Linux and passed every time.
Twelve referenced GitHub issues that have since been closed:
cli_root/zfs_get/zfs_get_009_pos #5479
cli_root/zpool_destroy/zpool_destroy_001_pos #6145
cli_root/zpool_import/zpool_import_missing_003_pos #6839
cli_root/zpool_upgrade/zpool_upgrade_004_pos #6141
history/history_004_pos #7026
history/history_006_neg #5657
reservation/reservation_008_pos #7741
reservation/reservation_018_pos #5642
snapshot/snapshot_009_pos #7961
[66 lines not shown]
ZTS: make raidz expansion pause wait actually wait
The three raidz expansion tests pause a reflow by setting
RAIDZ_EXPAND_MAX_REFLOW_BYTES, then call wait_expand_paused() before
snapshotting the vdevs. That helper polled for progress with:
zpool status $TESTPOOL | grep 'copied out of' | awk '{print $1}'
but "copied out of" is the device removal wording printed by
print_removal_status(). A raidz expansion is reported by
print_raidz_expand_status() as:
223M / 320M copied at 2.23M/s, 69.57% done, 00:00:43 to go
The grep therefore never matched. Both variables stayed empty, the
loop's "$oldcopied != $newcopied" test compared "" against "" and
returned after a single sleep, so the helper waited about two seconds
regardless of what the reflow was doing. Both the expansion wording
and this grep arrived together in 5caeef02f ("RAID-Z expansion
[26 lines not shown]
ZTS: fix and re-enable zpool_import_missing_003_pos
The test compared a checksum captured before the pools were exported
against one captured after importing them, but the two were read in
different ways:
read -r checksum1 < <(cksum $MYTESTFILE) # whole line
read -r checksum2 _ < <(cksum $mymtpt/$file) # first field only
With a single variable, read(1) assigns the entire line, so checksum1
held "<sum> <size> <path>" while checksum2 held just "<sum>". The
comparison could therefore never succeed:
ERROR: [ 2652792171 85331 libtest.shlib = 2652792171 ] exited 1
This dates back to 9423c932d ("tests: replace sum(1) with cksum(1)"),
which switched from sum(1) to cksum(1). sum(1) prints two fields and
cksum(1) prints three; the trailing _ was added at the second call
site but not at the first.
[14 lines not shown]
ZTS: fix mkbusy kill/pgrep race in zfs_destroy_001_pos, _005_neg
Both tests killed their accumulated mkbusy pidlist and then immediately
asserted "log_mustnot pgrep -fl mkbusy". That check is racy: kill(2)
only queues the signal, so the target still has to be scheduled to take
it, and it then lingers as a zombie until init reaps it. mkbusy
daemonizes, so the test shell is not its parent and cannot wait(2) for
it -- and pgrep lists the process for that whole window, zombie
included.
Add kill_mkbusy() to zfs_destroy_common.kshlib: kill the pidlist, poll
kill -0 for each pid until it is gone (up to 5s per pid), and only then
run the "no mkbusy anywhere" leak check the tests already had. Scoping
the wait to the pids we killed keeps the leak check meaningful -- a
stale mkbusy from elsewhere is still reported, rather than silently
absorbed by a global wait loop -- and names the offending pid when a
kill really does fail to take.
Convert all 5 call sites in the two tests. The leak check now runs
[10 lines not shown]
dnode: fix heap-use-after-free in dnode_rele_and_unlock()
The ZFS_DEBUG-only assert in dnode_rele_and_unlock() dereferenced dnh,
the dnode's handle, after mutex_exit(&dn->dn_mtx), below the comment
stating "dnode could get destroyed at this point, so don't use it
anymore".
For an objset's special dnodes that is a real use-after-free. They have
no dnode block (dn_dbuf == NULL), so nothing pins their handle; it is
embedded in the objset_t. Only dn_nodnholds orders a release against
teardown, and this function is what signals it: on the last hold it
broadcasts and drops dn_mtx, freeing dnode_special_close() to destroy
that handle and, via dmu_objset_evict_done(), the objset it lives in.
The assert then reads it.
Read the zrlock ownership before dropping dn_mtx instead, where dn_mtx
still blocks dnode_special_close() and, for an ordinary dnode, the
reference from dbuf_add_ref(db, dnh) is not yet released.
[5 lines not shown]
Linux: drop cached pages in zfs_rezget()
After zfs rollback (or zfs recv -F) of a mounted dataset,
zfs_resume_fs() calls zfs_rezget() for every surviving znode. On Linux
it reloads the SA attributes but never touches the inode's page cache.
A file whose object number, generation and size are unchanged but whose
data blocks differ keeps serving pre-rollback data from pages that were
resident (the file had been mmap()ed) and later refreshed by
update_pages() from a write(). Both mappedread() and mmap() return the
stale pages; reading through .zfs/snapshot is correct, so the on-disk
data is fine.
The FreeBSD port handles this by calling vn_pages_remove_valid() at the
start of zfs_rezget(). Do the equivalent on Linux with
truncate_inode_pages() before reloading the znode, so pages are dropped
on the error paths as well. Dirty pages are discarded on purpose: their
content belongs to the state being rolled back, and writeback is blocked
on z_teardown_lock during suspend/resume anyway. Live mappings are
unmapped and fault the correct data back in through zfs_getpage().
[4 lines not shown]
unit/btree: test the negative cases
Introduce the two negative tests, insert_duplicate and remove_missing,
from tests/zfs-tests/cmd/btree_test.c to the unit tests.
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Christos Longros <chris.longros at gmail.com>
Closes #19009
arc: apply arc_min tunables set before arc_init() has run
Tunables are applied as each parameter is registered, and zfs_arc_min is
registered before zfs_arc_max. So param_set_arc_min() can run while
arc_c_max is still zero, and its upper bound check rejects every
non-zero value:
Setting sysctl vfs.zfs.arc.min failed: 22
The value is dropped and the ARC floor falls back to allmem/32.
param_set_arc_max() is not affected, because it tests a lower bound
against arc_c_min, which a zero value satisfies.
Skip the upper bound while arc_c_max is zero and store the raw value.
arc_init() expects this: arc_set_limits() sets both limits, and the
arc_tuning_update() call after it applies zfs_arc_min.
Storing the value exposes a second bug. The other arc handlers call
arc_tuning_update() unconditionally, so one registered before arc_init()
[14 lines not shown]
Write uberblocks only to vdevs used by the txg
Instead of picking random top-level vdevs to write the uberblock to,
prefer the ones actually written during this txg. This allows idle
vdevs to stay asleep, which is important for pools with spun-down
HDDs.
If fewer than SPA_SYNC_MIN_VDEVS were written, top up from special
and dedup vdevs, which are expected to have no seek penalty. Pools
without those classes keep the old behavior of topping up from any
vdev, preserving the uberblock redundancy where there is nothing to
gain by reducing it.
While here, pass spa explicitly to vdev_config_sync() and
vdev_uberblock_sync_list() instead of deriving it from svd[0].
Reviewed-by: Brian Behlendorf <behlendorf1 at llnl.gov>
Signed-off-by: Alexander Motin <alexander.motin at TrueNAS.com>
Closes #19003
