| Age | Commit message (Collapse) | Author |
|---|
|
predicate_parse() adds two to nr_preds to reserve the terminal TRUE
and FALSE entries. It also uses that increased value as the limit for
parsed predicates. If the initial count is smaller than the number
actually parsed, the program can fill the allocation and subsequent
accesses can go past its end.
Keep nr_preds as the predicate limit and use a separate value for the
allocation size. This leaves space for both terminal entries and a
zeroed entry when parsing stops at the limit.
Cc: stable@vger.kernel.org
Fixes: 39f7c41c908b ("tracing/filters: Enable filtering a cpumask field by another cpumask")
Link: https://patch.msgid.link/atELCd1XjKVhrkFL@Taeyangs-MacBook-Pro.local
Signed-off-by: Taeyang Lee <0wn@theori.io>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
|
|
tracing_set_tracer() assigns the flags of the newly selected tracer
with:
tr->current_trace_flags = t->flags ? : t->tracer->flags;
If the new tracer defines neither static flags nor default flags, like
hwlat, irqsoff or wakeup, add_tracer() has no per instance flags copy
for it, and both operands are NULL, so tr->current_trace_flags ends up
being NULL as well.
The ring buffer is not cleared when switching tracers: it is reset
inside tracer_init(), after current_trace_flags was already assigned.
If the buffer still holds function_graph entries of the previous
tracer, printing one of them dereferences the NULL pointer. This was
hit by cat trace during an ftrace stress test:
BUG: kernel NULL pointer dereference, address: 0000000000000000
#PF: supervisor read access in kernel mode
RIP: 0010:print_graph_function_event+0xe/0x20
Call Trace:
s_show+0x4d/0x110
seq_read_iter+0x2d4/0x490
seq_read+0x130/0x170
vfs_read+0xb5/0x370
ksys_read+0x6d/0xe0
do_syscall_64+0xbc/0x450
The snapshot buffer is not reset at all, so this can be reproduced
without a race:
echo function_graph > /sys/kernel/tracing/current_tracer
echo 1 > /sys/kernel/tracing/snapshot
echo hwlat > /sys/kernel/tracing/current_tracer
cat /sys/kernel/tracing/snapshot
Rather than teaching every user of tr->current_trace_flags about this
corner case, make sure it is never NULL: when the newly selected
tracer has no flags at all, tracing_set_tracer() falls back to a
static empty no_tracer_flags. run_tracer_selftest() can hit the same
case, so the same fallback is added there too. The existing users
already handle opts == NULL, so these tracers keep behaving as before.
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261015130658.221198-1-hao.ge@linux.dev
Fixes: e29aa918a928 ("tracing: Have function graph tracer define options per instance")
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik into soc/dt
Ux500 second DTS update for v7.4:
- A AB8505 charger compatible and short interrupt list to go
with changes merged into the power supply subsystem.
* tag 'ux500-dts-for-7.4-2' of git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-nomadik:
ARM: dts: ux500: Use AB8505 charger compatible
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
|
|
tegra_spi_probe() initializes SPI_COMMAND1 with only SPI_M_S, leaving
CS_POL_INACTIVE clear for every chip select. This drives every native
active-low chip select low until tegra_spi_setup() runs for that device.
SPI children are registered and probed one at a time. A synchronous
probe of an earlier child can therefore transfer while a later child's
chip select is still asserted. On a Tegra234 system with active-low
devices on CS0 and CS1, this caused both devices to be selected when the
CS1 device probed first. Its initialization then intermittently failed.
A logic analyzer showed CS0 remained low throughout the CS1 transfer and
went high only after the CS1 probe failed.
Walk the controller's device tree children before SPI_COMMAND1 is
written and set CS_POL_INACTIVE for each valid chip select without
spi-cs-high. That way the line is already deasserted when the first
child transfers and does not change level when setup runs.
Disabled children are walked too. The core only registers available
children, so tegra_spi_setup() never runs for a disabled one and probe
is the only chance to park its chip select at the level its hardware
needs.
If an enabled child shares a chip select with disabled ones, its
polarity takes priority. If only disabled children use a chip select
and they disagree, warn and leave the line inactive high.
Fixes: f333a331adfa ("spi/tegra114: add spi driver")
Assisted-by: Claude:Opus-5
Signed-off-by: Abraham Zukor <abe@maticrobots.com>
Link: https://patch.msgid.link/20261010195503.623287-1-abe@maticrobots.com
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
snd_rk_mc_probe() acquires references to the codec and CPU device nodes
with of_parse_phandle() and stores them in the static rk_dailink, but the
driver never calls of_node_put() on them. All error paths and the success
path simply return, and the driver has no remove callback, so the
references are leaked for the whole lifetime of the card and each failed
or repeated probe leaks another pair. The same pattern was fixed for the
sibling rockchip_rt5645.c driver.
Release the codec and CPU device nodes on every probe error path and in a
new remove callback, and also drop the extra reference placed in args.np
by of_parse_phandle_with_fixed_args().
Fixes: eaae2ea73593 ("ASoC: rockchip: Add machine driver for RK3288 boards that use analog/HDMI")
Assisted-by: DeepSeek-V4.1-Flash
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
Link: https://patch.msgid.link/20261008181459.2762862-1-vulab@iscas.ac.cn
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux into soc/dt
TI Keystone device tree updates for v7.4
Generic Fixups/Cleanups:
* Correct white-space style across the Keystone DTSI files
* Align devctrl child node names with dtschema naming requirements
SoC Specific Fixes:
K2G:
* Add the required #phy-cells property to the usb-nop-xceiv nodes
* Move the usb-phy nodes out of simple-bus, as they have no MMIO address
(dtbs_check fixes)
K2L:
* Rename the deprecated ti,davinci-spi-num-cs property to the standard
num-cs (dtbs_check fix)
* tag 'ti-keystone-dt-for-v7.4' of https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux:
ARM: dts: ti: keystone: k2g: Move usb-phy nodes out of simple-bus
ARM: dts: ti: keystone: k2g: Add required #phy-cells to usb-nop-xceiv nodes
ARM: dts: ti: keystone: Correct white-space style
ARM: dts: ti: keystone: Align devctrl child node names with dtschema
ARM: dts: ti: keystone: k2l: Rename ti,davinci-spi-num-cs to num-cs
|
|
Both fsl_lpspi_dma_transfer() and fsl_lpspi_pio_transfer() only return
negative error values in case of error or 0 in case of success.
Directly return ret, there is no need to check for positive error values
and return them as 0.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Link: https://patch.msgid.link/20261009-spi-fsl-lpspi-more-cleanups-v1-2-2d4549d5205e@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
spi_controller if possible
Instead of passing a spi_controller pointer to functions and using
spi_controller_get_devdata() to get the struct fsl_lpspi_data pointer,
directly pass a struct fsl_lpspi_data pointer.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20261009-spi-fsl-lpspi-more-cleanups-v1-1-2d4549d5205e@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
Since commit faa8e404ad8e ("spi: imx: support dynamic burst length for
ECSPI DMA mode"), the driver allocates its private DMA TX/RX buffers
with kzalloc(dma_len), where dma_len is only rounded up to 4 bytes, but
maps, syncs and unmaps them with DMA_CACHE_ALIGNED_LEN(dma_len), i.e.
rounded up to the cache line size. As a result, the DMA API operates on
memory beyond the end of the allocation.
This was observed on a custom i.MX8MP platform with an mcp251xfd CAN
controller under heavy RPMSG load, as a series of refcount_t warnings
triggered from rpmsg_recv_done() in virtio_rpmsg_bus:
refcount_t: addition on 0; use-after-free.
refcount_t: underflow; use-after-free.
refcount_t: saturated; leaking memory.
Leading finally to an "Unable to handle kernel paging request" oops.
Reproducing this problem took between 30 minutes and roughly 30 hours
under the same workload.
Hardware watchpoints on the refcount of a struct rpmsg_endpoint showed
it was overwritten from 2 to 1 by __pi_memcpy_generic() in the
irq/155-spi2.0 thread. Additionally it was found that a "normal"
rpmsg_recv_done() took 11µs +/-2µs from its kref_get to kref_put. In
the error case the same sequence took about 215µs +/-10µs. Based on
this, an LLM was used to identify the size mismatch fixed here.
Allocate the buffers with DMA_CACHE_ALIGNED_LEN(dma_len) so they cover
the full mapped length, and use the same length for
dma_sync_single_for_cpu() to stay consistent with map and unmap.
With this patch applied, the same workload ran for more than 200 hours
without the problem reproducing.
Assisted-by: LLM
Fixes: faa8e404ad8e ("spi: imx: support dynamic burst length for ECSPI DMA mode")
Cc: stable@vger.kernel.org
Signed-off-by: Richard Leitner <richard.leitner@linux.dev>
Link: https://patch.msgid.link/20261015-fix-spi-imx-dma-v1-1-e28259671392@linux.dev
Signed-off-by: Mark Brown <broonie@kernel.org>
|
|
https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux into soc/dt
TI K3 device tree updates for v7.4
Generic Fixes/Cleanups:
* MMC: Configure the MMC clock pads as OUTPUT across AM62x, AM62A and AM62P
based boards (SK, LP-SK, BeaglePlay, PocketBeagle2, phyCORE, TQMa62xx,
Variscite and Verdin), so the clock pin is driven correctly.
* am62a: Fix the MMC0_CLK pin register address.
* am62*: Add the wakeup R5F nodes to the pre-RAM bootphase, and split the R5F
memory region on AM62A and AM62P.
* am62*: Correct the wkup R5F memory region size on the AM62A, AM62D and AM62P
boards and SoMs that reuse the shared IPC firmware layout.
* k3-pinctrl: Add virtual GPIO select mux macros.
* am62a/am62p/j722s: Fix watchdog comments.
* dt-bindings: Update the audio-refclk binding and the J721E system controller
binding.
SoC Specific features and Fixes:
AM62A:
* Assign on-chip SRAM to the VPU (wave5) node.
AM62P/J722S:
* Add the GPU node.
* Assign on-chip SRAM to the VPU node.
J721S2:
* Add an mmio-sram node to main_navss and assign SRAM to the VPU node.
Board Specific:
AM62x:
* SK: Mark the eMMC as non-removable.
* AM62L EVM: Remove unnecessary internal pull-ups on the MMC0 pins.
* Verdin, Ivy and Zinnia: Fix the main_gpio0/main_gpio1 line-names array sizes
and add the missing SODIMM_84 line name.
AM62A:
* SK: Add an ethernet alias.
AM62P:
* Verdin: Build the Zinnia DTBs.
AM654:
* Base board: Add a power-off delay so the MMC supply is properly power-cycled.
AM67 (J722S):
* Add support for the PHYTEC phyCORE-AM67x SoM, including its bindings.
AM69 (J784S4/J742S2):
* Aquila: Add the Toradex DSI-to-LVDS adapter with the 10.1" V2 display and fix
a typo in the AQUILA_B73 GPIO line name.
* EVM: Set the ACSPCIe refclk to MAIN_PLL2_HSDIV4 for PCIe1.
J7200:
* Common proc board: Add an ethernet alias.
J721S2:
* EVM: Add an audio overlay.
* Common proc board: Add an ethernet alias.
J722S:
* EVM: Switch the audio codec clock from audio_refclk1 to an external clock,
and add an ethernet alias
* tag 'ti-k3-dt-for-v7.4' of https://git.kernel.org/pub/scm/linux/kernel/git/ti/linux: (52 commits)
arm64: dts: ti: k3-am62*: Add wkup R5F nodes to pre-ram bootphase
arm64: dts: ti: k3-am62p: Split r5f memory region
arm64: dts: ti: k3-am62a: Split r5f memory region
arm64: dts: ti: var-som-am62p: Fix wkup R5F memory region size
arm64: dts: ti: k3-am62p5-sk: Fix wkup R5F memory region size
arm64: dts: ti: k3-am62p-verdin: Fix wkup R5F memory region size
arm64: dts: ti: k3-am62a7-sk: Fix wkup R5F memory region size
arm64: dts: ti: k3-am62d2-evm: Fix wkup R5F memory region size
arm64: dts: ti: k3-am62a-phycore-som: Fix wkup R5F memory region size
arm64: dts: ti: Add audio overlay for k3-j721s2-evm
dt-bindings: ti: Update audio-refclk binding and j721e system controller
arm64: dts: ti: k3-am62-verdin-zinnia: Fix main_gpio0/main_gpio1 line names size
arm64: dts: ti: k3-am62-verdin-ivy: Fix main_gpio0/main_gpio1 line names size
arm64: dts: ti: k3-pinctrl: Add virtual GPIO select mux macros
arm64: dts: ti: k3-am62p-j722s-common-main: Assign SRAM to VPU node
arm64: dts: ti: k3-am62a-main: Assign SRAM to VPU node
arm64: dts: ti: k3-j721s2-main: Assign SRAM to VPU node
arm64: dts: ti: k3-j721s2-main: Add mmio-sram node to main_navss
arm64: dts: ti: k3-am62-verdin: fix main_gpio0/main_gpio1 line names size
arm64: dts: ti: k3-am69-aquila: fix AQUILA_B73 gpio-line-names typo
...
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux into soc/dt
Device tree bindings changes for v7.4-rc1
This contains a minor fix for the Tegra132 external memory controller
bindings as well as additions of compatible strings for Tegra264 and
Tegra210-B01 to existing bindings. Also, the new Motorola Artix 4G and
Droid X2 devices are documented.
* tag 'tegra-for-7.4-dt-bindings' of git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux:
dt-bindings: soc: tegra: pmc: Document Tegra210B01
dt-bindings: arm: tegra: Add Motorola Artix 4G and Droid X2
dt-bindings: fuse: Document compatible string for Tegra264 FUSE
dt-bindings: memory: tegra: Fix Tegra132 compatible string
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
|
|
https://github.com/Broadcom/stblinux into soc/dt
This pull request contains Broadcom ARM64-based SoCs Device Tree changes
for 7.4, please pull the following:
- Peter adjust the display DT nodes status properties since it may not
be used by default and therefore that must be done at the board level.
He also adds the ARM PMU, thermal monitor (TMON), USB OTG nodes.
- Gregor adds the 2712D0 stepping compatible string because there is a
need to differientate the D0 stepping to avoid crashes on boot when an
incorrect DTB is selected. He also adds the SPI controller to the RP1
southbridge chip
- Stanimir adds the RP1 PWM node
- Daniel adds the GPU IOMMU and IOMMU cache nodes to the 2712 DTS
* tag 'arm-soc/for-7.4/devicetree-arm64' of https://github.com/Broadcom/stblinux:
arm64: dts: broadcom: bcm2712: Add GPU IOMMU and IOMMU cache nodes
arm64: dts: broadcom: rpi-5: Add RP1 PWM node
arm64: dts: broadcom: bcm2712-rpi-5-b: add spi0 interface
arm64: dts: broadcom: bcm2712: Add USB OTG port
arm64: dts: broadcom: bcm2712: Add thermal monitor
arm64: dts: broadcom: bcm2712: add arm PMU
arm64: dts: broadcom: rp1: add spi controller
arm64: dts: broadcom: bcm2712-d-rpi-5-b: add compatible
dt-bindings: arm: bcm2835: Add 2712-d0 Stepping
arm64: dts: broadcom: bcm2712: adjust display status
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
|
|
Commit 4d4c10f763d7 ("PCI: Explicitly put devices into D0 when
initializing") added pci_pm_power_up_and_verify_state() to pci_pm_init().
The helper ignores pci_power_up()'s return value indicating that BARs need
restoring. Its resume callers restore saved configuration afterwards;
pci_pm_init() does not.
D3hot-to-D0 may reset a device and lose its BARs (PCI PM 1.2, sec 5.4.1).
After enumeration powers it up, pci_enable_device() reads D0 and skips the
full power-up path. Previously, a device still in D3hot at enable time took
that path, including BAR restoration.
On AMD Venice SP7, two AMD xHCI controllers [1022:17d2] are in runtime
D3hot with No_Soft_Reset clear before kdump. Diagnostic reads on
v7.3-rc6-37 show the following for 0000:01:00.4 in the crash kernel:
PMCSR BAR0
before the platform power call 0003 f4100004
after the platform power call 0003 f4100004
after native D0 power-up 0000 00000004
The upper BAR DWORD stays zero. pci_power_up() returns 1, but the BAR
address remains zero while dev->resource[0] still contains 0xf4100000.
Without BAR restoration, the crash kernel hangs during boot. The last
console output includes:
pci 0000:01:00.4: enabling device (0000 -> 0002)
Restoring both controllers' BARs gives matching readbacks and allows kdump
to save a vmcore and reboot. An AMD Turin xHCI [1022:1557] shows the same
BAR loss; restoring its BAR also allows kdump to complete.
Read the initial power state first. Otherwise, pci_power_up() may request a
BAR restore based on a cached PCI_UNKNOWN state, even when the device is
already in D0.
For devices already in D0, only synchronize the platform state so
that ACPI calls such as _REG still run. For other states, use
pci_set_full_power_state() to power up the device and restore BARs
when needed.
Fixes: 4d4c10f763d7 ("PCI: Explicitly put devices into D0 when initializing")
Suggested-by: Mario Limonciello <mario.limonciello@amd.com>
Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Link: https://lore.kernel.org/r/CAJZ5v0ie2AvCbdv8-zkpYzHLcTwUPUj+=WWOQjDAPmSepSAQPg@mail.gmail.com/
Signed-off-by: Keng-Yu Lin <keng-yu.lin@hpe.com>
[bhelgaas: shorten comment]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
Reviewed-by: Desnes Nunes <desnesn@redhat.com>
Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20261015020811.1132314-1-keng-yu.lin@hpe.com
|
|
The devres conversion replaced manual pm_runtime_enable()/disable() with
devm_pm_runtime_set_active_enabled() and dropped the remove-time runtime
PM teardown. The managed release tears runtime PM down in the wrong
order: it calls pm_runtime_dont_use_autosuspend() before
pm_runtime_disable(), i.e. while runtime PM is still enabled, and devres
is LIFO so the devm_clk_get_enabled() release runs afterwards.
At remove(), pm_runtime_put_sync() leaves the device active with the
autosuspend timer armed. Clearing use_autosuspend then makes rpm_idle()
suspend immediately, and xiic_i2c_runtime_suspend() clk_disable()s the
clock. The later devm_clk_get_enabled() release clk_disable_unprepare()s
the already-disabled clock, so clk_core_disable() WARNs ("clkN already
disabled") on every teardown.
Drop the managed helper and restore the non-managed runtime PM setup and
teardown, so runtime PM is enabled once in probe and disabled once in
remove and the clock enable count stays balanced.
Order the remove teardown so runtime PM is disabled before the usage
count is dropped: pm_runtime_disable() followed by pm_runtime_put_noidle()
instead of pm_runtime_put_sync(). Dropping the count first with autosuspend
still enabled arms the autosuspend timer, and a preemption before
pm_runtime_disable() lets it expire, suspend the device and clk_disable()
the clock, which the devm release then disables again - the same WARN.
Restore the non-managed IRQ request as well. The probe error paths now
unwind runtime PM by hand via goto, and a devm-registered handler is only
freed later during devres unwind, so it would stay live across the manual
teardown (and across any future failing step added after it). Request the
IRQ with request_threaded_irq() and free it explicitly in the probe error
path and in remove. The probe error path also clears use_autosuspend, to
match the teardown in remove().
Fixes: 50c63491ff26 ("i2c: xiic: switch to devres managed APIs")
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Cc: <stable@vger.kernel.org> # v7.1+
Signed-off-by: Andi Shyti <andi.shyti@kernel.org>
Link: https://patch.msgid.link/20261014-i2c-xiic-restore-runtime-pm-teardown-v8-1-40668c270069@nexthop.ai
|
|
syzbot reports a lockdep splat when I/O is dispatched to a ublk device
with UBLK_F_SHMEM_ZC enabled:
WARNING: suspicious RCU usage
lib/maple_tree.c:720 suspicious rcu_dereference_check() usage!
...
Call Trace:
mas_root lib/maple_tree.c:720 [inline]
mas_start+0x3cc/0x640 lib/maple_tree.c:1250
mas_state_walk lib/maple_tree.c:3006 [inline]
mas_walk+0x1db/0x490 lib/maple_tree.c:4458
ublk_try_buf_match drivers/block/ublk_drv.c:5639 [inline]
ublk_init_iod+0x5d2/0x1120 drivers/block/ublk_drv.c:521
ublk_dispatch_req+0x97/0x9c0 drivers/block/ublk_drv.c:1776
tctx_task_work_run+0x73d/0xa80 io_uring/tw.c:96
ublk_try_buf_match() calls mas_walk() on the buffer tree without
holding the RCU read lock or the tree lock. Writers are already
excluded because ublk_lock_buf_tree() freezes the queue, so there is
no real race, but maple tree readers must still hold one of the two.
Take the RCU read lock around the walk.
Fixes: 4d4a512a1f87 ("ublk: add PFN-based buffer matching in I/O path")
Reported-by: syzbot+b48868c46e62c1c4cb2c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b48868c46e62c1c4cb2c
Assisted-by: LLM
Signed-off-by: Paolo Bertinetti <paolobertinetti200f4@proton.me>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/179010105967.8478.2344875677216734809@proton.me
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_flush_timeouts() measures the events since the last flush as the
distance of the current sequence from ->cq_last_tm_flush. A fired
timeout bumps ->cq_timeouts before its CQE is posted, so until it is
the sequence can be behind the last flush, and the distance wraps:
last flush: seq 100, cq_last_tm_flush = 100
Y: IORING_OP_TIMEOUT, off 50, target_seq 150
pure timeout A fires, cq_timeouts = 1
task_work posts A's CQE, cached_cq_tail = 101
pure timeout B fires before the flush, cq_timeouts = 2
io_flush_timeouts(): seq = 99, events_got = 0xffffffff
Y, and every sequence timeout behind it, is ended with -ETIME although
no event happened. Two timeouts expiring close together is all it takes,
a completion with IOSQE_CQE_SKIP_SUCCESS after a single one does it too.
Nothing but those in flight CQEs separates the two, so count from the
last flush when the sequence is behind it, as io_timeout() does.
Fixes: f010505b78a4 ("io_uring: flush timeouts that should already have expired")
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_timeout_fn() and io_kill_timeout() bump ->cq_timeouts when a timeout
fires, but its CQE is only posted later from task_work. Until then the
sequence io_timeout() computes is one below the last flush per fired
timeout. Now that ->cq_last_tm_flush is kept while sequence timeouts are
pending, a timeout armed in that window with a small offset gets a target
behind the base. Its distance from the base wraps, and it's never
completed by events, only by its timer. With DEFER_TASKRUN that window
is as wide as the application makes it, as submission runs before the
local task_work.
Nothing but those in flight CQEs separates the two, so use the last
flush as the base for the target when the sequence is behind it.
Fixes: c6c0b3eba88c ("io_uring/timeout: don't skip over reached sequences")
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
If a wakeup sets IO_POLL_RETRY_FLAG between the owning task consuming
the existing instance of IO_POLL_RETRY_FLAG and the loop check
of io_poll_check_events(), then we stall on processing the request
until an unrelated wakeup comes along.
Using atomic_cmpxchg instead of atomic_sub_return to guarantee both the
count v and poll_refs are 0 and no wake-ups are missed.
Cc: stable@vger.kernel.org
Fixes: a26a35e9019f ("io_uring: make poll refs more robust")
Signed-off-by: Juanlu Herrero <juanlu@fastmail.com>
Assisted-by: LLM
Suggested-by: Anand Natarajan <anand@meta.com>
Link: https://patch.msgid.link/20261013-pu-cmpxchg-v1-1-4ddc10fd8a03@fastmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_prep_async_work() decides by the file of a request what kind of work
it is for io-wq. I/O on regular files and block devices is bounded work,
limited to min(SQ entries, 4 * CPUs) workers, for anything else that may
block indefinitely the limit is RLIMIT_NPROC. Writes to a regular file
are hashed by the inode, so that they are run one after the other instead
of having a bunch of workers fight for the inode lock.
Since the file is only assigned when a request is issued, a request that
is queued without an attempt to issue it, REQ_F_FORCE_ASYNC, doesn't have
one at that point. io_prep_async_work() takes that for "not a regular
file or block device":
- Reads and writes end up as unbound work. 32 reads with IOSQE_ASYNC from
a slow block device on a ring with 4 SQ entries get 32 workers, not 4.
- Writes aren't hashed. Four buffered writes to one file get four
workers, three of which do nothing but wait for the lock.
The late assignment is for the benefit of links, where a request may use
a fixed file that one of its predecessors installs. By the time a request
gets to io_queue_iowq() these are done, it is as good as issued. Assign
the file there.
io_assign_file() moves up and is forced inline. With a third caller the
compiler may stop inlining it, and every request that is issued would
pay for a call.
IORING_OP_SPLICE is left as it is, unbound and not hashed. The file of
the request is where it writes to, which says nothing about how long it
takes: a splice from an empty pipe or a socket into a regular file waits
for its input. As bounded work, four of these on a ring with 4 SQ entries
keep all other bounded work of the task from running, and as hashed work
one of them blocks all writes to that file.
REQ_F_FORCE_ASYNC is not only set for IOSQE_ASYNC. What changes, and
brings back what was the case before the file assignment was deferred:
- Requests with IOSQE_ASYNC or IOSQE_IO_DRAIN, and all requests that are
submitted while a drain is active: reads and writes on regular files
and block devices count against the limit for bounded workers, including
the one set with IORING_REGISTER_IOWQ_MAX_WORKERS, so fewer of them run
at the same time.
- Of these, writes to one regular file are run one at a time, unless they
are O_DIRECT and the filesystem has FOP_DIO_PARALLEL_WRITE.
- IORING_OP_FALLOCATE and IORING_OP_FTRUNCATE, which always go to io-wq,
are run one at a time per file, and in turn with these writes.
- The descriptor or the fixed file slot is looked up when the request is
queued, not when a worker gets around to it, so closing or replacing it
in between no longer has an effect on the request.
Fixes: 6bf9c47a3989 ("io_uring: defer file assignment")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-43-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A linked timeout that fires cancels the request it is linked to, and
completes with -ETIME if that worked out:
ret = io_try_cancel(req->tctx, &cd, 0);
...
io_req_set_res(req, ret ?: -ETIME, 0);
io_try_cancel() returns 0 or an error, except for requests that are parked
on ->futex_list or ->waitid_list. io_futex_cancel() and io_waitid_cancel()
return the number of requests that they have cancelled, and
io_try_cancel() hands it on.
So the linked timeout of an IORING_OP_FUTEX_WAIT, IORING_OP_FUTEX_WAITV or
IORING_OP_WAITID that it has cancelled completes with 1, each time.
The waitid test of liburing checks for that 1, and has to be changed to
expect -ETIME, which is what the linked timeout of any other request
completes with.
Fixes: f31ecf671ddc ("io_uring: add IORING_OP_WAITID support")
Fixes: 194bb58c6090 ("io_uring: add support for futex wake and wait")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-42-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
IORING_TIMEOUT_MULTISHOT can't be combined with IORING_TIMEOUT_ABS, prep
rejects that, as the request is armed again with the same value each time
it fires. IORING_TIMEOUT_UPDATE doesn't check and takes an absolute time
for a multishot timeout. The first expiry is as asked for, but
io_timeout_complete() then arms the timer with ->mode of the original
request, i.e. it uses the absolute time as the interval. With
CLOCK_MONOTONIC the next expiry is an uptime away, with CLOCK_REALTIME
more than fifty years.
Fail such an update with -EINVAL, and do so before the timeout is
touched.
Fixes: ea97f6c8558e ("io_uring: add support for multishot timeouts")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-41-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Commit 41b70df5b38b ("io_uring/net: commit partial buffers on retry")
commits ring provided buffers when a receive is going to be retried after
a partial transfer, so that another request using the same buffer group
can't be handed the memory the first one keeps receiving into. It commits
the number of bytes received so far. That does the job for a single
buffer which is consumed as a whole, but the request hangs on to all it
has mapped:
- For a buffer ring with IOU_PBUF_RING_INC only these bytes are consumed.
The rest of the buffer stays at the head of the ring.
- For a bundle only the buffers which hold these bytes are consumed, the
others that were peeked stay in the ring.
In both cases the next receive with that buffer group gets memory that
the retried request is still going to write to, and whoever comes last
overwrites the data of the other. What the retry fills isn't committed at
all, as the buffer list isn't known any more at that point. So the kernel
hands it out again later on, while the CQE tells the application that it
was used, and for an incremental buffer that it is done with.
The remainder can't be committed in advance: if the request ends up
receiving less, there is no way to tell the application that more was
consumed than the CQE accounts for. Complete the request with what it has
got instead of retrying it. MSG_WAITALL is no longer honoured for
receives into incremental buffers or multi buffer bundles, they can
return a short count, with the request marked as failed like for other
short MSG_WAITALL transfers.
Fixes: 41b70df5b38b ("io_uring/net: commit partial buffers on retry")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-40-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
-ERESTARTSYS and friends are instructions for the system call exit code,
which either restarts the call or turns them into -EINTR. They are not
supposed to be seen by applications. A request isn't a system call,
nobody looks at what it returns before that is put into a CQE.
The read/write and the networking opcodes convert these codes to -EINTR
by hand. The others don't. An io-wq worker that is blocked in the open of
a FIFO, a tty or some other device, or in a splice from an empty pipe,
and that gets interrupted because the request is cancelled, completes it
with -512:
IORING_OP_OPENAT, IOSQE_ASYNC // FIFO without a writer
IORING_OP_ASYNC_CANCEL // -EALREADY, worker is signalled
CQE of the open: res == -512
There are more candidates, like whatever a file system or driver returns
from ->flush(), ->fsync(), ->fallocate() or ->uring_cmd(), or breaking a
lease or delegation.
Take care of it in io_issue_sqe(), which all of them go through. The
helper is moved from rw.c as is. The cost is a compare and a branch that
is not taken per request.
Fixes: 15b71abe7b52 ("io_uring: add support for IORING_OP_OPENAT")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-39-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_pipe_fixed() installs both ends of the pipe in the fixed file table,
drops ->uring_lock, and copies the slot numbers to the application. If
that faults, it takes the lock again and removes whatever is in these
slots.
If the request is run by io-wq the lock really is dropped in between, and
the slots are visible to everybody else. By the time the error path gets
to them, they may have been closed and reused, by IORING_OP_CLOSE and a
direct open or accept, or by IORING_REGISTER_FILES_UPDATE. Then a file is
removed from the table that has nothing to do with the failed request.
Do the copy with the lock held. Faulting in user memory under
->uring_lock is fine, the registration paths do it all the time.
Fixes: 53db8a71ecb4 ("io_uring: add support for IORING_OP_PIPE")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-38-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
__io_sq_thread() doesn't submit anything as long as the ring has
IORING_SETUP_R_DISABLED set. The check that keeps the SQPOLL thread from
going to sleep while the SQ ring holds entries doesn't know about that.
If an application fills in SQEs first and enables the ring later, the
thread finds that there is work each time it is about to sleep, goes
around, doesn't submit it, and so on. It uses up a CPU until the ring
gets enabled.
Entries in the SQ ring of a disabled ring are no reason to stay awake.
io_register_enable_rings() clears the flag and then wakes the thread, and
the flag is looked at after prepare_to_wait(), so the thread can't miss
that.
Fixes: 0298ef969a11 ("io_uring: clean R_DISABLED startup mess")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-37-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_ring_buffer_select() flags the request with REQ_F_BL_EMPTY if it takes
the last buffer of the ring. Head and tail are 16-bit counters, but
"head + 1" is computed as an int. If head is 65535 and tail has wrapped
to 0, that compares 65536 to 0 and the flag isn't set.
A bundle receive relies on the flag to tell that there is no point in
trying to get more data. Without it, it goes for another round, finds no
buffer, and the request, which has received data, is marked as failed on
account of the -ENOBUFS. That severs a link, and ends a multishot.
Fixes: c3f9109dbc9e ("io_uring/kbuf: flag request if buffer pool is empty after buffer pick")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-36-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_zcrx_queue_cqe() computes the offset of the buffer in its area from
net_iov_idx(), which is an unsigned int, so the shift is done in 32 bits
and only the result is widened to the u64 that goes into the CQE. An area
can be up to 1TB in size. For every niov beyond the first 4GB the CQE
points the application at a buffer 4GB, or a multiple of that, below the
one which has the data. It reads the wrong data, and then returns a
buffer it doesn't own via the refill queue while the real one is never
given back.
Fixes: 11ed914bbf94 ("io_uring/zcrx: add io_recvzc request")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-35-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
__io_napi_busy_loop() caps the busy poll time to what is left until the
timeout of the wait. The difference is negative if the timeout has passed
already, which is what an application gets if it uses
IORING_ENTER_ABS_TIMER with a time that isn't in the future any more. The
min_t(u64, ...) turns that into a huge value, so the cap has no effect
and the wait, which should return right away, spins for the full busy
poll time first.
Fixes: d5cce407e4f5 ("io_uring/napi: postpone napi timeout adjustment")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-34-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
With dynamic tracking a NAPI id that hasn't been seen for a minute is
meant to be dropped from the list the ring busy polls. The timeout is
added to jiffies, but it is defined as 60 * SEC_CONVERSION, and
SEC_CONVERSION isn't the number of jiffies in a second. It is the
internal factor of __timespec64_to_jiffies(), HZ scaled up by
SEC_JIFFIE_SC bits. For HZ=1000 that is 1000 << 22, which puts the
timeout at about eight years. On 32-bit the multiplication overflows.
So entries are never removed, and every id that a socket of the ring
ever had keeps being polled on each wait.
Fixes: 8d0c12a80cde ("io-uring: add napi busy poll support")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-33-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
When the min wait time is over and there is work to run, which for
DEFER_TASKRUN includes task_work that was queued lazily and didn't wake
the waiter, io_cqring_min_timer_wakeup() ends the wait via
io_cqring_timer_wakeup(). That flags it as timed out. io_cqring_wait()
runs the work, leaves the loop, and returns 0 if the CQ ring holds
something.
Running task_work doesn't have to result in a CQE, though. A poll wake up
can turn out to be spurious, for instance if somebody else has read the
data by the time the request is retried. The application then gets -ETIME
once the min wait time has passed, no matter how far away the timeout it
asked for is.
Wake the task without declaring a timeout. From now on a single CQE
satisfies the wait, like in the case where the timer is switched over to
the normal timeout. If running the work doesn't post one, the loop in
io_cqring_wait() goes back to sleep with the normal timeout.
Fixes: 1100c4a2656d ("io_uring: add support for batch wait timeout")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-32-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_parse_user_time() converts an absolute time from the time namespace of
the caller to the host's view, for the clock given by the flags. The
flags of IORING_TIMEOUT_UPDATE and IORING_LINK_TIMEOUT_UPDATE can't name
a clock, the updated timer keeps running on the clock the timeout was
set up with. So the time is always converted as if it was
CLOCK_MONOTONIC.
In a time namespace with a monotonic offset, that moves the new expiry of
a CLOCK_REALTIME timeout by that offset, although the clock isn't
affected by time namespaces at all, and the one of a CLOCK_BOOTTIME
timeout by the difference between the two offsets.
Do the conversion when the timeout has been looked up and its clock is
known. The update is run by the submitter, its SQPOLL thread or one of
its io-wq workers, all of which share the time namespace.
Fixes: 9cc6bac1bebf ("io_uring/timeout: honour caller's time namespace for IORING_TIMEOUT_ABS")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-31-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
IORING_TIMEOUT_UPDATE clears ->off of the timeout it updates, to turn a
timeout that waits for a number of completions into a pure one. For a
multishot timeout ->off isn't a sequence though, it tells whether the
number of repeats is limited: io_timeout_finish() only looks at
->repeats if ->off is set.
A multishot timeout that was set up to fire N times hence fires forever
once its interval has been updated. Leave ->off alone for those, they are
never treated as sequence timeouts anyway.
Fixes: ea97f6c8558e ("io_uring: add support for multishot timeouts")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-30-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_flush_timeouts() copes with the wrap of the CQ sequence by looking at
the distance from ->cq_last_tm_flush. That relies on the target of each
pending timeout being at or ahead of the last flush.
io_timeout() breaks that. It sets ->cq_last_tm_flush to the current
sequence, on the assumption that nothing has been posted since the last
flush. That isn't true. A multishot request posts its CQEs right when it
is issued or retried, but they are committed, and the timeouts flushed,
at the end of the submission or task_work run:
X: IORING_OP_TIMEOUT, off == 1 target_seq == N + 1
io_submit_sqes()
multishot recv, posts two CQEs sequence == N + 2
Y: IORING_OP_TIMEOUT, off == 100 ->cq_last_tm_flush = N + 2
io_flush_timeouts()
X: events_needed = N + 1 - (N + 2) // 0xffffffff
events_got = 0 // not yet
X isn't completed although the completions it was waiting for have
happened, only its timer can end it now. And as the list is sorted and
the walk ends at the first timeout that isn't due, it gets in the way of
the ones behind it. The insertion sort has the same problem, it measures
the distance of the pending targets from the current sequence.
->cq_last_tm_flush only needs to be brought up to date if no timeout is
waiting for a sequence, as io_flush_timeouts() isn't called before the
first one is set up. Otherwise it is current enough. Timeouts with a
sequence come first in the list, so looking at the first entry will do.
Use the last flush as the base for the insertion sort as well.
Fixes: f010505b78a4 ("io_uring: flush timeouts that should already have expired")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-29-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Since the CQ ring dump was changed to deal with IORING_SETUP_CQE_MIXED, a
32b CQE is taken to occupy two entries of the ring. That is how it works
for mixed rings. With IORING_SETUP_CQE32 each entry has twice the size:
head and tail count 32b CQEs, and entry n is found at cqes[n << 1].
For such rings the dump reads cqes[n], and skips every other entry. It
shows half of the pending CQEs at best, and what it shows is taken from
the wrong place unless the head happens to be at the start of the ring.
The extra data of the last entry isn't shown at all, as the check that
keeps the dump of a mixed ring from reading beyond the array is applied to
these rings as well.
Fixes: 82ceb7fcc5ff ("io_uring/fdinfo: handle mixed sized CQEs")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-28-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
On a ring with IORING_SETUP_SQE_MIXED a 128b SQE takes two entries that
have to be contiguous in memory, io_init_req() fails it with -EINVAL if
it starts on the last entry of the SQ ring. The entry a pending SQE ends
up in after IORING_REGISTER_RESIZE_RINGS depends on the size of the new
ring, and nothing takes care of that. With four entries instead of eight,
a 128b SQE which was fine in entries 3 and 4 is split up between entry 3
and entry 0. The resize succeeds, and the request fails later on.
If the ring has an SQ array, the second half is also taken from the
wrong place unless the array happens to map the following index to the
following entry. The submission side doesn't look at the array for the
second half.
Do what is done for 32b CQEs on IORING_SETUP_CQE_MIXED rings: fail the
resize with -EOVERFLOW if a 128b SQE would wrap. And copy the second half
from where it is.
Fixes: 1cba30bf9fdd ("io_uring: add support for IORING_SETUP_SQE_MIXED")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-27-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The ring heads and tails are free running 32-bit counters.
io_register_resize_rings() takes that into account when it checks whether
the pending entries fit into the new rings, but then copies them with
for (i = old_head; i < tail; i++)
If the tail has wrapped and the head hasn't, that copies nothing. The new
rings get the old head and tail, so the application finds whatever is in
the freshly allocated memory, zeroes, in place of the CQEs it hasn't
reaped yet, or the kernel in place of the SQEs that have not been
submitted yet. A ring that completes a million requests per second gets
there after little more than an hour.
Loop until the tail is reached, the number of iterations has been bounded
by the size of the new ring at that point.
Fixes: 79cfe9e59c2a ("io_uring/register: add IORING_REGISTER_RESIZE_RINGS")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-26-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
On an IORING_SETUP_CQE_MIXED ring, a 32b CQE uses two contiguous CQ
entries and is never allowed to wrap the end of the ring. When posting,
io_cqe_cache_refill() inserts a dummy IORING_CQE_F_SKIP entry if only
one entry is left before the ring wraps, and the application relies on
that to read a 32b CQE as one contiguous 32 byte slot.
io_register_resize_rings() copies pending CQEs to the new ring one 16b
entry at a time, placing entry i at i & (new_entries - 1). When the CQ
ring is shrunk, a pending 32b CQE that was contiguous in the old ring
can land with its first half on the last entry of the new ring and its
second half on entry 0. For example, with 8 old entries, 4 new entries
and a 32b CQE at index 3, the halves end up at entries 3 and 0. The
application then reads the extra data of that CQE from past the end of
the CQ array, and the CQE content it sees is wrong.
This was found by code inspection and has not been reproduced.
Fix by failing the resize with -EOVERFLOW if a pending 32b CQE would
wrap the new ring, the same way the resize fails when the pending CQEs
don't fit.
Fixes: e26dca67fde1 ("io_uring: add support for IORING_SETUP_CQE_MIXED")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-25-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
If an SQPOLL thread serves more than one ring, __io_sq_thread() submits
no more than IORING_SQPOLL_CAP_ENTRIES_VALUE entries per ring and round,
for fairness. On a ring with IORING_SETUP_SQE_MIXED the cap may fall
between the two entries of a 128b SQE. io_init_req() finds that just one
entry is left of what it was asked to submit and fails the request with
-EINVAL, although the second entry is sitting in the SQ ring.
In the next round the second entry is taken for an SQE of its own, so on
top of the failed request the application gets whatever its payload
amounts to when it is read as an SQE.
The application has done nothing wrong, and it can't know where the cap
is going to be applied. For SQPOLL, let the request have its second entry
if the SQ ring holds it, in excess of the number of entries that were to
be submitted.
Fixes: 1cba30bf9fdd ("io_uring: add support for IORING_SETUP_SQE_MIXED")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-24-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
->cqe_cached and ->cqe_sentinel delimit the CQ entries which can be
handed out without looking at the ring. io_get_cqe_overflow() only calls
io_cqe_cache_refill() if that range is too small. Without
IORING_SETUP_CQE_MIXED too small means empty. With it, a 32b CQE needs
two entries, and one may be left. If the refill then fails, the entry
stays in the cache, and the next 16b CQE gets it without any checks.
That CQE overtakes the 32b one, which went to the overflow list. The test
for IO_CHECK_CQ_OVERFLOW_BIT, which is there to prevent that, is part of
the refill:
CQ ring with 4 entries, head == 0
16b CQE, 32b CQE tail == 3, one entry cached
32b CQE no room, the CQE overflows
16b CQE written to entry 3, ahead of the 32b CQE
Forget about the rest of the cache when it is refilled.
Fixes: e26dca67fde1 ("io_uring: add support for IORING_SETUP_CQE_MIXED")
Reported-by: Leonid Ovchinnikov <teiny.work@gmail.com>
Closes: https://lore.kernel.org/io-uring/CAECZYD5OMD9vMTzjteFMMP40Dn+SWKj9SFj=43U6uO3XMnxJbw@mail.gmail.com/
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-23-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The futex code relies on there being no private futex waiters by the time
the first private hash of a mm is installed: __futex_pivot_hash() moves
the waiters from the previous private hash to the new one, but it does not
look for them in the global hash which is used as long as there is no
private hash. That is why every clone(CLONE_VM) allocates it up front: a
task which is alone in its mm cannot block in futex_wait() and create a
thread or invoke prctl(PR_FUTEX_HASH) at the same time.
IORING_OP_FUTEX_WAIT and IORING_OP_FUTEX_WAITV do not block the submitter
though:
io_uring_enter()
io_futex_wait()
futex_wait_setup() // no private hash: queued in the global hash
pthread_create()
futex_hash_allocate_default()
__futex_pivot_hash() // first private hash, nothing is moved
futex(FUTEX_WAKE_PRIVATE)
futex_hash() // private hash: no waiter
The wakeup is lost and the request only completes when it is cancelled.
prctl(PR_FUTEX_HASH, PR_FUTEX_HASH_SET_SLOTS, n) instead of
pthread_create() has the same effect.
Allocate the default private hash when a wait request on a private futex
is prepared, which happens in the context of the submitter. This is a RCU
protected pointer check once the hash exists. A failure to allocate it
fails the request with -ENOMEM.
Fixes: 80367ad01d93 ("futex: Add basic infrastructure for local task local hash")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-22-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A file that uses two wait queues for poll, e.g. a tty, gets a second
io_poll entry, which IORING_OP_POLL_ADD keeps in ->async_data until the
request is freed. io_poll_add() can run more than once for a request
though: IORING_OP_POLL_REMOVE with one of the update flags disarms and
rearms it, and io_poll_task_func() reissues it if a wake up turns out to
be spurious. The second poll_wait() of the new round then finds
*poll_ptr set, with ->head cleared by the disarm, and __io_queue_proc()
fails what it takes for a third wait queue with -EINVAL.
For an update that means that the poll request is completed with
-ECANCELED, while the update itself reports success. For a spurious wake
up, e.g. tcsetattr() on the polled tty, the poll request fails with
-EINVAL.
Free the old entry and clear the poll flags before arming, which is what
io_arm_apoll() does for internal polling. All entries have been removed
from their wait queues at that point.
Fixes: b69de288e913 ("io_uring: allow events and user_data update of running poll requests")
Fixes: 6e5aedb9324a ("io_uring/poll: attempt request issue after racy poll wakeup")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-21-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_wake_function() returns -1 if the waiter it is called for doesn't have
enough events yet, which makes __wake_up_common() stop walking the wait
queue. That assumes that the waiters behind it can't be satisfied either.
Each waiter has its own target though, computed from the min_complete it
passed in:
thread A: io_uring_enter(GETEVENTS, min_complete = 10) // queued first
thread B: io_uring_enter(GETEVENTS, min_complete = 1)
one CQE is posted: A isn't woken, and B isn't even considered
B keeps sleeping with the event it asked for sitting in the CQ ring, until
A's demand is met or a timeout or signal ends the wait.
Return 0 to carry on with the next waiter. Only waiters whose condition
is met are woken, like before.
Fixes: bda521624e75 ("io_uring: make CQ ring wakeups be more efficient")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-20-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
With IORING_SETUP_DEFER_TASKRUN completions are queued as local work, and
only become CQEs when the submitter enters the kernel to wait for events.
For an application that finds out via poll() or epoll on the ring fd when
it is time to do that, io_uring_poll() reports EPOLLIN if local work is
pending.
That only helps if the work is there at the time of the call. When
DEFER_TASKRUN was added, io_req_local_work_add() did io_cqring_wake(),
which took care of tasks sleeping in poll() as well. Then polling got a
wait queue of its own, ->poll_wq, and the wake up in
io_req_local_work_add() was replaced by waking the submitter directly.
Since then ->poll_wq is only woken when CQEs are committed, which for
such a ring is done by the very task that is waiting to be told to:
submitter completion
poll(ring_fd)
io_uring_poll() // nothing yet
schedule()
io_req_local_work_add()
// ->cq_wait_nr: nobody waits
// sleeps on
Wake ->poll_wq when the first entry is put on the work list. Like for the
eventfd, that must not be done right away if the work is queued from a
wake up handler, as the chain of wake ups could get back to the wait
queue whose lock is held. Use an RCU callback in that case, with a
reference to the ring.
Fixes: 130bd686d9be ("io_uring: waitqueue-less cq waiting")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-19-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A cancellation with IORING_ASYNC_CANCEL_ALL or _ANY goes on until it finds
nothing more. A request may stay where it was found for a while after it
has been cancelled, so each one that matches is stamped with the sequence
number of the cancellation, and is skipped from then on.
io_cancel_cb() does so for io-wq work as well. There is no need to:
pending work is taken off the list, and the workers are looked at once per
call. And it does harm. A request that is run by io-wq may be parked
while it still is the current work of the worker: poll gets armed for it,
or it is put on ->futex_list or ->waitid_list.
io-wq worker io_try_cancel()
io_wq_submit_work()
io_arm_poll_handler()
io_poll_add_hash()
io_async_cancel_one()
io_cancel_cb()
stamps the request
sets IO_WQ_WORK_CANCEL
= -EALREADY
io_poll_cancel()
skips the request
= -ENOENT
io_assign_current_work(NULL)
The flag comes too late for the worker to see it, and the search that
would have found the request takes it for done. The cancellation returns,
and the request stays.
With a multishot poll, IOSQE_ASYNC, and IORING_OP_ASYNC_CANCEL with
IORING_ASYNC_CANCEL_ALL up to 63us later, that happens to 4 of 1,000
requests (4 CPUs, KVM).
Take the stamp off again if the work matches. If the request is found
on one of the lists it gets a new one, and io_cancel_cb() doesn't match it
a second time.
__io_async_cancel() tries the io-wq of the caller twice. So far the stamp
has kept it from counting work that is running there the second time.
Skip it instead, so that the number that is returned stays the same.
Fixes: 8e29da69fead ("io_uring: add support for IORING_ASYNC_CANCEL_ALL")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-18-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
IORING_REGISTER_SYNC_CANCEL waits without a limit if both fields of
io_uring_sync_cancel_reg::timeout are -1. io_sync_cancel() compares them
with -1UL. They are of type long long. With a long of 32 bits -1UL is
4294967295, which is what it stays when it is converted for the
comparison, and the fields are never equal to it.
So { -1, -1 } is taken for a timeout. ktime_set() takes the nanoseconds as
an unsigned long, so what becomes of it is -1s + 4294967295ns = 3.29s.
If the request is being run by io-wq and can't be interrupted, the call
returns -ETIME after that time, rather than waiting for the request to be
gone. On i386, with a buffered read from a null_blk device that takes 4s
to complete a request, IOSQE_ASYNC, that is what each call does.
Fixes: 78a861b94959 ("io_uring: add sync cancelation API through io_uring_register()")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-17-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
If the request to be cancelled is being run by io-wq,
io_async_cancel_one() flags the work, signals the worker and returns
-EALREADY. Since commit ccbf726171b7 ("io_uring: perform poll removal even
if async work removal is successful") io_try_cancel() goes on regardless,
as poll may be armed for the request. If it isn't, and nothing else turns
up, the result is the -ENOENT of the last search.
__io_async_cancel() makes up for that. On -ENOENT it tries the io-wq of
every task that uses the ring, and that of the caller is one of them. So
the work is found a second time, and the result is -EALREADY after all.
Unless the worker is done with it by then. If the work is flagged before
io_wq_submit_work() looks at IO_WQ_WORK_CANCEL, all the worker does is to
queue the request to be failed by task work. Then it isn't anywhere to be
found until the task work has run, and
- IORING_REGISTER_SYNC_CANCEL returns -ENOENT,
- IORING_OP_ASYNC_CANCEL completes with -ENOENT,
for a request that they have just cancelled, and that completes with
-ECANCELED.
With a read of an empty pipe, IOSQE_ASYNC, and a sync cancel right behind
it, 1 to 3 of 1,000,000 calls return -ENOENT, and 50 to 110 with
IORING_SETUP_DEFER_TASKRUN (4 CPUs, KVM). Nothing is written to the pipe,
and all of these reads end with -ECANCELED.
Remember that the work was found. IORING_OP_ASYNC_CANCEL completes with
-EALREADY, as it does if the worker is a bit slower. For
IORING_REGISTER_SYNC_CANCEL it means to try again, which ends with 0.
The result of a linked timeout, which calls io_try_cancel() by itself,
and the number that is returned for IORING_ASYNC_CANCEL_ALL and _ANY don't
change.
Fixes: ccbf726171b7 ("io_uring: perform poll removal even if async work removal is successful")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-16-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
IORING_OP_FUTEX_WAIT, IORING_OP_FUTEX_WAITV and IORING_OP_WAITID don't
block. They put the request on a list of the ring, where a wake up or a
cancellation finds it, and return IOU_ISSUE_SKIP_COMPLETE. They do so as
well if they are run by io-wq, e.g. because of IOSQE_ASYNC.
io_wq_submit_work() looks at IO_WQ_WORK_CANCEL once, before it issues the
request. Somebody who wants to cancel the request after that, and before
it is on the list, finds it running:
io-wq worker io_try_cancel()
io_wq_submit_work()
IO_WQ_WORK_CANCEL isn't set
io_async_cancel_one()
sets IO_WQ_WORK_CANCEL
signals the worker
= -EALREADY
...
io_futex_cancel() = -ENOENT
...
io_futex_wait()
adds the request to ->futex_list
= IOU_ISSUE_SKIP_COMPLETE
Nobody looks at the flag again. IORING_OP_ASYNC_CANCEL returns -EALREADY
and the request stays.
It is the problem that "io_uring/poll: don't lose a cancel as io-wq arms
poll" deals with for poll.
Look at the flag once the request is on the list, and cancel it if it is
set. The lists are searched and the requests are added with ->uring_lock
held, and the flag is set before the search. So either the search finds
the request, or it came first and the flag is seen.
What the canceller is told doesn't change, see the other patch, also for
IORING_ASYNC_CANCEL_ALL and _ANY.
Fixes: 194bb58c6090 ("io_uring: add support for futex wake and wait")
Fixes: 8f350194d5cf ("io_uring: add support for vectored futex waits")
Fixes: f31ecf671ddc ("io_uring: add IORING_OP_WAITID support")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-15-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A request on a pollable file that was forced to io-wq is issued without
blocking there. If that ends in -EAGAIN, io_wq_submit_work() arms poll
for it and returns. It looks at IO_WQ_WORK_CANCEL once, before it issues
the request.
Somebody who wants to cancel the request in between finds it running:
io-wq worker io_try_cancel()
io_wq_submit_work()
IO_WQ_WORK_CANCEL isn't set
io_issue_sqe() = -EAGAIN
io_async_cancel_one()
sets IO_WQ_WORK_CANCEL
signals the worker
= -EALREADY
io_poll_cancel() = -ENOENT
...
= -ENOENT
io_arm_poll_handler() = IO_APOLL_OK
return
Nobody looks at the flag again, and the signal doesn't matter to a
worker that doesn't sleep. The request is armed as if nothing had
happened:
- a linked timeout completes with -ENOENT, and the request it was
meant to limit waits for as long as it takes.
- IORING_OP_ASYNC_CANCEL returns -EALREADY, and the request stays.
- IORING_REGISTER_SYNC_CANCEL waits for the request to complete if it
gets -EALREADY. Since commit 5623eb1ed035 ("io_uring/tctx: add
separate lock for list of tctx's in ctx") it doesn't sleep but spins,
so its next attempt finds the armed poll.
The same goes for a multishot poll, recv or accept that is armed from
io-wq.
With a read of an empty pipe, IOSQE_ASYNC, and a linked timeout of 5ms
or a cancel in the same submission, in a guest that is slow enough (TCG,
KASAN), the request is lost in 21 of 24 rounds of 40 or 100 attempts.
While it arms poll from io-wq, __io_arm_poll_handler() owns the request.
Look at the flag once the request is in the hash table. The table is
searched and the request is added with ->uring_lock held, and the flag is
set before the search. So either the search finds the request, or it
came first and the flag is seen. With the request marked as cancelled, the
task work that is queued, right away or because ownership can't be
released, fails it with -ECANCELED.
What the canceller is told doesn't change. In this window a linked timeout
still completes with -ENOENT rather than -ETIME, a cancel request with
-EALREADY, and one with IORING_ASYNC_CANCEL_ALL or _ANY with a count that
doesn't include the request. But the request is cancelled, so what is new
is a request that completes with -ECANCELED after such a result.
This doesn't help in two cases:
- IORING_ASYNC_CANCEL_ALL and _ANY, if the work is flagged after the
test added here, while io-wq still has it as the current work of the
worker. io_cancel_cb() stamps it with the sequence of the cancel, so the
search of the hash table on behalf of the same cancel takes it for
done and skips it.
- requests that are parked elsewhere by io-wq, like IORING_OP_FUTEX_WAIT
and IORING_OP_WAITID. They have the same problem.
Fixes: 90fa02883f06 ("io_uring: implement async hybrid mode for pollable requests")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-14-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io_get_next_work() takes all the work items that share a hash off the
work list in one go, and the worker runs them one after the other. Only
the one it is working on is visible as ->cur_work. The others are neither
on the list nor current, so io_wq_cancel_cb() can't find them.
IORING_OP_ASYNC_CANCEL and IORING_REGISTER_SYNC_CANCEL return -ENOENT for
a request that has not even been started, e.g. a buffered write queued
behind another write to the same file that is blocked, and the request is
carried out. The cancellation done for a task or ring that goes away gets
there in the end, as it keeps trying, but it has to wait for each item to
become current.
Keep a pointer to the remainder of the chain next to ->cur_work, and have
io_wq_worker_cancel() look at it. The items on it belong to the worker,
which needs ->lock to move on to the next one. Matching work is flagged
with IO_WQ_WORK_CANCEL and fails with -ECANCELED when its turn comes. The
worker isn't signalled for it, that would interrupt the unrelated work it
is busy with. As the request isn't gone yet, it is reported like running
work, with -EALREADY.
Fixes: 86f3cd1b589a ("io-wq: handle hashed writes in chains")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-13-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
A worker that is the last one to go to sleep while work is pending asks
for another worker. io_wq_dec_running() bumps ->nr_running on behalf of
the new worker and queues create_worker_cb() as task_work for the owner
of the io-wq. Until that has run, everybody else takes it that a worker
is running which is going to look after the work list: io_wq_enqueue()
doesn't create one, and other workers that go to sleep don't find
themselves to be the last.
If the worker that made the request exits, io_worker_exit() cancels the
task_work, and io_worker_cancel_cb() drops ->nr_running without a look at
the work list:
worker A worker B io_wq_enqueue()
blocks, work is pending
nr_running++
queues create_worker_cb()
wakes, handles the work
idle timeout, decides to exit
picks up work 1
queues work 2
B is on the free list
still: wakes B
work 1 blocks
nr_running 2 -> 1
io_worker_exit()
io_worker_cancel_cb()
nr_running 1 -> 0
Work 2 has to wait for work 1, although there is room for more workers.
It takes an owner that doesn't get around to run its task_work for the
time of the idle timeout, less if the worker exits because its affinity
was changed or the io-wq is marked exit-on-idle.
Let the worker stay around until its request has been dealt with. Only
the worker itself sets ->create_state, so it can't be set behind its
back once it has decided to leave.
Fixes: 1d5f5ea7cb7d ("io-wq: remove worker to owner tw dependency")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Link: https://patch.msgid.link/20261015-b4-io-uring-fixes-v1-12-c1556f5d7bf5@toxicpanda.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
