Coverity caught these. With the exception of the file descriptor leak in
tests/zfs-tests/cmd/draid.c, they are all memory leaks.
Also, there is a piece of dead code in zfs_get_enclosure_sysfs_path().
We delete it as cleanup.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes#13921
The RFC for this finally got published and, therefore,
now has a number. This patch puts this RFC number
in the man page.
This is a content change.
MFC after: 1 week
Coverity complained about unchecked return values and unused values that
turned out to be unused return values.
Different approaches were used to handle the different cases of
unchecked return values:
* cmd/zdb/zdb.c: VERIFY0 was used in one place since the existing code
had no error handling. An error message was printed in another to
match the rest of the code.
* cmd/zed/agents/zfs_retire.c: We dismiss the return value with `(void)`
because the value is expected to be potentially unset.
* cmd/zpool_influxdb/zpool_influxdb.c: We dismiss the return value with
`(void)` because the values are expected to be potentially unset.
* cmd/ztest.c: VERIFY0 was used since we want failures if something goes
wrong in ztest.
* module/zfs/dsl_dir.c: We dismiss the return value with `(void)`
because there is no guarantee that the zap entry will always be there.
For example, old pools imported readonly would not have it and we do
not want to fail here because of that.
* module/zfs/zfs_fm.c: `fnvlist_add_*()` was used since the
allocations sleep and thus can never fail.
* module/zfs/zvol.c: We dismiss the return value with `(void)` because
we do not need it. This matches what is already done in the analogous
`zfs_replay_write2()`.
* tests/zfs-tests/cmd/draid.c: We suppress one return value with
`(void)` since the code handles errors already. The other return value
is handled by switching to `fnvlist_lookup_uint8_array()`.
* tests/zfs-tests/cmd/file/file_fadvise.c: We add error handling.
* tests/zfs-tests/cmd/mmap_sync.c: We add error handling for munmap, but
ignore failures on remove() with (void) since it is expected to be
able to fail.
* tests/zfs-tests/cmd/mmapwrite.c: We add error handling.
As for unused return values, they were all in places where there was
error handling, so logic was added to handle the return values.
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes#13920
strchr returns a pointer to the ',', so if the first option in the list
isn't available, we need to step over the , to look at the next
option. So if kern.cfg.order="acpi,fdt" and we have no acpi, we'd loop
forever with order=',fdt'.
Sponsored by: Netflix
Reviewed by: andrew, jhb
Differential Revision: https://reviews.freebsd.org/D36682
The maximum CPU number of a cpuset_t is determined by CPU_SETSIZE. In
the kernel this is MAXCPU, but in userspace it is CPU_MAXSIZE unless
CPU_SETSIZE is defined before including sys/_cpuset.h. CPU_MAXSIZE is
256 and in userspace MAXCPU is generally 1 because it being set to a
larger MD value is gated on SMP being defined (not generally the case in
userspace).
Reported by: Nathaniel Wesley Filardo <nwfilardo@gmail.com>
Reviewed by: cem, jhb
MFC after: 3 days
Differential Revision: https://reviews.freebsd.org/D36679
A comment says that the caller should free k_out, but the pointer passed
via k_out is not the same pointer we received from strdup(). Instead,
it is a pointer into the region we received from strdup(). The free
function should always be called with the original pointer, so this is
likely a bug.
We solve this by calling `strdup()` a second time and then freeing the
original pointer.
Coverity reported this as a memory leak.
Reviewed-by: Neal Gompa <ngompa@datto.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes#13867
If you force fault a drive that's resilvering, it's scan stats can get
frozen in time, giving the false impression that it's being resilvered.
This commit checks the vdev state to see if the vdev is healthy before
reporting "resilvering" or "repairing" in zpool status.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tony Hutter <hutter2@llnl.gov>
Closes#13927Closes#13930
Traditionally, we've used /boot/msdos for the MBR mount point for the SD
images that we produced. For GPT and bsdinstall, we've used
/boot/efi. Migrate to using /boot/efi for MBR as well and add a
/boot/msdos -> /boot/efi symlink for compatibility (which may disappear
before 14.0, but will remain on the stable branches).
When we first created the arm images, there was no EFI booting and the
FAT partion on an MBR image was used to hold the firmware, uboot.bin,
SoC config files and ubldr. When we transitioned to uboot with EFI, we
put the loader files in the same partition. Later we standardized on
/boot/efi at about the same time we added GPT support to the RE produced
images. We left the MRB case as /boot/msdos for legacy reasons and since
it wasn't always EFI. Later, we dropped support of non-EFI booting on
the RE produced images, so the duality of /boot/msdos diminished even
more. Since so little secondary meaning remains, putting it all in
/boot/efi standardizes the location and reflects the RE images
better as using efi-only booting.
In addition, always label the msdosfs partion 'efi'. While a small
misnomer on some systems that store other files in the ESP, it was
requested in review for more consistency for similar reasons to the
mountpoint rename. There was no way to have an 'alias' or 'second label'
here, so this breaks compatibility. Since the images are self-contained,
this was judged to be an acceptable change.
Sponsored by: Netflix
Reviewed by: manu, allanjude, emaste, gjb
Differential Revision: https://reviews.freebsd.org/D36635
As with the GICv1/2 driver teach the GICv3 driver to translate memory
ranges of children. This allows us to create a common
bus_alloc_resource implementation for bot hACPI and FDT attachments.
Sponsored by: The FreeBSD Foundation
Fixes: eec02418d8 Remove support for FDDI and token ring media types in userland utilities.
Reviewed by: brooks, gjb, imp
Approved by: brooks (src), gjb (mentor, src), imp (src)
Differential Revision: https://reviews.freebsd.org/D36668
MFC after: 3 days
Currently, these two tests pass on disks with 512 byte sectors. In
environments where the backing store is different, the number of
blocks allocated to write the same file may differ. This change
modifies the reported size check to detect an expected change in the
reported number of blocks without specifying a particular number.
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Tony Nguyen <tony.nguyen@delphix.com>
Signed-off-by: John Kennedy <john.kennedy@delphix.com>
Closes #13931
When an internal or other error occurs during the listing of a pool,
return an error code when extiting ippool(8). Printing an error to
stderr without returning an error code is useless in shell scripts.
MFC after: 2 weeks
Add an ippool(8) option to dump a copy of the inm-memory ippool tables
in an ippool(5) format so that it can be reloaded using ippool -f.
MFC after: 2 weeks
This matches the return type of pmap_mapdev/bios.
Reviewed by: kib, markj
Sponsored by: DARPA
Differential Revision: https://reviews.freebsd.org/D36548
Incorrectly sizing the array of hash locks used to protect the
dbuf hash table can lead to contention and reduce performance.
We could unconditionally allocate a larger array for the locks
but it's wasteful, particularly for a low-memory system.
Instead, dynamically allocate the array of locks and scale
it based on total system memory.
Additionally, add a new `dbuf_mutex_cache_shift` module option
which can be used to override the hash lock array size. This is
disabled by default (dbuf_mutex_hash_shift=0) and can only be
set at module load time. The minimum target array size is set
to 8192, this matches the current constant value.
Note that the count of the dbuf hash table and count of the
mutex array were added to the /proc/spl/kstat/zfs/dbufstats
kstat.
Finally, this change removes the _KERNEL conditional checks.
These were not required since for the user space build there
is no difference between the kmem and vmem interfaces.
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes#13928
This reverts commit 34dbc618f5. While this
change resolved the lock contention observed for certain workloads, it
inadventantly reduced the maximum hash inserts/removes per second. This
appears to be due to the slightly higher acquisition cost of a rwlock vs
a mutex.
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Coverity complains about this. It is not a bug as long as we never shift
by more than 31, but it is not terrible to change the constants from 1
to 1ULL as clean up.
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes#13914
Coverity flagged this in its latest run but it is not a problem in
practice as the card's core clock would have to be > 4.2GHz for any
overflow to occur.
CID 1498303: Integer handling issues (OVERFLOW_BEFORE_WIDEN)
Potentially overflowing expression "sc->params.vpd.cclk * 1000U" with type "unsigned int" (32 bits, unsigned) is evaluated using 32-bit arithmetic, and then used in a context that expects an expression of type "uint64_t" (64 bits, unsigned).
Reported by: Coverity Scan (CID 1498303)
Sponsored by: Chelsio Communications
Factor out parts of pcim_iomap_regions_request_all() into
pcim_iomap_regions() now needed for a driver.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky
Differential Revision: https://reviews.freebsd.org/D36654
In addition to the ones added last year add more found in modern
drivers.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky
Differential Revision: https://reviews.freebsd.org/D36656
Constify "*from" arguments and add __ioread32_copy() and
__ioread64_copy() based on the already existing implementations.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky
Differential Revision: https://reviews.freebsd.org/D36657
Add the devres version dmam_alloc_coherent() of dma_alloc_coherent()
along with the ancillary free function.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky
Differential Revision: https://reviews.freebsd.org/D36661
Add some more defines used in drivers to make it easier to compile.
MFC after: 7 days
Reviewed by: hselasky, emaste
Differential Revision: https://reviews.freebsd.org/D36660
Add PCI vendor IDs found in ath and mt76 drivers. This should make it
easier for me and others not having to re-define them locally.
MFC after: 7 days
Reviewed by: hselasky, emaste
Differential Revision: https://reviews.freebsd.org/D36659
Add NOPs for lockdep_{,un}register_key().
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky, emaste
Differential Revision: https://reviews.freebsd.org/D36658
Add #defines for PCI_DEVICE_ID and repoint the PCI_VENDOR_ID one.
Add dev_is_pci().
Add pcie_capability_clear_word() according to similar implementations.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
Reviewed by: hselasky, emaste
Differential Revision: https://reviews.freebsd.org/D36653
Add devm_kmemdup() as needed by a networking driver.
Sponsored by: The FreeBSD Foundation
MFC after: 7 days
eviewed by: hselasky, emaste
Differential Revision: https://reviews.freebsd.org/D36652
Convert most of the cloner customers who require custom params
to the new if_clone KPI.
Reviewed by: kp
Differential Revision: https://reviews.freebsd.org/D36636
MFC after: 2 weeks
While doing the initial SACK retransmission segment while heavily cwnd
constrained, tcp_ouput can erroneously send out the entire sendbuffer
again. This may happen after an retransmission timeout, which resets
snd_nxt to snd_una while the SACK scoreboard is still populated.
Reviewed By: tuexen, #transport
PR: 264257
PR: 263445
PR: 260393
MFC after: 3 days
Sponsored by: NetApp, Inc.
Differential Revision: https://reviews.freebsd.org/D36637
for the default output. For '-a' (per-object needed printout) the
[preloaded] banner is kept.
Instead, use special format2 for printing the preloaded objects (and
vdso), which does not include DT_NEEDED, since there is no object
needing the printed one.
In this way, the output is more compatible with glibc.
Example:
LD_PRELOAD=/lib/libthr.so.3 LD_TRACE_LOADED_OBJECTS=1 /libexec/ld-elf.so.1 /bin/ls
libutil.so.9 => /lib/libutil.so.9 (0x801099000)
libncursesw.so.9 => /lib/libncursesw.so.9 (0x8010b0000)
libc.so.7 => /lib/libc.so.7 (0x801123000)
[vdso] (0x7ffffffff000)
/lib/libthr.so.3 (0x80106c000)
Note the absense of the part before and including '=>' for preloaded
libthr.so.3, and for vdso.
PR: 265750
Reviewed by: jhb
Sponsored by: The FreeBSD Foundation
MFC after: 1 week
Differential revision: https://reviews.freebsd.org/D36616
The current cloning KPI does not provide a way of creating interfaces
with parameres from within kernel. The reason is that those parameters
are passed as an opaque pointer and it is not possible to specify whether
this pointer references kernel-space or user-space.
Instead of just adding a flag, generalise the KPI to simplify the
extension process. Unify current notion of `SIMPLE` and `ADVANCED` users
by leveraging newly-added IFC_C_AUTOUNIT flag to automatically pick
unit number, which is a primary feature of the "SIMPLE" KPI.
Use extendable structures everywhere instead of passing function
pointers or parameters.
Isolate all parts of the oldKPI under `CLONE_COMPAT_13` so it can be safely
merged back to 13. Old KPI will be removed after the merge.
Differential Revision: https://reviews.freebsd.org/D36632
MFC after: 2 weeks
When doing Limited Transmit send an ACK when needed by the protocol
processing (like sending ACKs with a DSACK block).
PR: 264257
PR: 263445
PR: 260393
Reviewed by: rscheff@
MFC after: 3 days
Sponsored by: Netflix, Inc.
Differential Revision: https://reviews.freebsd.org/D36631
The breakage was introduced in OpenZFS commit 48cf170d5.
When a (different) fix solving this issue gets upstreamed
it will replace the current fix in the next merge from OpenZFS.
Simplify epair_clone_create() and epair_clone_destroy() by
factoring out epair softc allocation / desctruction and
interface setup/teardown into separate functions.
Reviewed By: kp, zlei.huang_gmail.com
Differential Revision: https://reviews.freebsd.org/D36614
MFC after: 2 weeks
tcp_respond() crafts a packet and sends it directly to ip[6]output(),
bypassing tcp_output(). Hence it must increment TCP send statistics.
Reviewed by: rscheff, tuexen, rrs (implicitly)
Differential revision: https://reviews.freebsd.org/D36641
- The soisconnected() call on transition from SYN_RCVD to ESTABLISHED
is also necessary for a half-synchronized connection. Fix that
just setting the flag, when we transfer SYN-SENT -> SYN-RECEIVED.
- Provide a comment that explains at what conditions the call to
soisconnected() is necessary.
- Hence mechanically rename the TF_INCQUEUE flag to TF_SONOTCONN.
- Extend the change to the BBR and RACK stacks.
Note: the interaction between the accept_filter(9) and the socket layer
is not fully consistent, yet. For most accept filters this call to
soisconnected() will not move the connection from the incomplete queue
to the complete. The move would happen only when the filter has received
the desired data, and soisconnected() would be called once again from
sorwakeup(). Ideally, we should mark socket as connected only there,
and leave the soisconnected() from SYN_RCVD->ESTABLISHED only for the
simultaneous open case. However, this doesn't yet work.
Reviewed by: rscheff, tuexen, rrs
Differential revision: https://reviews.freebsd.org/D36641
When we're unloading the if_ovpn module we sometimes end up only freeing
the softc after the module is unloaded and the M_OVPN malloc type no
longer exists.
Don't return from ovpn_clone_destroy() until the epoch callbacks have
been called, which ensures that we've freed the softc before we destroy
M_OVPN.
Sponsored by: Rubicon Communications, LLC ("Netgate")
