ufs: do not let a rolled back inode write end a data-only sync
With soft updates, ffs_syncvnode() writes the inode between its passes
while buffers are still dirty. That write can go out with new block
pointers rolled back, for instance a new indirect block whose contents
are only written in a later pass, yet ffs_update() clears IN_SIZEMOD
and IN_IBLKDATA because the write was synchronous. A data-only sync
then decides at the end that the inode need not be written, and
fdatasync(2) or an O_DSYNC write that allocated the first indirect
block returned with the on-disk inode not pointing to it.
Note on entry whether a data-only sync has to write the inode, and base
the final write, done once all dependencies are flushed, on that alone:
nothing else can set IN_SIZEMOD or IN_IBLKDATA meanwhile, as the vnode
is locked. A full sync always writes the inode last and was not
affected.
Sponsored by: Sippy Software, Inc.
Reviewed by: kib
[3 lines not shown]
tests: add UFS synchronous write durability tests
Check that once fsync(2), fdatasync(2), or a write(2) with O_SYNC or
O_DSYNC returns, the new data, the file size and every block pointer
leading to the data are on disk. The cases append a block, fill a hole,
append the first block that needs an indirect block and append below an
existing indirect block. Two more append after another write left a
new direct block unsynced, which with soft updates cuts the size and the
later pointers of an inode written meanwhile. With O_SYNC and O_DSYNC
on soft updates, those also check that the open already made the
unsynced block durable.
Each case runs on UFS without soft updates, with soft updates and with
SU+J.
The file systems live on md(4) and the checks read the inode, the
indirect block and the data block straight from the device with libufs
right after the synchronous call returns. There is no cache in front
of the disk device, so this shows what would survive a crash at that
moment without having to crash anything.
[6 lines not shown]
ufs: make fdatasync(2) and O_DSYNC writes update the inode
ffs_syncvnode() in the DATA_ONLY case and ffs_write() for IO_DATASYNC
decide whether the inode has to be written by testing IN_SIZEMOD and
IN_IBLKDATA against ip->i_flags, the chflags(2) word, instead of the
in-core state ip->i_flag. IN_SIZEMOD and IN_IBLKDATA alias UF_READONLY
and UF_ARCHIVE, so for ordinary files the test was always false: after
an fdatasync() or an O_DSYNC write that extended a file or filled a
hole, the data blocks were on disk but the inode size and block pointers
were not, and a crash before the syncer got to the inode lost the
"synced" data. Files with uarchive or urdonly set got a synchronous
inode write on every call instead.
journal_mount() has the same mixup and sets IN_MODIFIED in i_flags,
which leaves UF_OPAQUE set on the in-core .sujournal inode. The
synchronous ffs_update() that follows writes the inode regardless, so
only the stray flag is fixed here.
Fixes: 7428630b757 ("UFS: write inode block for fdatasync(2) if pointers in inode where allocated")
[9 lines not shown]
ufs: flush soft updates dependencies for O_SYNC and O_DSYNC writes
For a synchronous write, ffs_write() writes the data buffers with
bwrite() and then the inode with ffs_update(vp, 1). With soft updates,
a new block pointer can only reach the disk after the dependencies of
the block, such as the cylinder group map that allocates it, are
written. Until then initiate_write_inodeblock_ufs[12]() and
initiate_write_indirdep() roll the pointer, and for an extending write
the size, back in the buffer being written. Nothing in the write path
flushes those dependencies, so the inode went out with the old size and
pointers, ffs_update() cleared IN_SIZEMOD and IN_IBLKDATA, and a crash
before the syncer rewrote the inode lost data that O_SYNC or O_DSYNC
promised was on stable storage. The same held for a new block below an
indirect block, which ffs_balloc() writes synchronously but with the
pointer rolled back.
When soft updates are enabled and the write allocated blocks, finish it
with ffs_syncvnode(), as fsync(2) and fdatasync(2) do: it flushes the
dependencies, rewrites the rolled back indirect blocks and writes the
[30 lines not shown]
ufs: make fdatasync(2) write dirty indirect blocks
A data-only ffs_syncvnode() skips dirty indirect blocks on file systems
without soft updates, and with soft updates stops making passes once
only indirect blocks are left dirty. Indirect blocks only get dirty
when the block pointers in them change, and those pointers are what
makes a newly allocated block reachable. So when fdatasync() follows
a write that extended the file or filled a hole past the direct blocks,
the data and the inode size reached the disk but the pointer to the new
block did not, and a crash lost the "synced" data. With soft updates
the indirect block was usually written once, with the new pointer
rolled back, and never rewritten.
POSIX requires fdatasync() to write the metadata needed to retrieve the
data, which 52488b51489 already acknowledged for the inode, so treat
dirty indirect blocks the same way for a data-only sync as for a full
one. This restores the loop to what it was before 2f514f92cf0. Plain
overwrites, the case data-only sync is meant to speed up, do not dirty
indirect blocks and still write neither them nor the inode.
[8 lines not shown]
stress2: add a test for forced unmount racing mkdir at UFS_LINK_MAX
On a soft updates file system, keep a directory at UFS_LINK_MAX with
workers removing and recreating subdirectories in it, so that mkdir(2)
keeps going through ufs_sync_nlink(), and force-unmount, check and
remount the file system in a loop. The workers work relative to the
directory: a lookup through the mount point would wait for the unmount
on the covered vnode lock and never race with it. Before the fix, the
first forced unmount deadlocked with mkdir(2) sleeping in vfs_busy().
With dtrace=1, count the vfs_busy() calls made during an unmount by
caller, and the mkdir(2) calls that synced and restarted with ERELOOKUP
during an unmount, which shows that the race was hit.
Sponsored by: Sippy Software, Inc.
Reviewed by: pho
Tested by: pho
Differential Revision: https://reviews.freebsd.org/D60204
MFC after: 1 month
ufs: do not busy or reference the mount in ufs_sync_nlink()
With soft updates, mkdir(2), link(2) and rename(2) into a directory
whose i_nlink is at UFS_LINK_MAX while i_effnlink is below it call
ufs_sync_nlink1() to sync the file system and restart. It busied the
mount with vfs_busy(mp, 0), which sleeps for as long as an unmount is in
progress. The caller is still between vn_start_write() and
vn_finished_write() there, and the unmount, once it has set
MNTK_UNMOUNT, goes on to suspend writes in VFS_UNMOUNT() and waits in
vfs_write_suspend() for that write to end. Neither could make progress,
and every later vfs_busy() of the mount, such as from getfsstat(2) or a
lookup crossing the mount point, hung behind them.
The window is wide: after setting MNTK_UNMOUNT, the unmount waits for
the busy references to drain, which includes another thread already
syncing in ufs_sync_nlink1(), and for vfs_periodic(). A forced unmount
of a file system with a directory at the link limit under mkdir/rmdir
load hits it within seconds.
[19 lines not shown]
ncal(1): handle Sweden's transitional calendar (1700-1712)
Sweden used a unique intermediate calendar during 1700-1712. In 1700,
Sweden omitted the Julian leap day (Feb 29, 1700) to begin a gradual
transition to the Gregorian calendar. The transition was abandoned
mid-way due to the Great Northern War; to revert to the Julian calendar,
an extra day "Feb 30, 1712" was added. Sweden finally adopted the
Gregorian calendar in 1753 (already handled correctly by ncal).
ncal previously displayed Feb 1700 as a 29-day month and Feb 1712 as a
29-day month, both incorrect for the Swedish (-s SE) calendar mode.
Add flag_sweden, which is set when the SE country code is selected (via
-s SE or the locale) and cleared by -J, since -J selects the plain
Julian calendar. When active, sndaysr() and sdater() apply the one-day
offset that characterised the Swedish transitional calendar:
- Feb 1700: displayed with 28 days (no leap day)
- 1700-03-01 to 1712-02-30: dates run one day ahead of Julian, so
[15 lines not shown]
mbuf: simplify m_free
The check for (M_PKTHDR|M_NOFREE) was added by np@ in
"91d1b03d3617 Merge r254736 from user/np/cxl_tuning."
in order to support a clever rx mode optimization used
by cxgbe at the time. That optimization was removed in
46e1e307edb6 several years ago. This check is otherwise
unneeded and adds an extraneous test to every mbuf free,
so let's remove it.
Reviewed by: glebius, np
Differential Revision: https://reviews.freebsd.org/D60686
Sponsored by: Netflix
iflib: unify prefetching
This change makes all prefetching use the iflib prefetch wrappers, rather
than using prefetch() in some places and __builtin_prefetch() in others.
Note that __builtin_prefetch() does not work in the kernel on amd64 as clang
disables it when -mnosse is specified, so we must use the asm directives there,
and builtin_prefetch() elsewhere.
This change enables prefetching on non-x86 (notably arm64), and it enables
prefetching in netmap that has been there since day one, but which has never worked.
This is a no-op on non-netmap amd64 and i386
Differential Revision: https://reviews.freebsd.org/D60602
Reviewed by: glebius, kbowling
Sponsored by: Netflix
rms: Order a writer's stores before its readers see it leave
Readers check rms->writers without a fence, and rms_wunlock() drops
it with a plain store. rms_wlock() interrupts every CPU before the
first writer blocks readers, but nothing orders them when the last
writer leaves: on a weakly ordered CPU a reader can see the count
reach zero and still read what the writer replaced, such as callbacks
of a module unloaded under the lock.
Fence every CPU before the last writer drops the count: rename
rms_wlock_switch() to rms_block_readers() and add
rms_unblock_readers(), a wrapper for cpus_fence_seq_cst(). Readers
stay fence-free; the cost lands on the rare unlock.
Reviewed by: mjg
Discussed with: kib
Fixes: 1f162fef76a0 ("Add read-mostly sleepable locks")
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D60658
arm64: Fix and rename midr_check_var_part_range()
At the lowest and highest variants in the specified range,
midr_check_var_part_range() was comparing the part number, rather than
the revision, against the bounds. In other words, the "Y" in "rXpY"
should be compared to CPU_REV() instead of CPU_PART(). Apply this
correction, and rename the function to midr_check_var_rev_range() to
better reflect the field that we are checking.
Reported by: Claude Code (Opus 5.5)
Reviewed by: andrew
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D60624
powerpc: keep FP, VMX and VSX ownership changes atomic with preemption
trap() and set_mcontext() change the FP/VMX/VSX ownership state in
several steps with preemption enabled. A context switch in between
leaves the thread computing with another thread's register contents:
- enable_fpu()/enable_vec() set PCB_FPU/PCB_VEC before loading the
registers, so a switch mid-load saves a half-loaded unit over the PCB.
- set_mcontext() clears the frame's MSR bits and then the PCB flags;
a switch in between re-enables the unit, and the thread returns to
user space with it enabled but not owned, so it is neither saved nor
restored until the next signal.
Both paths run after every sigreturn(2), setcontext(2) and
swapcontext(3). A POWER9 test that keeps f14-f31 live across a signal,
with other threads using the FPU on the same CPU, saw 283 corrupted
round trips out of 882000; none after this change.
Hold a critical section around both, as amd64 and arm64 do.
[7 lines not shown]
powerpc: keep FP, VMX and VSX ownership changes atomic with preemption
trap() and set_mcontext() change the FP/VMX/VSX ownership state in
several steps with preemption enabled. A context switch in between
leaves the thread computing with another thread's register contents:
- enable_fpu()/enable_vec() set PCB_FPU/PCB_VEC before loading the
registers, so a switch mid-load saves a half-loaded unit over the PCB.
- set_mcontext() clears the frame's MSR bits and then the PCB flags;
a switch in between re-enables the unit, and the thread returns to
user space with it enabled but not owned, so it is neither saved nor
restored until the next signal.
Both paths run after every sigreturn(2), setcontext(2) and
swapcontext(3). A POWER9 test that keeps f14-f31 live across a signal,
with other threads using the FPU on the same CPU, saw 283 corrupted
round trips out of 882000; none after this change.
Hold a critical section around both, as amd64 and arm64 do.
[6 lines not shown]
pf: Change source limiter tables from a task
pf_source_used() puts a source in the table of its source limiter when
the source has more states than the high-water mark, and
pf_source_rele() takes it out again below the low-water mark. Both
changed the table in place: pf_source_used() from pf_create_state(),
with the rules lock held for reading, and pf_source_rele() from
pf_remove_state(), with no rules lock at all. A table may only be
changed with the rules lock held for writing, as packets look addresses
up in its radix tree at the same time. With INVARIANTS the first
insertion panics ("Lock pf rulesets not exclusively locked" in
pfr_route_kentry()); without, the tree can be corrupted.
Queue the changes instead, as pf_overload_task() does for overload
tables, and make them from a task with the rules lock held for writing.
An entry names the limiter by its id and remembers its table: the
limiter, or its table, may be gone or replaced by the time the task
runs, and then the entry is dropped. pfsr_intable changes only once
the change is queued, so a failed allocation is retried the next time
[9 lines not shown]
Fix build failure after 904efcc64398
On main, casting PHYS_TO_DMAP(pa) does not require casting to pt_entry_t
* because it returns void * after fe3e92e6868d ("PHYS_TO_DMAP: Return a
void * instead of a vm_offset_t"). That was not MFCed to stable/14 and
stable/15, thus cast the return value of PHYS_TO_DMAP(pa) to
pt_entry_t * to fix build failure.
Approved by: jhb (mentor)
Fixes: 904efcc64398 ("powerpc/radix: include page tables...")
Sponsored by: FreeBSD Foundation
Fix build failure after f02a5391295e
On main, casting PHYS_TO_DMAP(pa) does not require casting to pt_entry_t
* because it returns void * after fe3e92e6868d ("PHYS_TO_DMAP: Return a
void * instead of a vm_offset_t"). That was not MFCed to stable/14 and
stable/15, thus cast the return value of PHYS_TO_DMAP(pa) to
pt_entry_t * to fix build failure.
Approved by: jhb (mentor)
Fixes: f02a5391295e ("powerpc/radix: include page tables...")
Sponsored by: FreeBSD Foundation
aq: Use the common RSS key without options RSS
Fetch the common RSS key independently of options RSS instead of
generating a private key in non-RSS kernels. Keep the existing
software-RSS or round-robin indirection policy and hardware key
serialization unchanged.
Reviewed by: gallatin
MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D59793
(cherry picked from commit 772994d7748287be70fe850ec9e1117d4e7b1747)
aq: Report partial initialization failures to iflib
Stop initialization when hardware setup, ring initialization/start, or
datapath start fails. Run the existing best-effort stop/cache/reset
cleanup and report the failure through iflib_init_failed(). Do not keep
configuring later rings or publish the interface as running.
Reviewed by: nprice
MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D59853
(cherry picked from commit 050683bb8e136981e4a1f66e845ab05c135630f4)
aq: Invalidate the descriptor cache after stopping all rings
Atlantic controllers can retain receive descriptors and their data
addresses after their rings are disabled. Reusing or releasing those
mappings without invalidating the device cache has caused observed
IOMMU and SMMU faults in the referenced Linux reports (7a1bb49461b1,
ed4d81c4b3f2 and 7526183cfdbe).
Move global cache invalidation out of the per-ring stop routine. Disable
every ring first, toggle invalidation once, and wait for its completion
indication. Exclude Atlantic A0, as in the upstream workaround. Report
a completion timeout rather than silently discarding it.
Reviewed by: nprice
MFC after: 2 weeks
Sponsored by: BBOX.io
Differential Revision: https://reviews.freebsd.org/D59852
(cherry picked from commit 4993c1052f899b9a971815df5661ca241641ad8e)
mgb: Propagate DMA setup errors and use the requested channel
Check the receive DMA and frame transfer setup results that were
previously discarded. Return EINVAL for invalid ring/writeback addresses
rather than reporting success, and stop initialization through
iflib_init_failed() when a ring could not be configured.
Use the transmit queue count for transmit initialization and the supplied
channel index for DMA operations. The driver currently allocates one
queue in each direction, so those index/count corrections are latent.
MFC after: 2 weeks
Sponsored by: BBOX.io
(cherry picked from commit 3f6c1e731376ffaf8612e7e196119acabbf357e4)
mgb: Correct TX interrupt bit
The TX interrupt handler checked MGB_INTR_STS_RX(qidx) instead of
MGB_INTR_STS_TX(qidx) -- presumably a copy-paste issue.
Because the driver does not use TX interrupts so there was no actual
issue in practice.
Reviewed by: adrian
Sponsored by: The FreeBSD Foundation
Fixes: 8890ab7758b8 ("Introduce if_mgb driver for Microchip LAN743x PCIe NIC")
Differential Revision: https://reviews.freebsd.org/D60550
(cherry picked from commit 6c3f2b8be268de075d60387158f2889140851725)
pf: check AH header length.
pf_walk_header() advances the packet offset for each AH extension
header. It was not checked that this does not exceed the packet
length if no next header was processed. Fix it like in IPv6.
OK henning@ sashan@
MFC after: 1 week
Obtained from: OpenBSD, bluhm <bluhm at openbsd.org>, fcd4e5522e
Sponsored by: Rubicon Communications, LLC ("Netgate")
pf: check extension header length.
pf_walk_header6() advances the packet offset for each IPv6 extension
header. It was not checked that this does not exceed the packet
length if no next header was processed. When used with af-to rules,
this could trigger a panic: m_copydata: null mbuf.
from Acts1631; OK henning@ sashan@
MFC after: 1 week
Obtained from: OpenBSD, bluhm <bluhm at openbsd.org>, 6d56112588
Sponsored by: Rubicon Communications, LLC ("Netgate")
pf: do not free the caller's ruleset when attaching a table fails
pfr_attach_table() takes a struct pf_ruleset * from its caller and, on its
own error paths, calls pfr_destroy_ktable() on the table it just made. That
drops the table's reference on the ruleset and calls
pf_remove_if_empty_ruleset(), which may unlink and free the anchor the
ruleset is embedded in -- and then pfr_attach_table() returns NULL to a
caller still holding the pointer.
Reached from userland with DIOCADDRULE and a PF_ADDR_DYNIFTL source, after
DIOCSETLIMIT has lowered PF_LIMIT_TABLES to one item of headroom: the first
pfr_create_ktable() takes it and the root table's fails.
Hold a table reference on the ruleset for the duration instead, so the
ruleset outlives every failure path inside pfr_attach_table() and stays the
caller's to remove.
OK sashan@
[2 lines not shown]
pf: Drain the overload task before a vnet's rules are freed
pf_src_connlimit() queues an entry for pf_overload_task() when a
source exceeds max-src-conn or max-src-conn-rate and the rule has an
overload table. The entry holds a pointer to the rule, and the task
runs from taskqueue_swi with the vnet of the queue. Nothing drained
the task when the vnet went away: an entry queued just before could
be applied to a rule pf_unload_vnet_purge() had freed, or to a vnet
that was gone.
Drain the task at the start of pf_unload_vnet_purge(), before the
unlinked rules are freed. No packet can queue another by then, as the
hooks are already removed.
Reviewed by: kp
MFC after: 2 weeks
Sponsored by: Rubicon Communications, LLC ("Netgate")
Differential Revision: https://reviews.freebsd.org/D60529
