Paul Dagnelie [Thu, 6 Apr 2023 17:29:27 +0000 (10:29 -0700)]
Storage device expansion "silently" fails on degraded vdev
When a vdev is degraded or faulted, we refuse to expand it when doing
online -e. However, we also don't actually cause the online command
to fail, even though the disk didn't expand. This is confusing and
misleading, and can result in violated expectations.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Matthew Ahrens <mahrens@delphix.com> Reviewed-by: Mark Maybee <mark.maybee@delphix.com> Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Closes 14145
Alexander Motin [Wed, 5 Apr 2023 17:42:22 +0000 (02:42 +0900)]
Fix some signedness issues in arc_evict()
It may happen that "wanted total ARC size" (wt) is negative, that was
expected. But multiplication product of it and unsigned fractions
result in unsigned value, incorrectly shifted right with a sing loss.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Prakash Surya <prakash.surya@delphix.com> Reviewed-by: Paul Dagnelie <pcd@delphix.com> Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Sponsored by: iXsystems, Inc.
Closes #14692
Linux 6.3 compat: writepage_t first arg struct folio*
The type def of writepage_t in kernel 6.3 is changed to take
struct folio* as the first argument. We need to detect this
change and pass correct function to write_cache_pages().
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes #14699
Running `zfs list -o avail rpool` resulted in a core dump.
This commit will fix this.
Run the needed overhead only, when `use_color()` is true.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Wilson <gwilson@delphix.com> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes #14712
наб [Fri, 31 Mar 2023 16:47:48 +0000 (18:47 +0200)]
contrib: dracut: fix race with root=zfs:dset when necessities required
This had always worked in my testing, but a user on hardware reported
this to happen 100%, and I reproduced it once with cold VM host caches.
dracut-zfs-generator runs as a systemd generator, i.e. at Some
Relatively Early Time; if root= is a fixed dataset, it tries to
"solve [necessities] statically at generation time".
If by that point zfs-import.target hasn't popped (because the import is
taking a non-negligible amount of time for whatever reason), it'll see
no children for the root datase, and as such generate no mounts.
This has never had any right to work. No-one caught this earlier because
it's just that much more convenient to have root=zfs:AUTO, which orders
itself properly.
To fix this, always run zfs-nonroot-necessities.service;
this additionally simplifies the implementation by:
* making BOOTFS from zfs-env-bootfs.service be the real, canonical,
root dataset name, not just "whatever the first bootfs is",
and only set it if we're ZFS-booting
* zfs-{rollback,snapshot}-bootfs.service can use this instead of
re-implementing it
* having zfs-env-bootfs.service also set BOOTFSFLAGS
* this means the sysroot.mount drop-in can be fixed text
* zfs-nonroot-necessities.service can also be constant and always
enabled, because it's conditioned on BOOTFS being set
There is no longer any code generated at run-time
(the sysroot.mount drop-in is an unavoidable gratuitous cp).
The flow of BOOTFS{,FLAGS} from zfs-env-bootfs.service to sysroot.mount
is not noted explicitly in dracut.zfs(7), because (a) at some point it's
just visual noise and (b) it's already ordered via d-p-m.s from z-i.t.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Closes #14690
youzhongyang [Fri, 31 Mar 2023 16:46:22 +0000 (12:46 -0400)]
linux 6.3 compat: needs REQ_PREFLUSH | REQ_OP_WRITE
Modify bio_set_flush() so if kernel version is >= 4.10, flags
REQ_PREFLUSH and REQ_OP_WRITE are set together.
Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Youzhong Yang <yyang@mathworks.com>
Closes #14695
Brian Behlendorf [Fri, 31 Mar 2023 16:43:54 +0000 (09:43 -0700)]
Use vmem_zalloc to silence allocation warning
The kmem allocation in zfs_prune_aliases() will trigger a large
allocation warning on systems with 64K pages. Resolve this by
switching to vmem_alloc() which internally uses kvmalloc() so the
right allocator will be used based on the allocation size.
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #8491
Closes #14694
2) When deleting files containing error blocks in those clones (from
"clone" the example above), do not break the check chain.
3) When deleting files in the originating fs before syncing the errlog
to disk, do not break the check chain. This happens because at the
time of introducing the error block in the error list, we do not have
its birth txg and the head filesystem. If the original file is
deleted before the error list is synced to the error log (which is
when we actually lookup the birth txg and the head filesystem), then
we do not have access to this info anymore and break the check chain.
The most prominent change is related to achieving (3). We expand the
spa_error_entry_t structure to accommodate the newly introduced
zbookmark_err_phys_t structure (containing the birth txg of the error
block).Due to compatibility reasons we cannot remove the
zbookmark_phys_t structure and we also need to place the new structure
after se_avl, so it is not accounted for in avl_find(). Then we modify
spa_log_error() to also provide the birth txg of the error block. With
these changes in place we simplify the previously introduced function
get_head_and_birth_txg() (now named get_head_ds()).
We chose not to follow the same approach for the head filesystem (thus
completely removing get_head_ds()) to avoid introducing new lock
contentions.
The stack sizes of nested functions (as measured by checkstack.pl in the
linux kernel) are:
check_filesystem [zfs]: 272 (was 912)
check_clones [zfs]: 64
We also introduced two new tests covering the above changes.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes #14633
Kevin Jin [Tue, 28 Mar 2023 15:43:41 +0000 (11:43 -0400)]
Fix short-lived txg caused by autotrim
Current autotrim causes short-lived txg through:
1. calling txg_wait_synced() in metaslab_enable()
2. calling txg_wait_open() with should_quiesce = true
This patch addresses all the issues mentioned above.
A new cv, vdev_autotrim_kick_cv is added to kick autotrim activity.
It will be signaled once a txg is synced so that it does not change
the original autotrim pace. Also because it is a cv, the wait is
interruptible which speeds up the vdev_autotrim_stop_wait() call.
Finally, combining big zfs_txg_timeout, txg_wait_open() also causes
delay when exporting a pool.
Brian Behlendorf [Tue, 28 Mar 2023 15:19:03 +0000 (08:19 -0700)]
Additional limits on hole reporting
Holding the zp->z_rangelock as a RL_READER over the range
0-UINT64_MAX is sufficient to prevent the dnode from being
re-dirtied by concurrent writers. To avoid potentially
looping multiple times for external caller which do not
take the rangelock holes are not reported after the first
sync. While not optimal this is always functionally correct.
This change adds the missing rangelock calls on FreeBSD to
zvol_cdev_ioctl().
Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #14512
Closes #14641
Reviewed-by: Prakash Surya <prakash.surya@delphix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: George Wilson <gwilson@delphix.com>
Closes #14678
Rob N [Mon, 27 Mar 2023 18:55:54 +0000 (05:55 +1100)]
config: don't link libudev on FreeBSD
FreeBSD has a libudev shim in libudev-devd. If present, configure would
detect it and produce binaries linked against it, even though nothing
used it. That is surprising and unnecessary, so lets remove it.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com>
Closes #14669
Ameer Hamza [Fri, 24 Mar 2023 17:30:38 +0000 (22:30 +0500)]
Update vdev state for spare vdev
zfsd fetches new pool configuration through ZFS_IOC_POOL_STATS but
it does not get updated nvlist configuration for spare vdev since
the configuration is read by spa_spares->sav_config. In this commit,
updating the vdev state for spare vdev that is consumed by zfsd on
spare disk hotplug.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ameer Hamza <ahamza@ixsystems.com>
Closes #14653
Rich Ercolani [Fri, 24 Mar 2023 17:29:19 +0000 (13:29 -0400)]
Drop lying to the compiler in the fletcher4 code
This is probably the uncontroversial part of #13631, which fixes
a real problem people are having.
There's still things to improve in our code after this is merged,
but it should stop the breakage that people have reported, where
we lie about a type always being aligned and then pass in stack
objects with no alignment requirement and hope for the best.
Of course, our SIMD code was written with unaligned accesses, so it
doesn't care if we drop this...but some auto-vectorized code that
gcc emits sure does, since we told it it can assume they're aligned.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: Rich Ercolani <rincebrain@gmail.com>
Closes #14649
George Wilson [Fri, 24 Mar 2023 17:27:07 +0000 (13:27 -0400)]
panic loop when removing slog device
There is a window in the slog removal code where a panic loop could
ensue if the system crashes during that operation. The original design
of slog removal did not persisted any state because the removal happened
synchronously. This was changed by a later commit which persisted the
vdev_removing flag and exposed this bug. If a slog removal is in
progress and happens to crash after persisting the vdev_removing flag to
the label but before the vdev is removed from the spa config, then the
pool will continue to panic on import. Here's a sample of the panic:
This change no longer persists the vdev_removing flag when removing slog
devices and also cleans up some code that was added which is not used.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Matthew Ahrens <mahrens@delphix.com> Reviewed-by: Mark Maybee <mark.maybee@delphix.com> Signed-off-by: George Wilson <gwilson@delphix.com>
Closes #14652
Tino Reichardt [Mon, 13 Mar 2023 22:30:09 +0000 (23:30 +0100)]
Colorize zpool iostat output
Use a bold header and colorize the space suffixes in iostat
by order of magnitude like this:
- K is green
- M is yellow
- G is red
- T is lightblue
- P is magenta
- E is cyan
- 0 space is colored gray
Matthew Ahrens [Fri, 24 Mar 2023 17:20:07 +0000 (10:20 -0700)]
Fix prefetching of indirect blocks while destroying
When traversing a tree of block pointers (e.g. for `zfs destroy <fs>` or
`zfs send`), we prefetch the indirect blocks that will be needed, in
`traverse_prefetch_metadata()`. In the case of `zfs destroy <fs>`, we
do a little traversing each txg, and resume the traversal the next txg.
So the indirect blocks that will be needed, and thus are candidates for
prefetching, does not include blocks that are before the resume point.
The problem is that the logic for determining if the indirect blocks are
before the resume point is incorrect, causing the (up to 1024) L1
indirect blocks that are inside the first L2 to not be prefetched. In
practice, if we are able to read many more than 1024 blocks per txg,
then this will be inconsequential. But if i/o latency is more than a
few milliseconds, almost no L1's will be prefetched, so they will be
read serially, and thus the destroying will be very slow. This can be
observed as `zpool get freeing` decreasing very slowly.
Specifically: When we first examine the L2 that contains the block we'll
be resuming from, we have not yet resumed, so `td_resume` is nonzero.
At this point, all calls to `traverse_prefetch_metadata()` will fail,
even if the L1 in question is after the resume point. It isn't until
the callback is issued for the resume point that we zero out
`td_resume`, but by this point we've already attempted and failed to
prefetch everything under this L2 indirect block.
This commit addresses the issue by reusing the existing
`resume_skip_check()` to determine if the L1's bookmark is before or
after the resume point. To do so, this function is made non-mutating
(the caller now zeros `td_resume`).
Note, this bug likely predates (was not introduced by) #11803.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
Closes #14603
Undirty the dbuf and destroy its buffer when cloning into it.
Coverity ID: CID-1535375 Reported-by: Richard Yao Reported-by: Benjamin Coddington Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Pawel Jakub Dawidek <pawel@dawidek.net>
Closes #14655
Rob N [Fri, 24 Mar 2023 17:14:39 +0000 (04:14 +1100)]
man: add ZIO_STAGE_BRT_FREE to zpool-events
And bump all the values after it, matching the header update in 67a1b037.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Rob Norris <robn@despairlabs.com>
Closes #14665
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: Pawel Jakub Dawidek <pawel@dawidek.net>
Closes #14656
Timothy Day [Wed, 22 Mar 2023 16:22:52 +0000 (12:22 -0400)]
Fix kmodtool for packaging mainline Linux
kmodtool currently incorrectly identifies official
RHEL kernels, as opposed to custom kernels. This
can cause the openZFS kmod RPM build to break.
The issue can be reproduced by building a set of
mainline Linux RPMs, installing them, and then
attempting to build the openZFS kmod package
against them.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Timothy Day <timday@amazon.com>
Closes #14617
Alexander Motin [Sat, 18 Mar 2023 00:31:08 +0000 (20:31 -0400)]
FreeBSD: Remove extra arc_reduce_target_size() call
Remove arc_reduce_target_size() call from arc_prune_task(). The idea
of arc_prune_task() is to remove external references on ARC metadata,
such as vnodes. Since arc_prune_async() is called only from ARC itself,
it makes no sense to create a parasitic loop between ARC eviction and
the pruning, treatening to drop ARC to its minimum. I can't guess why
it was added as part of FreeBSD to OpenZFS integration.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Sponsored by: iXsystems, Inc.
Closes #14639
Note that contrary to the claim in the ZFS disk format specification
that a maximum of 6 levels are possible, 9 levels are possible with
recordsize=512 and and indirect block size of 16KB. In this unusual
configuration, span will be 65. The maximum size of span at 70 can be
reached at recordsize=16K and an indirect blocksize of 16KB.
When we are at this indirection level and are traversing backward, the
minimum value is start, but we cannot calculate that with 64-bit
arithmetic, so we avoid the calculation and instead rely on the earlier
statement that did `*offset = start;`.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-1466214)
Closes #14618
naivekun [Thu, 16 Mar 2023 18:54:10 +0000 (02:54 +0800)]
QAT: Fix uninitialized seed in QAT compression
CpaDcRqResults have to be initialized with checksum=1 for adler32.
Otherwise when error CPA_DC_OVERFLOW occurred, the next compress
operation will continue on previously part-compressed data, and write
invalid checksum data. When zfs decompress the compressed data, a
invalid checksum will occurred and lead to #14463
Attila Fülöp [Wed, 15 Mar 2023 18:13:25 +0000 (19:13 +0100)]
spl: cmn_err_once() should be usable in brace-less if else statements
Commit 11913870 (#14567) added cmn_err_once() by #define'ing a
compound statement but failed to consider usage in a single
statement brace-less if else.
Fix the problem by using the common "do {} while (0)" construct.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes #14629
Tino Reichardt [Wed, 15 Mar 2023 17:41:05 +0000 (18:41 +0100)]
Split functional testings via github action matrix
This commit changes the workflow of the github actions.
We split the workflow into different parts:
1) build zfs modules for Ubuntu 20.04 and 22.04 (~25m)
2) 2x zloop test (~10m) + 2x sanity test (~25m)
3) functional testings in parts 1..5 (each ~1h)
- these could be triggered, when sanity tests are ok
- currently I just start them all in the same time
4) cleanup and create summary
When everything is fine, the full run with all testings
should be done in around 2 hours.
The codeql.yml and checkstyle.yml are not part in this circle.
The testings are also modified a bit:
- report info about CPU and checksum benchmarks
- reset the debugging logs for each test
- when some error occurred, we call dmesg with -c to get
only the log output for the last failed test
- we empty also the dbgsys
Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes #14078
Alek P [Wed, 15 Mar 2023 17:34:10 +0000 (10:34 -0700)]
Improve tests and update man page for healing recv
Fix the manpage. The "SYNOPSIS" section is incorrectly formatted for
receive -c. I also took this opportunity to reword some parts and
fix a run-on sentence in the manpage.
Add large block testing for corrective recv. This adds a new test
that makes sure blocks generated using zfs send -L/--large-block
large-block send flag are able to be used for healing.
Since with unloaded key and errlog feature enabled corruption is not
shown in zpool status #13675 is fixed the zfs_receive_corrective.ksh
test no longer sets -o feature@head_errlog=disabled on pool creation
so that it can also test for regressions related to head_errlog feature.
Note that the zfs_receive_compressed_corrective.ksh and
zfs_receive_large_block_corrective.ksh tests are still creating pools
with -o feature@head_errlog=disabled.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Alek Pinchuk <apinchuk@axcient.com>
Closes #14615
WHR [Tue, 14 Mar 2023 13:25:25 +0000 (21:25 +0800)]
Refactor CONFIG_SPE check on Linux/powerpc
Commit 5401472 adds a check to call enable_kernel_spe and
disable_kernel_spe only if CONFIG_SPE is defined. Refactor this check
in a way similar to what CONFIG_ALTIVEC and CONFIG_VSX are checked, in
order to remove redundant kfpu_begin() and kfpu_end() implementations.
ofthesun9 [Tue, 14 Mar 2023 23:40:55 +0000 (00:40 +0100)]
Fix for mountpoint=legacy
We need to clear mountpoint only after checking it.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: ofthesun9 <olivier@ofthesun.net>
Closes #14599
Closes #14604
Richard Yao [Sat, 11 Mar 2023 18:39:24 +0000 (13:39 -0500)]
nvpair: Use flexible array member for nvpair name strings
Coverity reported possible out-of-bounds reads from doing `((char
*)(nvp) + sizeof (nvpair_t))` to get the nvpair name string. These were
initially marked as false positives, but since we are now using C99
flexible array members elsewhere, we could use them here too as cleanup
to make the code easier to understand.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-977165) Reported-by: Coverity (CID-1524109) Reported-by: Coverity (CID-1524642)
Closes #14612
Richard Yao [Sat, 11 Mar 2023 18:39:24 +0000 (13:39 -0500)]
nvpair: Constify string functions
After addressing coverity complaints involving `nvpair_name()`, the
compiler started complaining about dropping const. This lead to a rabbit
hole where not only `nvpair_name()` needed to be constified, but also
`nvpair_value_string()`, `fnvpair_value_string()` and a few other static
functions, plus variable pointers throughout the code. The result became
a fairly big change, so it has been split out into its own patch.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14612
Richard Yao [Sun, 12 Mar 2023 16:30:21 +0000 (12:30 -0400)]
discover_cached_paths() should not corrupt nvlist string value
discover_cached_paths() will write a NULL into a string from a nvlist to
use it as a substring, but does not restore it before return. This
corrupts the nvlist. It should be harmless unless the string is needed
again later, but we should not do this, so let us fix it.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14612
Richard Yao [Sat, 11 Mar 2023 20:25:04 +0000 (15:25 -0500)]
zpool_valid_proplist() should not corrupt nvpair name string on error
The strings returned from parsing nvlists should be immutable, but to
simplify the code when we want a substring from it, we sometimes will
write a NULL into it and then restore the value afterward. Provided
there is no concurrent access, this is okay, unless we forget to restore
the value afterward. This was caught when constifying string functions
related to nvlists.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14612
Richard Yao [Sat, 11 Mar 2023 18:21:04 +0000 (13:21 -0500)]
Fix possible NULL pointer dereference in nvlist_lookup_nvpair_ei_sep()
Clang's static analyzer complains about a possible NULL pointer
dereference in nvlist_lookup_nvpair_ei_sep() because it unconditionally
dereferences a pointer initialized by `nvpair_value_nvlist_array()`
under the assumption that `nvpair_value_nvlist_array()` will always
initialize the pointer without checking to see if an error was returned
to indicate otherwise. This itself is improper error handling, so we fix
it. However, fixing it to properly respond to errors is not enough to
avoid a NULL pointer dereference, since we can receive NULL when the
array is empty, so we also add a NULL check.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14612
Richard Yao [Sat, 11 Mar 2023 18:17:59 +0000 (13:17 -0500)]
Silence clang static analyzer warnings about stored stack addresses
Clang's static analyzer complains that nvs_xdr() and nvs_native()
functions return pointers to stack memory. That is technically true, but
the pointers are stored in stack memory from the caller's stack frame,
are not read by the caller and are deallocated when the caller returns,
so this is harmless. We set the pointers to NULL to silence the
warnings.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14612
Richard Yao [Tue, 14 Mar 2023 22:00:54 +0000 (18:00 -0400)]
Fix possible NULL pointer dereference in dbuf_verify()
Coverity reported a dereference after a NULL check in dbuf_verify(). If
`dn` is `NULL`, we can just assume that !dn->dn_free_txg, so we change
`!dn->dn_free_txg` to `(dn == NULL || !dn->dn_free_txg)`.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-992298)
Closes #14619
Richard Yao [Fri, 10 Mar 2023 22:42:09 +0000 (17:42 -0500)]
Zero zio_prop_t in flush_write_batch_impl()
After 67a1b0379159c46bcd60a462a2790248046c8804 was merged, coverity
started complaining about an uninitialized scalar variable in
flush_write_batch_impl() due to the new field zp.zp_brtwrite. Upon
inspection, it appears that uninitialized memory was being copied for
non-raw streams, so this is a pre-existing issue. The addition of
zp_brtwrite by the block cloning commit caused Coverity to begin to
notice it.
Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Rob Norris <robn@despairlabs.com> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-1535378)
Closes #14607
Richard Yao [Fri, 10 Mar 2023 22:47:56 +0000 (17:47 -0500)]
Fix uninitialized scalar value read regression in dmu_recv_begin()
da19d919a853ad05ef300fe000e6c96c4db84bcf changed this in a way that
permits execution to reach `if (err == 0)` without initializing err.
This could randomly cause the sync task to not execute. We fix that by
initializing err to zero.
Reviewed-by: George Melikov <mail@gmelikov.ru> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Rob Norris <robn@despairlabs.com> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-1535377)
Closes #14607
Matthew Ahrens [Tue, 14 Mar 2023 21:30:29 +0000 (14:30 -0700)]
ZFS_IOC_COUNT_FILLED does unnecessary txg_wait_synced()
`lseek(SEEK_DATA | SEEK_HOLE)` are only accurate when the on-disk blocks
reflect all writes, i.e. when there are no dirty data blocks. To ensure
this, if the target dnode is dirty, they wait for the open txg to be
synced, so we can call them "stabilizing operations". If they cause
txg_wait_synced often, it can be detrimental to performance.
Typically, a group of files are all modified, and then SEEK_DATA/HOLE
are performed on them. In this case, the first SEEK does a
txg_wait_synced(), and subsequent SEEKs don't need to wait, so
performance is good.
However, if a workload involves an interleaved metadata modification,
the subsequent SEEK may do a txg_wait_synced() unnecessarily. For
example, if we do a `read()` syscall to each file before we do its SEEK.
This applies even with `relatime=on`, when the `read()` is the first
read after the last write. The txg_wait_synced() is unnecessary because
the SEEK operations only care that the structure of the tree of indirect
and data blocks is up to date on disk. They don't care about metadata
like the contents of the bonus or spill blocks. (They also don't care
if an existing data block is modified, but this would be more involved
to filter out.)
This commit changes the behavior of SEEK_DATA/HOLE operations such that
they do not call txg_wait_synced() if there is only a pending change to
the bonus or spill block.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Matthew Ahrens <mahrens@delphix.com>
Closes #13368
Issue #14594
Issue #14512
Issue #14009
Attila Fülöp [Tue, 14 Mar 2023 16:45:28 +0000 (17:45 +0100)]
zcommon: Refactor FPU state handling in fletcher4
Currently calls to kfpu_begin() and kfpu_end() are split between
the init() and fini() functions of the particular SIMD
implementation. This was done in #14247 as an optimization measure
for the ABD adapter. Unfortunately the split complicates FPU
handling on platforms that use a local FPU state buffer, like
Windows and macOS.
To ease porting, we introduce a boolean struct member in
fletcher_4_ops_t, indicating use of the FPU, and move the FPU state
handling from the SIMD implementations to the call sites.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Jorgen Lundman <lundman@lundman.net> Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes #14600
George Melikov [Sat, 11 Mar 2023 00:23:01 +0000 (03:23 +0300)]
ABI files: sync with new Ubuntu release in CI
We may try to build ZFS inside container too,
but let's just sync them for now.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: George Melikov <mail@gmelikov.ru>
Closes #14605
Block Cloning allows to manually clone a file (or a subset of its
blocks) into another (or the same) file by just creating additional
references to the data blocks without copying the data itself.
Those references are kept in the Block Reference Tables (BRTs).
The whole design of block cloning is documented in module/zfs/brt.c.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Christian Schwarz <christian.schwarz@nutanix.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Rich Ercolani <rincebrain@gmail.com> Signed-off-by: Pawel Jakub Dawidek <pawel@dawidek.net>
Closes #13392
Paul Dagnelie [Fri, 10 Mar 2023 17:52:44 +0000 (09:52 -0800)]
Fix incremental receive silently failing for recursive sends
The problem occurs because dmu_recv_begin pulls in the payload and
next header from the input stream in order to use the contents of
the begin record's nvlist. However, the change to do that before the
other checks in dmu_recv_begin occur caused a regression where an
empty send stream in a recursive send could have its END record
consumed by this, which broke the logic of recv_skip. A test is
also included to protect against this case in the future.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Paul Dagnelie <pcd@delphix.com>
Closes #12661
Closes #14568
Low-power [Fri, 10 Mar 2023 17:35:00 +0000 (01:35 +0800)]
Workaround for Linux PowerPC GPL-only cpu_has_feature()
Linux since 4.7 makes interface 'cpu_has_feature' to use jump labels on
powerpc if CONFIG_JUMP_LABEL_FEATURE_CHECKS is enabled, in this case
however the inline function references GPL-only symbol
'cpu_feature_keys'.
ZFS currently uses 'cpu_has_feature' either directly or indirectly from
several places; while it is unknown how this issue didn't break ZFS on
64-bit little-endian powerpc, it is known to break ZFS with many Linux
versions on both 32-bit and 64-bit big-endian powerpc.
Until this issue is fixed in Linux, we have to workaround it by
overriding affected inline functions without depending on
'cpu_feature_keys'.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: WHR <msl0000023508@gmail.com>
Closes #14590
Richard Yao [Fri, 10 Mar 2023 17:34:00 +0000 (12:34 -0500)]
txg_sync should handle write errors in ZIL
The txg_sync thread will see certain buffers in a DR_IN_DMU_SYNC state
when ZIL is writing them out. Then it waits until the state changes, but
has an assertion to check that they were not DR_NOT_OVERRIDDEN. If the
data write failed with an error, ZIL will put it into the
DR_NOT_OVERRIDDEN state. It looks like the code will handle that state
without an issue, so we can just delete the assertion.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@klarasystems.com> Sponsored-By: Wasabi Technology, Inc.
Closes #14283
Richard Yao [Mon, 6 Mar 2023 18:29:36 +0000 (13:29 -0500)]
Suppress clang static analyzer warning in vdev_stat_update()
63652e154643cfe596fe077c13de0e7be34dd863 added unnecessary branches in
`vdev_stat_update()` to suppress an ASAN false positive the breaks
ztest. This had the downside of causing false positive reports in both
Coverity and Clang's static analyzer. vd is never NULL, so we add a
preprocessor check to only apply the workaround when compiling with ASAN
support.
Reported-by: Coverity (CID-1524583) Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Mon, 6 Mar 2023 14:48:42 +0000 (09:48 -0500)]
Refactor loop in dump_histogram()
The current loop triggers a complaint that we are using an array offset
prior to a range check from cpp/offset-use-before-range-check when we
are actually calculating maximum and minimum values. I was about to file
a false positive report with CodeQL, but after looking at how the code
is structured, I really cannot blame CodeQL for mistaking this for a
range check.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Mon, 6 Mar 2023 16:15:37 +0000 (11:15 -0500)]
Suppress static analyzer warning in dmu_objset_create_impl_dnstats()
ae7e7006500ca37c471dd625cd5cbfc590b49885 added an assertion to suppress
a complaint from Clang's static analyzer. Unfortunately, it missed
another way for Clang to complain about this function. This adds another
assertion to handle that.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 5 Mar 2023 07:01:58 +0000 (02:01 -0500)]
Linux: Fix octal detection in define_ddi_strtox()
Clang Tidy reported this as a misc-redundant-expression because writing
`8` instead of `'8'` meant that the condition could never be true.
The only place where we have a chance of this being a bug would be in
nvlist_lookup_nvpair_ei_sep(). I am not sure if we ever pass an octal to
that, but if we ever do, it should work properly now instead of failing.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 5 Mar 2023 06:32:03 +0000 (01:32 -0500)]
Linux: Suppress clang static analyzer warning in zfs_remove()
Clang's static analyzer points out that if we fail to find an extended
attribute directory, but somehow find it when calculating delete_now and
delete_now is true, we will have a NULL pointer dereference when we try
to unlink the extended attribute directory.
I am not sure if this is possible, but if it is, I do not see a sane way
of handling this other than rolling back the transaction and retrying.
For now, let us do an VERIFY_IMPLY(). If this trips, it will stop the
transaction from committing, which will prevent an attribute directory
leak.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 5 Mar 2023 04:48:06 +0000 (23:48 -0500)]
Linux: Silence static analyzer warning in crypto_create_ctx_template()
A CodeChecker report from Clang's CTU analysis indicated that we were
assigning uninitialized values in crypto_create_ctx_template() when we
call it from zio_crypt_key_init(). This occurs because the ->cm_param
and ->cm_param_len fields are uninitialized. Thankfully, the
uninitialized values are only used in the skein via
KCF_PROV_CREATE_CTX_TEMPLATE() -> skein_create_ctx_template() ->
skein_mac_ctx_build() -> skein_get_digest_bitlen(), but that should not
be called from here. We fix this to avoid a possible trap should this
code change in the future.
The FreeBSD version of zio_crypt_key_init() is unaffected.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 5 Mar 2023 03:31:33 +0000 (22:31 -0500)]
Suppress Clang Static Analyzer warning in bpobj_enqueue()
scan-build does not do cross translation unit analysis to realize that
`dmu_buf_hold()` will always set `bpo->bpo_cached_dbuf` to a non-NULL
pointer, so we add an assertion to make it realize this.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 5 Mar 2023 03:23:48 +0000 (22:23 -0500)]
Suppress Clang Static Analyzer warning in dsl_dir_rename_sync()
Clang's static analyzer reports that if we try to rename a root dataset
in `dsl_dir_rename_sync()`, we will have a NULL pointer passed to
strlcpy(). This is impossible because `dsl_dir_rename_check()` will
prevent us from doing this. We add an assertion to silence this warning.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sat, 4 Mar 2023 22:57:01 +0000 (17:57 -0500)]
Cleanup: Remove constant comparisons reported by CodeQL
CodeQL's cpp/constant-comparison query from its security-and-extended
query set reported 4 instances where we have comparions that always
evaluate the same way.
In `draid_config_by_type()`, we have an early `if (nparity == 0)` check
that returns `EINVAL`, making a later `if (nparity == 0 || nparity >
VDEV_DRAID_MAXPARITY)` partially redundant. The later check prints an
error message when parity is 0, but the early check does not. This is
not useful feedback, so we move the later check to the place where the
early check runs to replace the early check.
In `perform_thread_merge()`, we return when `num_threads == 0`. After
that block, we do `if (num_threads > 0) {`, which will always be true.
We remove the `if` statement.
In `sa_modify_attrs()`, we have a loop condition that is `k != 2`, but
at the end of the loop, we have `if (k == 0 && hdl->sa_spill)` followed
by an else that does a break. The result is that k != 2 will never be
evaluated when it is false. We drop the comparison.
In `zap_leaf_array_read()`, we have a for loop condition that is `i <
ZAP_LEAF_ARRAY_BYTES && len > 0`. However, that loop itself is in a loop
that is `while (len > 0)` and while the value of len is decremented
inside the loop, when `len == 0`, it will return, such that `len > 0`
inside the loop condition will always be true. We drop that part of the
condition.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sat, 4 Mar 2023 21:11:49 +0000 (16:11 -0500)]
Suppress Clang Static Analyzer warning in dbuf_dnode_findbp()
Clang's static analyzer reports that if a `blkid == DMU_SPILL_BLKID` is
passed, then we can have a NULL pointer dereference when either
->dn_have_spill or `DNODE_FLAG_SPILL_BLKPTR` is not set. This should not
happen. We add an `ASSERT()` to suppress reports about NULL pointer
dereferences.
Originally, I wanted to use one or two IMPLY statements on
pre-conditions before the call to `dbuf_findbp()`, but Clang's static
analyzer did not understand it.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sat, 4 Mar 2023 20:38:06 +0000 (15:38 -0500)]
Suppress Clang Static Analyzer warning in vdev_split()
Clang's static analyzer pointed out that we can have a NULL pointer
dereference if we ever attempt to split a vdev that has only 1 child. If
that happens, we are left with zero children, but then try to access a
non-existent child. Calling vdev_split() on a vdev with only 1 child
should be impossible due to how the code is structured. If this ever
happens, it would be best to stop execution immediately even in a
production environment to allow for the best possible chance of recovery
by an expert, so we use `VERIFY3U()` instead of `ASSERT3U()`.
Unfortunately, while that defensive assertion will prevent execution
from ever reaching the NULL pointer dereference, Clang's static analyzer
does not realize that, so we add an `ASSERT()` to inform it of this.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sat, 4 Mar 2023 02:15:28 +0000 (21:15 -0500)]
Suppress Clang Static Analyzer warning about SNPRINTF_BLKPTR()
Clang's static analyzer pointed out that if we can pass a -1 array index
to copyname[copies] if there are no valid DVAs. This is an absurd
situation, but it suggests that we are missing an assertion, so we add
it.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 12 Feb 2023 21:38:27 +0000 (16:38 -0500)]
Suppress Clang Static Analyzer false positive in the AVL tree code.
This has been filed as llvm/llvm-project#60694. Switching from a copy
through a C pointer dereference to an explicit memcpy() is a workaround
that prevents a false positive.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Tue, 7 Feb 2023 23:47:40 +0000 (18:47 -0500)]
Suppress Clang Static Analyzer defect report in abd_get_size()
Clang's static analyzer reports a possible NULL pointer dereference in
abd_get_size() when called from vdev_draid_map_alloc_write() called from
vdev_draid_map_alloc_row() and vdc->vdc_nparity == 0. This should be
impossible, so we add an assertion to silence the defect report.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Richard Yao [Sun, 4 Dec 2022 22:42:43 +0000 (17:42 -0500)]
Fix TOCTOU race in zpool_do_labelclear()
Coverity reported a TOCTOU race in `zpool_do_labelclear()`. This is not
believed to be a real security issue, but fixing it reduces the number
of syscalls we do and will prevent other static analyzers from
complaining about this.
The code is expected to be equivalent. However, under rare
circumstances, such as ELOOP, ENAMETOOLONG, ENOMEM, ENOTDIR and
EOVERFLOW, we will display the error message that we currently display
for the `open()` syscall rather than the one that we currently display
for the `stat()` syscall. This is considered to be an improvement.
Reported-by: Coverity (CID-1524188) Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14575
Alexander Motin [Wed, 8 Mar 2023 19:17:23 +0000 (14:17 -0500)]
More adaptive ARC eviction
Traditionally ARC adaptation was limited to MRU/MFU distribution. But
for years people with metadata-centric workload demanded mechanisms to
also manage data/metadata distribution, that in original ZFS was just
a FIFO. As result ZFS effectively got separate states for data and
metadata, minimum and maximum metadata limits etc, but it all required
manual tuning, was not adaptive and in its heart remained a bad FIFO.
This change removes most of existing eviction logic, rewriting it from
scratch. This makes MRU/MFU adaptation individual for data and meta-
data, same as the distribution between data and metadata themselves.
Since most of required states separation was already done, it only
required to make arcs_size state field specific per data/metadata.
The adaptation logic is still based on previous concept of ghost hits,
just now it balances ARC capacity between 4 states: MRU data, MRU
metadata, MFU data and MFU metadata. To simplify arc_c changes instead
of arc_p measured in bytes, this code uses 3 variable arc_meta, arc_pd
and arc_pm, representing ARC balance between metadata and data, MRU and
MFU for data, and MRU and MFU for metadata respectively as 32-bit fixed
point fractions. Since we care about the math result only when need to
evict, this moves all the logic from arc_adapt() to arc_evict(), that
reduces per-block overhead, since per-block operations are limited to
stats collection, now moved from arc_adapt() to arc_access() and using
cheaper wmsums. This also allows to remove ugly ARC_HDR_DO_ADAPT flag
from many places.
This change also removes number of metadata specific tunables, part of
which were actually not functioning correctly, since not all metadata
are equal and some (like L2ARC headers) are not really evictable.
Instead it introduced single opaque knob zfs_arc_meta_balance, tuning
ARC's reaction on ghost hits, allowing administrator give more or less
preference to metadata without setting strict limits.
Some of old code parts like arc_evict_meta() are just removed, because
since introduction of ABD ARC they really make no sense: only headers
referenced by small number of buffers are not evictable, and they are
really not evictable no matter what this code do. Instead just call
arc_prune_async() if too much metadata appear not evictable.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Allan Jude <allan@klarasystems.com> Signed-off-by: Alexander Motin <mav@FreeBSD.org>
Sponsored by: iXsystems, Inc.
Closes #14359
Attila Fülöp [Wed, 8 Mar 2023 19:12:15 +0000 (20:12 +0100)]
ICP: AES-GCM: Unify gcm_init_ctx() and gmac_init_ctx()
gmac_init_ctx() duplicates most of the code in gcm_int_ctx() while
it just needs to set its own IV length and AAD tag length.
Introduce gcm_init_ctx_impl() which handles the GCM and GMAC
differences while reusing the duplicated code.
While here, fix a flaw where the AVX implementation would accept a
context using a byte swapped key schedule which it could not
handle. Also constify the IV and AAD pointers passed to
gcm_init{,_avx}().
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Rob Norris <robn@despairlabs.com> Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes #14529
Richard Yao [Wed, 8 Mar 2023 00:12:28 +0000 (19:12 -0500)]
Do not hold spa_config in ZIL while blocked on IO
Otherwise, we can get a deadlock that looks like this:
1. fsync() grabs spa_config_enter(zilog->zl_spa, SCL_STATE, lwb,
RW_READER) as part of zil_lwb_write_issue() . It then blocks on the
txg_sync when a flush fails from a drive power cycling.
2. The txg_sync then blocks on the pool suspending due to the loss of
too many disks.
3. zpool clear then blocks on spa_config_enter(spa, SCL_STATE |
SCL_L2ARC | SCL_ZIO, spa, RW_WRITER) because it is a writer.
The disks cannot be brought online due to fsync() holding that lock and
the user gets upset since fsync() is uninterruptibly blocked inside the
kernel.
We need to grab the lock for vdev_lookup_top(), but we do not need to
hold it while there is outstanding IO.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@klarasystems.com> Sponsored-By: Wasabi Technology, Inc.
Closes #14519
Rob N [Tue, 7 Mar 2023 22:05:14 +0000 (09:05 +1100)]
Better handling for future crypto parameters
The intent is that this is like ENOTSUP, but specifically for when
something can't be done because we have no support for the requested
crypto parameters; eg unlocking a dataset or receiving a stream
encrypted with a suite we don't support.
Its not intended to be recoverable without upgrading ZFS itself.
If the request could be made to work by enabling a feature or modifying
some other configuration item, then some other code should be used.
load-key: In the future we might have more crypto suites (ie new values
for the `encryption` property. Right now trying to load a key on such
a future crypto suite will look up suite parameters off the end of the
crypto table, resulting in misbehaviour and/or crashes (or, with debug
enabled, trip the assertion in `zio_crypt_key_unwrap`).
Instead, lets check the value we got from the dataset, and if we can't
handle it, abort early.
recv: When receiving a raw stream encrypted with an unknown crypto
suite, `zfs recv` would report a generic `invalid backup stream`
(EINVAL). While technically correct, its not super helpful, so lets
ship a more specific error code and message.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: Rob Norris <robn@despairlabs.com>
Closes #14577
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: George Amanakis <gamanakis@gmail.com>
Closes #14585
Closes #14592
Andriy Gapon [Tue, 7 Mar 2023 21:48:43 +0000 (23:48 +0200)]
[FreeBSD] fix false assert in cache_vop_rmdir when replaying ZIL
The assert is enabled when DEBUG_VFS_LOCKS kernel option is set.
The exact panic is:
panic: condition seqc_in_modify(_vp->v_seqc) not met
It happens because seqc protocol is not followed for ZIL replay.
But we actually do not need to make any namecache calls at that stage,
because the namecache use is not enabled until after the replay is
completed.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Andriy Gapon <avg@FreeBSD.org>
Closes #14566
Attila Fülöp [Tue, 7 Mar 2023 21:44:11 +0000 (22:44 +0100)]
spl: Add cmn_err_once() to log a message only on the first call
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Brian Atkinson <batkinson@lanl.gov> Signed-off-by: Attila Fülöp <attila@fueloep.org>
Closes #14567
q66 [Tue, 7 Mar 2023 01:07:01 +0000 (02:07 +0100)]
initramfs: fix zpool get argument order
When using the zfs initramfs scripts on my system, I get various
errors at initramfs stage, such as:
cannot open '-o': name must begin with a letter
My zfs binaries are compiled with musl libc, which may be why
this happens. In any case, fix the argument order to make the
zpool binary happy, and to match its --help output.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: Daniel Kolesa <daniel@octaforge.org>
Closes #14572
Andriy Gapon [Tue, 7 Mar 2023 00:30:54 +0000 (02:30 +0200)]
[FreeBSD] zfs_znode_alloc: lock the vnode earlier
This is needed because of a possible error path where zfs_vnode_forget()
is called. That function calls vgone() and vput(), the former requires
the vnode to be exclusively locked and the latter expects it to be
locked.
It should be safe to lock the vnode as early as possible because it is
not yet visible, so there is no interaction with other locks.
While here, remove a tautological assignment to 'vp'.
Reviewed-by: Alexander Motin <mav@FreeBSD.org> Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Signed-off-by: Andriy Gapon <avg@FreeBSD.org>
Closes #14565
Richard Yao [Mon, 6 Mar 2023 23:30:29 +0000 (18:30 -0500)]
Fix memory leak in ztest
This is tripping LeakSanitizer, which causes zloop test failures on pull
requests.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14583
Richard Yao [Sat, 4 Mar 2023 23:42:01 +0000 (18:42 -0500)]
Add missing increment to dsl_deadlist_move_bpobj()
dc5c8006f684b1df3f2d4b6b8c121447d2db0017 was recently merged to prefetch
up to 128 deadlists. Unfortunately, a loop was missing an increment,
such that it will prefetch all deadlists. The performance properties of
that patch probably should be re-evaluated.
This was caught by CodeQL's cpp/constant-comparison check in an
experimental branch where I am testing the security-and-extended
queries. It complained about the `i < 128` part of the loop condition
always evaluating to the same thing. The standard CodeQL configuration
we use missed this because it does not include that check.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #14573
Richard Yao [Sat, 4 Mar 2023 20:53:58 +0000 (15:53 -0500)]
SHA2Init() should use signed assertions when checking an enum
The recent 4c5fec01a48acc184614ab8735e6954961990235 commit caused
Coverity to report that ASSERT3U(algotype, >=, SHA256_MECH_INFO_TYPE);
is always true. That is because the signed algotype and signed
SHA256_MECH_INFO_TYPE values were cast to unsigned types. To fix this,
we switch the assertions to use ASSERT3S(), which retains the signedness
of the original values for the comparison.
Reviewed-by: Tino Reichardt <milky-zfs@mcmilk.de> Reviewed-by: Alexander Motin <mav@FreeBSD.org> Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> Reported-by: Coverity (CID-1535300)
Closes #14573
These are added via HWCAP interface:
- zfs_neon_available() for arm and aarch64
- zfs_sha256_available() for arm and aarch64
- zfs_sha512_available() for aarch64
This one via cpuid() call:
- zfs_shani_available() for x86_64
Tested-by: Rich Ercolani <rincebrain@gmail.com> Tested-by: Sebastian Gottschall <s.gottschall@dd-wrt.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes #13741
These are added:
- zfs_neon_available() for arm and aarch64
- zfs_sha256_available() for arm and aarch64
- zfs_sha512_available() for aarch64
- zfs_shani_available() for x86_64
Tested-by: Rich Ercolani <rincebrain@gmail.com> Tested-by: Sebastian Gottschall <s.gottschall@dd-wrt.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de> Co-Authored-By: Sebastian Gottschall <s.gottschall@dd-wrt.com>
Closes #13741
These are added:
- zfs_neon_available() for arm and aarch64
- zfs_sha256_available() for arm and aarch64
- zfs_sha512_available() for aarch64
- zfs_shani_available() for x86_64
Changes:
- simd_powerpc.h: change license from CDDL to BSD
Tested-by: Rich Ercolani <rincebrain@gmail.com> Tested-by: Sebastian Gottschall <s.gottschall@dd-wrt.com> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes #13741
projects / mirror_zfs.git / log