* Re: [PATCH v2 1/3] scsi: scan: allocate sdev and starget on the NUMA node of the host adapter
From: John Garry @ 2026-04-20 12:10 UTC (permalink / raw)
To: Sumit Saxena, martin.petersen, axboe
Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, James Rizzo
In-Reply-To: <20260420113846.1401374-2-sumit.saxena@broadcom.com>
On 20/04/2026 12:38, Sumit Saxena wrote:
> From: James Rizzo <james.rizzo@broadcom.com>
>
> When a host adapter is attached to a specific NUMA node, allocating
> scsi_device and scsi_target via kzalloc() may place them on a remote
> node. All hot-path I/O accesses to these structures then cross the NUMA
> interconnect, adding latency and consuming inter-node bandwidth.
>
> Use kzalloc_node() with dev_to_node(shost->dma_dev) so allocations land
> on the same node as the HBA, reducing cross-node traffic and improving
> I/O performance on NUMA systems.
I suppose that this makes sense. We already do this sort of thing in
scsi_mq_setup_tags() (in setting numa node) and
scsi_realloc_sdev_budget_map() ->
sbitmap_init_node(sdev->request_queue->numa_node)
For the actual shost allocation, we still use kzalloc() in
scsi_host_alloc(). However, shost associated device is often a pci
device, and we probe pci devices in the same NUMA node it exists, and we
try NUMA local allocations by default, so nothing is needed to change
for the shost allocation - is this right?
>
> Signed-off-by: James Rizzo <james.rizzo@broadcom.com>
> Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
> ---
> drivers/scsi/scsi_scan.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index ef22a4228b85..9749a8dbe964 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -34,6 +34,7 @@
> #include <linux/kthread.h>
> #include <linux/spinlock.h>
> #include <linux/async.h>
> +#include <linux/topology.h>f
> #include <linux/slab.h>
> #include <linux/unaligned.h>
>
> @@ -286,9 +287,10 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
> int display_failure_msg = 1, ret;
> struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
> struct queue_limits lim;
> + int node = dev_to_node(shost->dma_dev);
this variable is only used once, so we can use
dev_to_node(shost->dma_dev) directly
>
> - sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
> - GFP_KERNEL);
> + sdev = kzalloc_node(sizeof(*sdev) + shost->transportt->device_size,
> + GFP_KERNEL, node);
> if (!sdev)
> goto out;
>
> @@ -501,8 +503,9 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
> struct scsi_target *starget;
> struct scsi_target *found_target;
> int error, ref_got;
> + int node = dev_to_node(shost->dma_dev);
same as above
>
> - starget = kzalloc(size, GFP_KERNEL);
> + starget = kzalloc_node(size, GFP_KERNEL, node);
> if (!starget) {
> printk(KERN_ERR "%s: allocation failure\n", __func__);
> return NULL;
^ permalink raw reply
* Re: [PATCH v11 03/13] lib/group_cpus: Add group_mask_cpus_evenly()
From: Marco Crivellari @ 2026-04-20 13:11 UTC (permalink / raw)
To: atomlin
Cc: James.Bottomley, MPT-FusionLinux.pdl, aacraid, akpm, axboe,
bigeasy, chandrakanth.patil, chenridong, chjohnst, frederic, hare,
hch, jinpu.wang, juri.lelli, kashyap.desai, kbusch, kch,
linux-block, linux-kernel, linux-nvme, linux-scsi, liyihang9,
longman, martin.petersen, maz, megaraidlinux.pdl, ming.lei, mingo,
mpi3mr-linuxdrv.pdl, mproche, mst, neelx, nick.lange, peterz,
ranjan.kumar, ruanjinjie, sagi, sathya.prakash, sean,
shivasharan.srikanteshwara, sreekanth.reddy, steve,
suganath-prabu.subramani, sumit.saxena, tglx, tom.leiming,
vincent.guittot, virtualization, wagi, yphbchou0911
In-Reply-To: <20260416192942.1243421-4-atomlin@atomlin.com>
Hi,
Maybe I'm missing some background here, I noticed group_mask_cpus_evenly()
is called inside irq_create_affinity_masks() and the "numgrps" it is this_vecs
in the latter.
Looking to group_cpus_evenly(), seems it is possible to have this_vecs
equals to 0, indeed this function return NULL if numgrps == 0:
>488 struct cpumask *group_cpus_evenly(unsigned int numgrps, unsigned int *nummasks)
>489 {
>[...]
>495
>496 if (numgrps == 0)
>497 return NULL;
Without it, I guess the kmalloc() in `group_mask_cpus_evenly()` will return ZERO_SIZE_PTR:
>+struct cpumask *group_mask_cpus_evenly(unsigned int numgrps,
>+ const struct cpumask *mask,
>+ unsigned int *nummasks)
>+{
>
> [...]
>
>+ masks = kcalloc(numgrps, sizeof(*masks), GFP_KERNEL);
>+ if (!masks)
>+ goto fail_node_to_cpumask;
and this value is then passed to `__group_cpus_evenly()`.
Should this check be added or it is not needed? Or maybe rely on `ZERO_OR_NULL_PTR()` ?
Thanks!
--
Marco Crivellari
SUSE Labs
^ permalink raw reply
* Re: [PATCH 6.6.y] blktrace: fix __this_cpu_read/write in preemptible context
From: Sasha Levin @ 2026-04-20 13:21 UTC (permalink / raw)
To: Rajani Kantha; +Cc: Sasha Levin, stable, linux-block, Jens Axboe
In-Reply-To: <20260416100859.2492-1-681739313@139.com>
On Thu, Apr 16, 2026, Rajani Kantha wrote:
> Backport of da46b5dfef48 ("blktrace: fix __this_cpu_read/write in
> preemptible context") to 6.6.y.
Queued for 6.6, thanks.
--
Thanks,
Sasha
^ permalink raw reply
* [PATCH AUTOSEL 7.0] block: reject zero length in bio_add_page()
From: Sasha Levin @ 2026-04-20 13:17 UTC (permalink / raw)
To: patches, stable
Cc: Qu Wenruo, Jens Axboe, Sasha Levin, linux-block, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Qu Wenruo <wqu@suse.com>
[ Upstream commit 643893647cac7317bafca4040dd0cfb815b510d4 ]
The function bio_add_page() returns the number of bytes added to the
bio, and if that failed it should return 0.
However there is a special quirk, if a caller is passing a page with
length 0, that function will always return 0 but with different results:
- The page is added to the bio
If there is enough bvec slot or the folio can be merged with the last
bvec.
The return value 0 is just the length passed in, which is also 0.
- The page is not added to the bio
If the page is not mergeable with the last bvec, or there is no bvec
slot available.
The return value 0 means page is not added into the bio.
Unfortunately the caller is not able to distinguish the above two cases,
and will treat the 0 return value as page addition failure.
In that case, this can lead to the double releasing of the last page:
- By the bio cleanup
Which normally goes through every page of the bio, including the last
page which is added into the bio.
- By the caller
Which believes the page is not added into the bio, thus would manually
release the page.
I do not think anyone should call bio_add_folio()/bio_add_page() with zero
length, but idiots like me can still show up.
So add an extra WARN_ON_ONCE() check for zero length and rejects it
early to avoid double freeing.
Signed-off-by: Qu Wenruo <wqu@suse.com>
Link: https://patch.msgid.link/bc2223c080f38d0b63f968f605c918181c840f40.1773734749.git.wqu@suse.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have enough information for a comprehensive analysis. Let me
compile my findings.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: Subject Line**
- Subsystem: `block` (core block layer)
- Action verb: `reject` (defensive fix/hardening)
- Summary: Reject zero-length pages in `bio_add_page()` to prevent
double-free
**Step 1.2: Tags**
- `Signed-off-by: Qu Wenruo <wqu@suse.com>` - Author, btrfs developer
- `Link:` patch.msgid.link URL (lore blocked by anti-scraping)
- `Signed-off-by: Jens Axboe <axboe@kernel.dk>` - Block layer maintainer
committed it
- No `Fixes:` tag (expected for candidates), no `Cc: stable`, no
`Reported-by`
**Step 1.3: Commit Body Analysis**
The commit describes a clear bug mechanism:
- When `len=0` is passed to `bio_add_page()`, return value is always 0
- But 0 can mean "success with zero bytes" (page added) OR "failure"
(page not added)
- Caller cannot distinguish these two cases, treats return 0 as failure
- If the page WAS added, the caller releases the page manually (thinking
it wasn't added), AND the bio cleanup also releases it → **double-
free**
The author says: "I do not think anyone should call
bio_add_folio()/bio_add_page() with zero length, but idiots like me can
still show up" — referencing his own btrfs zlib bug.
**Step 1.4: Hidden Bug Fix Detection**
This IS a bug fix. While framed as adding a defensive check, it prevents
a concrete double-free scenario that was actually triggered in btrfs
(commit `0dcabcb920a5c`).
Record: [block] [reject] [Adds WARN_ON_ONCE check for zero-length to
prevent double-free from API ambiguity]
## PHASE 2: DIFF ANALYSIS
**Step 2.1: Inventory**
- 1 file changed: `block/bio.c`
- +2 lines added (only)
- Function modified: `bio_add_page()`
- Scope: Single-file, single-function, surgical fix
**Step 2.2: Code Flow Change**
Single hunk: After the BIO_CLONED check, adds:
```c
if (WARN_ON_ONCE(len == 0))
return 0;
```
Before: zero-length pages could be silently added, causing return value
ambiguity.
After: zero-length is rejected early with a WARN, returning 0
unambiguously meaning failure.
**Step 2.3: Bug Mechanism**
Category: Double-free prevention (memory safety fix). The zero-length
case creates an ambiguous return path where the page can be freed by
both the bio cleanup and the caller.
**Step 2.4: Fix Quality**
- Obviously correct — nobody should add zero bytes to a bio
- Minimal — 2 lines
- No regression risk — no valid caller should pass len=0
- WARN_ON_ONCE is low-overhead, fires once per boot maximum
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: Blame**
`bio_add_page()` was refactored by Christoph Hellwig in commit
`0aa69fd32a5f76` (2018-06-01), but the fundamental function dates back
to Linus's original `1da177e4c3f41` (2005). The zero-length ambiguity
has existed since the function's creation.
**Step 3.2: Fixes tag** — No Fixes: tag present. The bug is in the API
design of `bio_add_page()` itself, not introduced by a specific commit.
**Step 3.3: File History**
`block/bio.c` has been actively modified — 159 commits since v6.6.
Recent refactoring by Christoph Hellwig (`38446014648c9`,
`12da89e8844ae`) changed the merge logic but didn't address zero-length.
**Step 3.4: Author**
Qu Wenruo is a prolific btrfs developer. He discovered this issue while
debugging the btrfs zlib crash (`0dcabcb920a5c`), which was reported by
David Sterba and syzbot. He fixed both the btrfs caller AND added this
block-level defense.
**Step 3.5: Dependencies**
None. The 2-line check has no prerequisites. It uses only existing
macros (`WARN_ON_ONCE`).
## PHASE 4: MAILING LIST RESEARCH
**Step 4.1-4.5:** Lore.kernel.org was blocked by anti-scraping
protection. However, from examining the related btrfs fix commit
(`0dcabcb920a5c`), I can confirm:
- The bug was reported by David Sterba (btrfs maintainer), Jean-
Christophe Guillain (user), and syzbot
- A bugzilla was filed:
https://bugzilla.kernel.org/show_bug.cgi?id=221176
- The root cause was bio_add_folio/bio_add_page accepting zero-length
- The fix was signed off by Jens Axboe (block maintainer)
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1:** Modified function: `bio_add_page()`
**Step 5.2: Callers**
`bio_add_page()` is called from 44+ files across the kernel: filesystems
(btrfs, gfs2, ocfs2, ntfs3, f2fs, squashfs, nfs, erofs, direct-io),
block layer (blk-map, blk-crypto), device mapper (dm-crypt, dm-io, dm-
writecache, dm-log-writes, dm-flakey, dm-zoned), RAID (raid1, raid5,
raid10), NVMe target, SCSI target, drbd, zram, xen-blkback, floppy. This
is a CORE API.
**Step 5.3:** `bio_add_page` calls `bvec_try_merge_page` and
`__bio_add_page`, manipulating bio vectors.
**Step 5.4:** Any filesystem or block driver issuing I/O can reach this
function. It's on the hot path for ALL block I/O.
**Step 5.5:** The same zero-length ambiguity exists in `bio_add_folio()`
which wraps `bio_add_page()`, so this fix protects both paths.
## PHASE 6: STABLE TREE ANALYSIS
**Step 6.1:** `bio_add_page()` exists in ALL stable trees (present since
2005). The zero-length ambiguity has existed since the beginning.
**Step 6.2: Backport Compatibility**
- v6.6/v6.12: Function has slightly different structure (uses
`same_page` variable, `bvec_try_merge_page` has different signature),
but the fix location (after the BIO_CLONED check, before the size
check) is identical. Patch should apply cleanly or with trivial
context adjustment.
- v6.1: Function uses `__bio_try_merge_page()` instead. Fix still
applies at the top of the function.
- v5.15: Same as v6.1.
**Step 6.3:** No related zero-length fix exists in any stable tree.
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1:** Block layer (`block/`) — **CORE** criticality. Affects all
users who do any I/O.
**Step 7.2:** Actively developed subsystem (20+ recent commits).
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1:** Universal — every kernel user performs block I/O through
`bio_add_page()`.
**Step 8.2: Trigger Conditions**
Currently, the btrfs zlib path (`3d74a7556fba`, only in 7.0+) was the
known trigger. In stable trees, no known caller currently passes zero-
length. However, any future backported fix or existing edge case that
accidentally computes zero-length would trigger the double-free.
**Step 8.3: Failure Mode**
Double-free of a page → memory corruption, crash, or security
vulnerability. Severity: **CRITICAL** when triggered.
**Step 8.4: Risk-Benefit Ratio**
- BENEFIT: Prevents double-free from API misuse; hardens a core API used
by 44+ files
- RISK: Effectively zero — 2 lines, adds a check for an invalid input
that should never occur
- Ratio: Very favorable
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: Evidence Compilation**
FOR backporting:
- Prevents double-free (memory safety, potential security issue)
- 2-line fix, obviously correct, zero regression risk
- Core block layer API used by 44+ files
- Real bug was triggered (btrfs zlib crash with syzbot report + user
reports)
- Block maintainer (Jens Axboe) signed off
- The API ambiguity exists in all stable kernels
- No caller should ever pass zero-length; this enforces a correct
invariant
AGAINST backporting:
- The specific known trigger (btrfs zlib) only exists in 7.0+ code
- No known caller in stable trees currently passes zero-length
- Somewhat defensive/hardening in nature for older stable trees
**Step 9.2: Stable Rules Checklist**
1. Obviously correct? **YES** — trivially verifiable
2. Fixes a real bug? **YES** — double-free is real, demonstrated in
btrfs
3. Important issue? **YES** — double-free = memory corruption/crash
4. Small and contained? **YES** — 2 lines in 1 file
5. No new features? **YES** — purely defensive check
6. Applies to stable? **YES** — with minor context adjustments
**Step 9.3: Exception Categories** — N/A
**Step 9.4: Decision**
The fix is tiny, obviously correct, and addresses a genuine API-level
design flaw that leads to double-free when any caller passes zero-
length. While the known trigger exists only in 7.0+, the underlying API
ambiguity has existed since 2005 and could be triggered by any of the
44+ callers if they ever compute a zero-length. The defensive hardening
of such a critical, widely-used API is appropriate for stable.
## Verification
- [Phase 1] Parsed tags: Signed-off-by Qu Wenruo (author), Jens Axboe
(block maintainer), Link: tag present
- [Phase 2] Diff analysis: 2 lines added to `bio_add_page()` —
WARN_ON_ONCE(len == 0) + return 0
- [Phase 3] git blame: bio_add_page refactored in 0aa69fd32a5f76 (2018),
original from 1da177e (2005); zero-length bug exists since origin
- [Phase 3] Author check: Qu Wenruo is prolific btrfs developer,
discovered bug while fixing btrfs zlib crash (0dcabcb920a5c)
- [Phase 3] Related commit 0dcabcb920a5c confirmed: btrfs zlib double-
free from zero-length bio_add_folio, reported by David Sterba, syzbot,
and user
- [Phase 4] Lore blocked by anti-scraping; patch link confirmed via
commit message
- [Phase 5] grep found 44+ files calling bio_add_page() across fs/,
drivers/, block/ — CORE API
- [Phase 6] bio_add_page in v6.6, v6.1, v5.15 confirmed via git show —
function exists in all stable trees; fix applies at same location (top
of function)
- [Phase 6] No existing zero-length check in any stable tree version
confirmed
- [Phase 7] Block layer — CORE subsystem, affects all users
- [Phase 8] Double-free → memory corruption → CRITICAL severity when
triggered; 2-line fix → zero regression risk
- UNVERIFIED: Could not access lore.kernel.org to read full review
discussion due to anti-scraping protection
**YES**
block/bio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/block/bio.c b/block/bio.c
index d80d5d26804e3..6048d9382fecf 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1064,6 +1064,8 @@ int bio_add_page(struct bio *bio, struct page *page,
{
if (WARN_ON_ONCE(bio_flagged(bio, BIO_CLONED)))
return 0;
+ if (WARN_ON_ONCE(len == 0))
+ return 0;
if (bio->bi_iter.bi_size > BIO_MAX_SIZE - len)
return 0;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.6] blk-iocost: fix busy_level reset when no IOs complete
From: Sasha Levin @ 2026-04-20 13:18 UTC (permalink / raw)
To: patches, stable
Cc: Jialin Wang, Tejun Heo, Jens Axboe, Sasha Levin, josef, cgroups,
linux-block, linux-kernel
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>
From: Jialin Wang <wjl.linux@gmail.com>
[ Upstream commit f91ffe89b2016d280995a9c28d73288b02d83615 ]
When a disk is saturated, it is common for no IOs to complete within a
timer period. Currently, in this case, rq_wait_pct and missed_ppm are
calculated as 0, the iocost incorrectly interprets this as meeting QoS
targets and resets busy_level to 0.
This reset prevents busy_level from reaching the threshold (4) needed
to reduce vrate. On certain cloud storage, such as Azure Premium SSD,
we observed that iocost may fail to reduce vrate for tens of seconds
during saturation, failing to mitigate noisy neighbor issues.
Fix this by tracking the number of IO completions (nr_done) in a period.
If nr_done is 0 and there are lagging IOs, the saturation status is
unknown, so we keep busy_level unchanged.
The issue is consistently reproducible on Azure Standard_D8as_v5 (Dasv5)
VMs with 512GB Premium SSD (P20) using the script below. It was not
observed on GCP n2d VMs (with 100G pd-ssd and 1.5T local-ssd), and no
regressions were found with this patch. In this script, cgA performs
large IOs with iodepth=128, while cgB performs small IOs with iodepth=1
rate_iops=100 rw=randrw. With iocost enabled, we expect it to throttle
cgA, the submission latency (slat) of cgA should be significantly higher,
cgB can reach 200 IOPS and the completion latency (clat) should below.
BLK_DEVID="8:0"
MODEL="rbps=173471131 rseqiops=3566 rrandiops=3566 wbps=173333269 wseqiops=3566 wrandiops=3566"
QOS="rpct=90 rlat=3500 wpct=90 wlat=3500 min=80 max=10000"
echo "$BLK_DEVID ctrl=user model=linear $MODEL" > /sys/fs/cgroup/io.cost.model
echo "$BLK_DEVID enable=1 ctrl=user $QOS" > /sys/fs/cgroup/io.cost.qos
CG_A="/sys/fs/cgroup/cgA"
CG_B="/sys/fs/cgroup/cgB"
FILE_A="/path/to/sda/A.fio.testfile"
FILE_B="/path/to/sda/B.fio.testfile"
RESULT_DIR="./iocost_results_$(date +%Y%m%d_%H%M%S)"
mkdir -p "$CG_A" "$CG_B" "$RESULT_DIR"
get_result() {
local file=$1
local label=$2
local results=$(jq -r '
.jobs[0].mixed |
( .iops | tonumber | round ) as $iops |
( .bw_bytes / 1024 / 1024 ) as $bps |
( .slat_ns.mean / 1000000 ) as $slat |
( .clat_ns.mean / 1000000 ) as $avg |
( .clat_ns.max / 1000000 ) as $max |
( .clat_ns.percentile["90.000000"] / 1000000 ) as $p90 |
( .clat_ns.percentile["99.000000"] / 1000000 ) as $p99 |
( .clat_ns.percentile["99.900000"] / 1000000 ) as $p999 |
( .clat_ns.percentile["99.990000"] / 1000000 ) as $p9999 |
"\($iops)|\($bps)|\($slat)|\($avg)|\($max)|\($p90)|\($p99)|\($p999)|\($p9999)"
' "$file")
IFS='|' read -r iops bps slat avg max p90 p99 p999 p9999 <<<"$results"
printf "%-8s %-6s %-7.2f %-8.2f %-8.2f %-8.2f %-8.2f %-8.2f %-8.2f %-8.2f\n" \
"$label" "$iops" "$bps" "$slat" "$avg" "$max" "$p90" "$p99" "$p999" "$p9999"
}
run_fio() {
local cg_path=$1
local filename=$2
local name=$3
local bs=$4
local qd=$5
local out=$6
shift 6
local extra=$@
(
pid=$(sh -c 'echo $PPID')
echo $pid >"${cg_path}/cgroup.procs"
fio --name="$name" --filename="$filename" --direct=1 --rw=randrw --rwmixread=50 \
--ioengine=libaio --bs="$bs" --iodepth="$qd" --size=4G --runtime=10 \
--time_based --group_reporting --unified_rw_reporting=mixed \
--output-format=json --output="$out" $extra >/dev/null 2>&1
) &
}
echo "Starting Test ..."
for bs_b in "4k" "32k" "256k"; do
echo "Running iteration: BS=$bs_b"
out_a="${RESULT_DIR}/cgA_1m.json"
out_b="${RESULT_DIR}/cgB_${bs_b}.json"
# cgA: Heavy background (BS 1MB, QD 128)
run_fio "$CG_A" "$FILE_A" "cgA" "1m" 128 "$out_a"
# cgB: Latency sensitive (Variable BS, QD 1, Read/Write IOPS limit 100)
run_fio "$CG_B" "$FILE_B" "cgB" "$bs_b" 1 "$out_b" "--rate_iops=100"
wait
SUMMARY_DATA+="$(get_result "$out_a" "cgA-1m")"$'\n'
SUMMARY_DATA+="$(get_result "$out_b" "cgB-$bs_b")"$'\n\n'
done
echo -e "\nFinal Results Summary:\n"
printf "%-8s %-6s %-7s %-8s %-8s %-8s %-8s %-8s %-8s %-8s\n" \
"" "" "" "slat" "clat" "clat" "clat" "clat" "clat" "clat"
printf "%-8s %-6s %-7s %-8s %-8s %-8s %-8s %-8s %-8s %-8s\n\n" \
"CGROUP" "IOPS" "MB/s" "avg(ms)" "avg(ms)" "max(ms)" "P90(ms)" "P99" "P99.9" "P99.99"
echo "$SUMMARY_DATA"
echo "Results saved in $RESULT_DIR"
Before:
slat clat clat clat clat clat clat
CGROUP IOPS MB/s avg(ms) avg(ms) max(ms) P90(ms) P99 P99.9 P99.99
cgA-1m 166 166.37 3.44 748.95 1298.29 977.27 1233.13 1300.23 1300.23
cgB-4k 5 0.02 0.02 181.74 761.32 742.39 759.17 759.17 759.17
cgA-1m 167 166.51 1.98 748.68 1549.41 809.50 1451.23 1551.89 1551.89
cgB-32k 6 0.18 0.02 169.98 761.76 742.39 759.17 759.17 759.17
cgA-1m 166 165.55 2.89 750.89 1540.37 851.44 1451.23 1535.12 1535.12
cgB-256k 5 1.30 0.02 191.35 759.51 750.78 759.17 759.17 759.17
After:
slat clat clat clat clat clat clat
CGROUP IOPS MB/s avg(ms) avg(ms) max(ms) P90(ms) P99 P99.9 P99.99
cgA-1m 162 162.48 6.14 749.69 850.02 826.28 834.67 843.06 851.44
cgB-4k 199 0.78 0.01 1.95 42.12 2.57 7.50 34.87 42.21
cgA-1m 146 146.20 6.83 833.04 908.68 893.39 901.78 910.16 910.16
cgB-32k 200 6.25 0.01 2.32 31.40 3.06 7.50 16.58 31.33
cgA-1m 110 110.46 9.04 1082.67 1197.91 1182.79 1199.57 1199.57 1199.57
cgB-256k 200 49.98 0.02 3.69 22.20 4.88 9.11 20.05 22.15
Signed-off-by: Jialin Wang <wjl.linux@gmail.com>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20260331100509.182882-1-wjl.linux@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
I now have all the information needed for a comprehensive analysis. Let
me compile the findings.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Subject Line
- **Subsystem:** `blk-iocost` (block layer IO cost controller)
- **Action verb:** "fix"
- **Summary:** Fix incorrect busy_level reset when no IO completions
occur within a timer period
- Record: [blk-iocost] [fix] [busy_level incorrectly reset to 0 when no
IOs complete, preventing vrate reduction during saturation]
### Step 1.2: Tags
- **Signed-off-by:** Jialin Wang <wjl.linux@gmail.com> (author)
- **Acked-by:** Tejun Heo <tj@kernel.org> (blk-iocost creator and
maintainer)
- **Link:**
https://patch.msgid.link/20260331100509.182882-1-wjl.linux@gmail.com
- **Signed-off-by:** Jens Axboe <axboe@kernel.dk> (block layer
maintainer)
- No Fixes: tag (expected for AUTOSEL candidates)
- No Cc: stable (expected for AUTOSEL candidates)
- Record: Acked by the subsystem maintainer (Tejun Heo) and merged by
the block layer maintainer (Jens Axboe). Strong quality signals.
### Step 1.3: Commit Body Analysis
- **Bug description:** When a disk is saturated, no IOs may complete
within a timer period. When this happens, rq_wait_pct=0 and
missed_ppm=0, which iocost incorrectly interprets as "meeting QoS
targets."
- **Symptom:** busy_level gets reset to 0, preventing it from reaching
threshold (4) needed to reduce vrate. On certain cloud storage (Azure
Premium SSD), iocost can fail to reduce vrate for tens of seconds
during saturation, breaking cgroup IO isolation.
- **Failure mode:** Noisy neighbor problem - heavy IO from one cgroup is
not properly throttled, causing high latency for latency-sensitive
workloads in other cgroups.
- **Testing:** Detailed benchmark script provided with before/after
results showing dramatic improvement (cgB: 5 IOPS -> 200 IOPS; clat:
181ms -> 1.95ms).
- Record: Clear real-world bug with concrete impact on cloud
environments. Reproducible with specific test setup.
### Step 1.4: Hidden Bug Fix Detection
This is explicitly labeled as "fix" - no hidden nature. The commit
message clearly explains the bug mechanism and the fix approach.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory
- **Files changed:** 1 (`block/blk-iocost.c`)
- **Lines added/removed:** ~15 lines of actual code (plus comments)
- **Functions modified:** `ioc_lat_stat()` (signature + 1 line),
`ioc_timer_fn()` (variable + call site + new branch)
- **Scope:** Single-file, surgical fix
- Record: Minimal change in a single file affecting two functions in the
same subsystem.
### Step 2.2: Code Flow Change
**Hunk 1 - `ioc_lat_stat()` signature:**
- Before: `ioc_lat_stat(ioc, missed_ppm_ar, rq_wait_pct_p)` - 2 output
params
- After: `ioc_lat_stat(ioc, missed_ppm_ar, rq_wait_pct_p, nr_done)` - 3
output params
- Adds computation: `*nr_done = nr_met[READ] + nr_met[WRITE] +
nr_missed[READ] + nr_missed[WRITE]`
**Hunk 2 - `ioc_timer_fn()` variable:**
- Adds `u32 nr_done` variable and passes `&nr_done` to `ioc_lat_stat()`.
**Hunk 3 - busy_level decision logic:**
- Before: Directly checks `rq_wait_pct > RQ_WAIT_BUSY_PCT ||
missed_ppm...`
- After: First checks `if (!nr_done && nr_lagging)` - if no completions
and lagging IOs exist, skip all busy_level changes (keep unchanged).
Otherwise, proceed with existing logic.
### Step 2.3: Bug Mechanism
This is a **logic/correctness fix**. When no IOs complete during a timer
period:
1. `nr_met` and `nr_missed` are both 0 → `missed_ppm = 0`
2. `rq_wait_ns = 0` → `rq_wait_pct = 0`
3. All metrics being 0 falls into the "UNBUSY" branch (second condition)
4. If `nr_shortages = 0`: `busy_level` is reset to 0 (the bug)
5. This prevents `busy_level` from ever reaching 4, which is required to
trigger vrate reduction
The fix adds a guard: when `nr_done == 0 && nr_lagging > 0`, the
saturation status is truly unknown, so busy_level is preserved
unchanged.
### Step 2.4: Fix Quality
- Obviously correct: if we have zero completions, we have zero data to
make QoS decisions
- Minimal/surgical: only adds a new guard condition before the existing
logic
- Regression risk: Very low. The new code path only triggers when
`nr_done == 0 && nr_lagging > 0`, and it preserves the previous state
rather than making any change.
- The existing logic is completely unchanged in all other cases.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame
The buggy busy_level decision logic was introduced by:
- `7caa47151ab2e6` (Tejun Heo, 2019-08-28): Original blk-iocost
implementation (v5.4)
- `81ca627a933063` (Tejun Heo, 2019-10-14): "iocost: don't let vrate run
wild while there's no saturation signal" - This commit restructured
the busy_level logic and added the `else { ioc->busy_level = 0; }`
branch for "Nobody is being throttled." This is the commit that
introduced the specific behavior this fix addresses. Present since
v5.8.
### Step 3.2: Fixes Tag
No explicit Fixes: tag. However, the buggy behavior was introduced by
81ca627a933063, which is present in v5.8+ (including all currently
active stable trees: v5.10, v5.15, v6.1, v6.6, v6.12).
### Step 3.3: File History
Recent blk-iocost changes are mostly unrelated (hrtimer_setup, min_t
cleanup, treewide conversions). No conflicting changes. The busy_level
decision logic has been stable since 81ca627a933063 with only one minor
change (065655c862fedf removed `nr_surpluses` check).
### Step 3.4: Author
Jialin Wang is not the regular blk-iocost maintainer, but the fix was
acked by Tejun Heo (creator and maintainer of blk-iocost) and merged by
Jens Axboe (block layer maintainer).
### Step 3.5: Dependencies
No dependencies. The patch is self-contained and the code it modifies is
identical across all stable trees (v5.10 through current mainline,
verified by comparing `ioc_lat_stat()` and `ioc_timer_fn()` busy_level
logic).
---
## PHASE 4: MAILING LIST RESEARCH
### Step 4.1-4.5
Lore.kernel.org was blocked by bot protection. Web search found limited
direct results. However, the commit has strong signals:
- **Acked-by: Tejun Heo** - the creator and primary maintainer of blk-
iocost
- **Merged by: Jens Axboe** - the block layer maintainer
- The commit was merged via the standard block tree path
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Functions Modified
- `ioc_lat_stat()`: Collects per-CPU IO latency statistics
- `ioc_timer_fn()`: The main timer callback that evaluates QoS and
adjusts vrate
### Step 5.2: Callers
- `ioc_lat_stat()` is called only from `ioc_timer_fn()` (single call
site)
- `ioc_timer_fn()` is the periodic timer callback for the IO cost
controller, runs once per period
### Step 5.3: Impact Surface
The `ioc_timer_fn()` timer runs periodically for every block device with
iocost enabled. The busy_level directly controls vrate adjustment, which
governs IO throttling for cgroups. This is the core feedback loop of the
entire iocost controller.
### Step 5.4: Call Chain
`timer_list callback` → `ioc_timer_fn()` → evaluates QoS → adjusts
`busy_level` → calls `ioc_adjust_base_vrate()` → adjusts
`vtime_base_rate`. This path is always active when iocost is enabled.
---
## PHASE 6: STABLE TREE ANALYSIS
### Step 6.1: Code Existence in Stable Trees
Verified the exact busy_level decision logic is **byte-for-byte
identical** in:
- v5.10 (line 2272-2310)
- v5.15 (line 2348-2390)
- v6.1 (line 2354-2396)
- v6.6 (line 2381-2420)
- v7.0 mainline (line 2399-2435)
The `ioc_lat_stat()` function is also identical across all these
versions.
### Step 6.2: Backport Complications
The patch should apply **cleanly** to all active stable trees. The code
context is identical. In v5.10, `ioc_adjust_base_vrate()` is inline
rather than a separate function, but the busy_level decision logic
(where the patch applies) is identical.
### Step 6.3: No Related Fixes Already in Stable
No prior fix for this specific issue was found in stable trees.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem
- **Subsystem:** Block layer - IO cost controller (blk-iocost)
- **Criticality:** IMPORTANT - affects all users of cgroup v2 IO
control, widely used in cloud environments (systemd, container
orchestrators, cloud VMs)
### Step 7.2: Activity
The subsystem is mature with occasional fixes. The busy_level logic
hasn't changed since 2020, indicating this is a long-standing bug.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
### Step 8.1: Affected Users
- All users of cgroup v2 IO cost control during disk saturation
- Particularly cloud users on virtualized block devices (Azure, etc.)
- Container environments using IO throttling (Kubernetes, Docker with
cgroup v2)
### Step 8.2: Trigger Conditions
- Disk saturated with large IOs (e.g., 1MB writes at high queue depth)
- Timer period passes with zero IO completions
- **Common trigger:** Any scenario where IO completion time exceeds the
timer period (~10ms-100ms typically)
- Reproducible consistently on certain cloud storage types
### Step 8.3: Severity
- **Failure mode:** IO isolation failure - a noisy neighbor cgroup
cannot be properly throttled during saturation
- **Impact:** Latency-sensitive workloads experience 100x+ higher
latency than expected (181ms vs 1.95ms in the benchmark)
- **Severity:** HIGH - not a crash, but a significant functional failure
of the IO controller that defeats its core purpose
### Step 8.4: Risk-Benefit
- **Benefit:** HIGH - Fixes IO isolation during saturation, critical for
cloud multi-tenant environments
- **Risk:** VERY LOW - ~15 lines, only adds a "preserve state" guard
condition, all existing paths unchanged
- **Ratio:** Strongly favorable for backporting
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence Summary
**FOR backporting:**
- Fixes a real, reproducible bug with concrete user impact
- Affects IO isolation in cloud/container environments (high-value use
case)
- Small, surgical fix (~15 lines actual code in 1 file)
- Obviously correct (no completions = no data = preserve state)
- Acked by subsystem creator (Tejun Heo)
- Merged by block maintainer (Jens Axboe)
- Detailed testing with before/after benchmarks showing 40x improvement
- Code is identical across all stable trees - clean apply expected
- Bug present since v5.8 (81ca627a933063), affects v5.10+ stable trees
- No regression found in testing (including GCP which worked before)
**AGAINST backporting:**
- No explicit Cc: stable or Fixes: tag (expected for AUTOSEL)
- Not a crash/panic - it's a performance/isolation failure
- The commit message is long (includes test script), but the actual diff
is small
### Step 9.2: Stable Rules Checklist
1. **Obviously correct?** YES - if zero IOs completed, QoS metrics are
meaningless
2. **Fixes real bug?** YES - IO throttling fails during saturation,
breaking cgroup isolation
3. **Important issue?** YES - significant performance isolation failure
in cloud environments
4. **Small and contained?** YES - ~15 lines in 1 file
5. **No new features?** CORRECT - only fixes existing logic
6. **Can apply to stable?** YES - code is identical across all stable
trees
### Step 9.3: Exception Categories
Not an exception category - this is a standard important bug fix.
---
## Verification
- [Phase 1] Parsed tags: Acked-by Tejun Heo (blk-iocost maintainer),
Signed-off-by Jens Axboe (block maintainer)
- [Phase 2] Diff analysis: ~15 lines added in `ioc_lat_stat()` and
`ioc_timer_fn()`, adds nr_done tracking and guard condition
- [Phase 3] git blame: buggy code introduced in 81ca627a933063 (v5.8,
"iocost: don't let vrate run wild"), verified present in all stable
trees
- [Phase 3] git show 81ca627a933063: confirmed it added the `else {
busy_level = 0 }` branch that causes the bug
- [Phase 3] git merge-base: confirmed 81ca627a933063 is in v5.8+,
present in v5.10, v5.15, v6.1, v6.6
- [Phase 3] git log --oneline -20: no conflicting or prerequisite
changes found
- [Phase 4] b4 dig: could not find commit (not yet in tree);
lore.kernel.org blocked by bot protection
- [Phase 5] Grep for callers: `ioc_lat_stat()` called only from
`ioc_timer_fn()`, single call site
- [Phase 5] Grep for busy_level: 20 references in file, all consistent
with this fix
- [Phase 6] Compared ioc_lat_stat() across v5.10, v5.15, v6.1, v6.6,
v7.0: byte-for-byte identical
- [Phase 6] Compared busy_level decision logic across v5.10, v5.15,
v6.1, v6.6, v7.0: structurally identical
- [Phase 6] No prior fix for this issue found in any stable tree
- [Phase 8] Failure mode: IO isolation failure, 40x latency increase for
latency-sensitive workloads, severity HIGH
- UNVERIFIED: Mailing list discussion (lore blocked), but commit has
strong trust signals (Acked-by maintainer, merged via standard path)
The fix is small, surgical, obviously correct, acked by the subsystem
maintainer, fixes a real-world IO isolation failure affecting cloud
environments, and applies cleanly to all active stable trees. The risk
is very low (adds only a "preserve state" guard) while the benefit is
high (restores IO throttling during disk saturation).
**YES**
block/blk-iocost.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index d145db61e5c31..0cca88a366dc9 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -1596,7 +1596,8 @@ static enum hrtimer_restart iocg_waitq_timer_fn(struct hrtimer *timer)
return HRTIMER_NORESTART;
}
-static void ioc_lat_stat(struct ioc *ioc, u32 *missed_ppm_ar, u32 *rq_wait_pct_p)
+static void ioc_lat_stat(struct ioc *ioc, u32 *missed_ppm_ar, u32 *rq_wait_pct_p,
+ u32 *nr_done)
{
u32 nr_met[2] = { };
u32 nr_missed[2] = { };
@@ -1633,6 +1634,8 @@ static void ioc_lat_stat(struct ioc *ioc, u32 *missed_ppm_ar, u32 *rq_wait_pct_p
*rq_wait_pct_p = div64_u64(rq_wait_ns * 100,
ioc->period_us * NSEC_PER_USEC);
+
+ *nr_done = nr_met[READ] + nr_met[WRITE] + nr_missed[READ] + nr_missed[WRITE];
}
/* was iocg idle this period? */
@@ -2250,12 +2253,12 @@ static void ioc_timer_fn(struct timer_list *timer)
u64 usage_us_sum = 0;
u32 ppm_rthr;
u32 ppm_wthr;
- u32 missed_ppm[2], rq_wait_pct;
+ u32 missed_ppm[2], rq_wait_pct, nr_done;
u64 period_vtime;
int prev_busy_level;
/* how were the latencies during the period? */
- ioc_lat_stat(ioc, missed_ppm, &rq_wait_pct);
+ ioc_lat_stat(ioc, missed_ppm, &rq_wait_pct, &nr_done);
/* take care of active iocgs */
spin_lock_irq(&ioc->lock);
@@ -2397,9 +2400,17 @@ static void ioc_timer_fn(struct timer_list *timer)
* and should increase vtime rate.
*/
prev_busy_level = ioc->busy_level;
- if (rq_wait_pct > RQ_WAIT_BUSY_PCT ||
- missed_ppm[READ] > ppm_rthr ||
- missed_ppm[WRITE] > ppm_wthr) {
+ if (!nr_done && nr_lagging) {
+ /*
+ * When there are lagging IOs but no completions, we don't
+ * know if the IO latency will meet the QoS targets. The
+ * disk might be saturated or not. We should not reset
+ * busy_level to 0 (which would prevent vrate from scaling
+ * up or down), but rather to keep it unchanged.
+ */
+ } else if (rq_wait_pct > RQ_WAIT_BUSY_PCT ||
+ missed_ppm[READ] > ppm_rthr ||
+ missed_ppm[WRITE] > ppm_wthr) {
/* clearly missing QoS targets, slow down vrate */
ioc->busy_level = max(ioc->busy_level, 0);
ioc->busy_level++;
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 1/2] Revert "nvmet-tcp: Don't free SQ on authentication success"
From: Keith Busch @ 2026-04-20 16:23 UTC (permalink / raw)
To: alistair23
Cc: hare, hch, sagi, kch, linux-nvme, linux-kernel, yi.zhang,
mlombard, linux-block, shinichiro.kawasaki, Alistair Francis
In-Reply-To: <20260417004809.2894745-1-alistair.francis@wdc.com>
On Fri, Apr 17, 2026 at 10:48:08AM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
>
> In an attempt to fix REPLACETLSPSK we stopped freeing the secrets on
> successful connections. This resulted in memory leaks in the kernel, so
> let's revert the commit. A improved fix is being developed to just avoid
> clearing the tls_key variable.
>
> This reverts commit 2e6eb6b277f593b98f151ea8eff1beb558bbea3b.
Thanks, patches 1 and 2 applied to nvme-7.1.
^ permalink raw reply
* Re: [PATCH v2 3/3] scsi: use percpu counters for iorequest_cnt and iodone_cnt
From: Bart Van Assche @ 2026-04-20 16:41 UTC (permalink / raw)
To: Sumit Saxena, martin.petersen, axboe
Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl
In-Reply-To: <20260420113846.1401374-4-sumit.saxena@broadcom.com>
On 4/20/26 4:38 AM, Sumit Saxena wrote:
> +#define show_sdev_iostat_percpu(field) \
> +static ssize_t \
> +show_iostat_##field(struct device *dev, struct device_attribute *attr, \
> + char *buf) \
> +{ \
> + struct scsi_device *sdev = to_scsi_device(dev); \
> + unsigned long long count = percpu_counter_sum(&sdev->field); \
Why "unsigned long long" instead of "u64"? u64 values can be formatted
with "%llx" since "u64" is a synonym for "unsigned long long".
> + return snprintf(buf, 20, "0x%llx\n", count); \
> +} \
> +static DEVICE_ATTR(field, 0444, show_iostat_##field, NULL)
Why "snprintf(buf, 20, ...)"? New code should use sysfs_emit(buf, ...),
isn't it?
Otherwise this patch looks good to me.
Thanks,
Bart.
^ permalink raw reply
* Re: [PATCH] rbd: fix null-ptr-deref when device_add_disk() fails
From: Ilya Dryomov @ 2026-04-20 20:19 UTC (permalink / raw)
To: Dawei Feng
Cc: axboe, dongsheng.yang, mcgrof, ceph-devel, linux-block,
linux-kernel, jianhao.xu, stable, Zilin Guan
In-Reply-To: <20260419090348.1817082-1-dawei.feng@seu.edu.cn>
On Sun, Apr 19, 2026 at 11:05 AM Dawei Feng <dawei.feng@seu.edu.cn> wrote:
>
> do_rbd_add() publishes the device with device_add() before calling
> device_add_disk(). If device_add_disk() fails after device_add()
> succeeds, the error path calls rbd_free_disk() directly and then later
> falls through to rbd_dev_device_release(), which calls rbd_free_disk()
> again. This double teardown can leave blk-mq cleanup operating on
> invalid state and trigger a null-ptr-deref in
> __blk_mq_free_map_and_rqs(), reached from blk_mq_free_tag_set().
>
> Fix this by following the normal remove ordering: call device_del()
> before rbd_dev_device_release() when device_add_disk() fails after
> device_add(). That keeps the teardown sequence consistent and avoids
> re-entering disk cleanup through the wrong path.
>
> The bug was first flagged by an experimental analysis tool we are
> developing for kernel memory-management bugs while analyzing
> v6.13-rc1. The tool is still under development and is not yet publicly
> available.
>
> We reproduced the bug on v7.0 with a real Ceph backend and a QEMU x86_64
> guest booted with KASAN and CONFIG_FAILSLAB enabled. The reproducer
> confines failslab injections to the __add_disk() range and injects
> fail-nth while mapping an RBD image through
> /sys/bus/rbd/add_single_major.
>
> On the unpatched kernel, fail-nth=4 reliably triggered the fault:
>
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
> CPU: 0 UID: 0 PID: 273 Comm: bash Not tainted 7.0.0-01247-gd60bc1401583 #6 PREEMPT(lazy)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
> RIP: 0010:__blk_mq_free_map_and_rqs+0x8c/0x240
> Code: 00 00 48 8b 6b 60 41 89 f4 49 c1 e4 03 4c 01 e5 45 85 ed 0f 85 0a 01 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 e9 48 c1 e9 03 <80> 3c 01 00 0f 85 31 01 00 00 4c 8b 6d 00 4d 85 ed 0f 84 e2 00 00
> RSP: 0018:ff1100000ab0fac8 EFLAGS: 00000246
> RAX: dffffc0000000000 RBX: ff1100000c4806a0 RCX: 0000000000000000
> RDX: 0000000000000002 RSI: 0000000000000000 RDI: ff1100000c4806f4
> RBP: 0000000000000000 R08: 0000000000000001 R09: ffe21c000189001b
> R10: ff1100000c4800df R11: ff1100006cf37be0 R12: 0000000000000000
> R13: 0000000000000000 R14: ff1100000c480700 R15: ff1100000c480004
> FS: 00007f0fbe8fe740(0000) GS:ff110000e5851000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fe53473b2e0 CR3: 0000000012eef000 CR4: 00000000007516f0
> PKRU: 55555554
> Call Trace:
> <TASK>
> blk_mq_free_tag_set+0x77/0x460
> do_rbd_add+0x1446/0x2b80
> ? __pfx_do_rbd_add+0x10/0x10
> ? lock_acquire+0x18c/0x300
> ? find_held_lock+0x2b/0x80
> ? sysfs_file_kobj+0xb6/0x1b0
> ? __pfx_sysfs_kf_write+0x10/0x10
> kernfs_fop_write_iter+0x2f4/0x4a0
> vfs_write+0x98e/0x1000
> ? expand_files+0x51f/0x850
> ? __pfx_vfs_write+0x10/0x10
> ksys_write+0xf2/0x1d0
> ? __pfx_ksys_write+0x10/0x10
> do_syscall_64+0x115/0x690
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f0fbea15907
> Code: 10 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
> RSP: 002b:00007ffe22346ea8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000058 RCX: 00007f0fbea15907
> RDX: 0000000000000058 RSI: 0000563ace6c0ef0 RDI: 0000000000000001
> RBP: 0000563ace6c0ef0 R08: 0000563ace6c0ef0 R09: 6b6435726d694141
> R10: 5250337279762f78 R11: 0000000000000246 R12: 0000000000000058
> R13: 00007f0fbeb1c780 R14: ff1100000c480700 R15: ff1100000c480004
> </TASK>
>
> With this fix applied, rerunning the reproducer over fail-nth=1..256
> yields no KASAN reports.
>
> Fixes: 27c97abc30e2 ("rbd: add add_disk() error handling")
> Cc: stable@vger.kernel.org # 5.16+
> Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
> Signed-off-by: Dawei Feng <dawei.feng@seu.edu.cn>
> ---
> drivers/block/rbd.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
> index e7da06200c1e..d92730d8c342 100644
> --- a/drivers/block/rbd.c
> +++ b/drivers/block/rbd.c
> @@ -7165,7 +7165,7 @@ static ssize_t do_rbd_add(const char *buf, size_t count)
>
> rc = device_add_disk(&rbd_dev->dev, rbd_dev->disk, NULL);
> if (rc)
> - goto err_out_cleanup_disk;
> + goto err_out_device_del;
>
> spin_lock(&rbd_dev_list_lock);
> list_add_tail(&rbd_dev->node, &rbd_dev_list);
> @@ -7179,8 +7179,8 @@ static ssize_t do_rbd_add(const char *buf, size_t count)
> module_put(THIS_MODULE);
> return rc;
>
> -err_out_cleanup_disk:
> - rbd_free_disk(rbd_dev);
> +err_out_device_del:
> + device_del(&rbd_dev->dev);
> err_out_image_lock:
> rbd_dev_image_unlock(rbd_dev);
> rbd_dev_device_release(rbd_dev);
> --
> 2.34.1
>
Applied.
Thanks,
Ilya
^ permalink raw reply
* Re: [PATCH v3 0/6] block: fix integrity offset/length conversions
From: Martin K. Petersen @ 2026-04-21 2:08 UTC (permalink / raw)
To: Caleb Sander Mateos
Cc: Martin K. Petersen, Jens Axboe, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni, Anuj Gupta, linux-block, linux-nvme,
linux-scsi, target-devel, linux-kernel
In-Reply-To: <CADUfDZrwzUTi2TOj6M-+FtBK6u5evMsWSBqRDwJsLb8yLbOGvw@mail.gmail.com>
Hi Caleb!
> NVM Command Set specification 1.1 section 5.3.3 requires the reference
> tag to increment by 1 per logical block, so that seems to determine
> the increment unit:
SCSI allows PI to be interleaved at intervals smaller than the logical
block size. This was done for PI compatibility in mixed environments
with both 512[en] and 4Kn disks. Interleaving allows 8 bytes of PI per
512 bytes of data on devices using 4 KB logical blocks. That is the
reason why we use the term "integrity interval" instead of assuming
logical block size.
> The ref tag used for a particular block needs to be consistent. And
> since reftag(block N) can be computed as the reftag(M) + N - M if
> block N is accessed as part of an I/O that begins at block M, the
> function must be of the form reftag(block N) = N + c for some constant
> c. Thus, the ref tag seed needs to be computed in units of logical
> blocks (integrity intervals); no other unit (e.g. 512-byte sectors)
> works.
Whoever attaches the PI decides on the seed value. In the case of the
block layer it made sense to use block layer sector number since that
value is inevitably going to be the same for a future read.
Note that with MD, DM, and partitioning in the mix, the sector number
seen by whoever submits the I/O is going to be different from the LBAs
on the target devices which eventually receive the I/O. Nobody says
there is a computable constant offset. Think scattered LVM extent
allocations. Or RAID stripes placed at mismatched LBA offsets.
> To see the issue with the current approach, consider an example
> accessing LBA 1 on a device with a 4 KB block size. If the block is
> written as part of a write that begins at LBA 0, its ref tag in the
> generated PI will be 1 (sector 0 + 1 integrity interval). If it's
> later read by a read starting at LBA 1, its expected ref tag will be 8
> (sector 8 + 0 integrity intervals), and the auto-integrity code will
> fail the read due to a reftag mismatch.
Something is broken, then. Because the ref tag in the received PI should
have been remapped to start at 8 in that case.
> I agree, the seed doesn't need to match the final LBA, but it does
> need to be in *units* of logical blocks, plus some constant offset.
Your concept of "unit" still sends the wrong message. The seed is an
integer value used to initialize a counter or hardware register. The
seed only has meaning to whichever entity submits the I/O. To everything
else it is a value used for remapping ref tags from the I/O submitter's
point of view to whichever interpretation is mandated by the storage
hardware's PI format.
> With a ublk device. It should affect any block device that supports
> integrity and has a logical block size > 512.
It sounds like the seed value is set incorrectly for reads in your
configuration.
--
Martin K. Petersen
^ permalink raw reply
* Re: fix /dev/sg allocation failures register
From: Martin K. Petersen @ 2026-04-21 2:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Doug Gilbert, Martin K. Petersen,
Shin'ichiro Kawasaki, linux-block, linux-scsi
In-Reply-To: <20260415060813.807659-1-hch@lst.de>
Christoph,
> This series has two independent patches to fix this from different
> angles. The first one changes the completely pointless GFP_ATOMIC to
> use GFP_ATOMIC in sg. The other drops the reduction of the gfp mask in
> the bio allocator, so that atomic allocations of bios work as
> expected, even if they are a pretty bad idea as the bio submission
> actually needs a user context anyway. The only other uses of this
> seems to be the ocfs2 cluster managed.
Applied #1 to 7.1/scsi-staging, thanks!
#2 needs to go through block.
--
Martin K. Petersen
^ permalink raw reply
* Re: [LSF/MM/BPF ATTEND][LSF/MM/BPF TOPIC] : blktests: status, expansion plan for the storage stack test framework
From: Shin'ichiro Kawasaki @ 2026-04-21 6:05 UTC (permalink / raw)
To: Johannes Thumshirn
Cc: Haris Iqbal, Daniel Wagner, Chaitanya Kulkarni,
linux-block@vger.kernel.org, linux-scsi@vger.kernel.org,
linux-nvme@lists.infradead.org, lsf-pc@lists.linux-foundation.org,
Bart Van Assche, Hannes Reinecke, hch, Jens Axboe,
sagi@grimberg.me, tytso@mit.edu, Christian Brauner,
Martin K. Petersen, linux-fsdevel@vger.kernel.org,
Javier González, willy@infradead.org, Jan Kara,
amir73il@gmail.com, vbabka@suse.cz, Damien Le Moal
In-Reply-To: <ae47ef06-3f66-4aab-b4ab-f3ae2b634f87@wdc.com>
[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]
On Feb 23, 2026 / 07:44, Johannes Thumshirn wrote:
> On 2/15/26 10:18 PM, Haris Iqbal wrote:
> >> From my view, blktests keep on finding kernel bugs. I think it demonstrates the
> >> value of this community effort, and I'm happy about it. Said that, I find what
> >> blktests can improve more, of course. Here I share the list of improvement
> >> opportunities from my view point (I already mentioned the first three items).
> > A possible feature for blktest could be integration with something
> > like virtme-ng.
> > Running on VM can be versatile and fast. The run can be made parallel
> > too, by spawning multiple VMs simultaneously.
>
> This is actually rather trivial to solve I have some pre-made things for
> fstests and that can be adopted for blktests as well:
>
> vng \
> --user=root -v --name vng-tcmu-runner \
> -a loglevel=3 \
> --run $KDIR \
> --cpus=8 --memory=8G \
> --exec "~johannes/src/ci/run-fstests.sh" \
> --qemu-opts="-device virtio-scsi,id=scsi0 -drive
> file=/dev/sda,format=raw,if=none,id=zbc0 -device
> scsi-block,bus=scsi0.0,drive=zbc0" \
> --qemu-opts="-device virtio-scsi,id=scsi1 -drive
> file=/dev/sdb,format=raw,if=none,id=zbc1 -device
> scsi-block,bus=scsi1.0,drive=zbc1"
>
> and run-fstests.sh is:
>
> #!/bin/sh
> # SPDX-License-Identifier: GPL-2.0
>
> DIR="/tmp/"
> MKFS="mkfs.btrfs -f"
> FSTESTS_DIR="/home/johannes/src/fstests"
> HOSTCONF="$FSTESTS_DIR/configs/$(hostname -s)"
> TESTDEV="$(grep TEST_DEV $HOSTCONF | cut -d '=' -f 2)"
>
> mkdir -p $DIR/{test,scratch,results}
> $MKFS $TESTDEV
>
> cd $FSTESTS_DIR
> ./check -x raid
>
> I'm not sure it'll make sense to include this into blktests other than
> maybe providing an example in the README.
I guess the example can be added in contrib/. This idea interested me, so I did
some quick scripting and created the patch attached. I did some blktests runs
with virtme-ng, and found that:
- virtme-ng allows to skip kernel installation step. Fast and useful as Haris
pointed out.
- systemd does not look working well with virtme-ng, even when I specify the
--systemd option. This will be a condition difference from normal
blktests runs.
I hope that the patch will help discussion at the conference.
[-- Attachment #2: 0001-contrib-add-scripts-to-run-blktests-with-virtme-ng.patch --]
[-- Type: text/plain, Size: 5517 bytes --]
From 4a310c98f674e65218fb7e33e3b176a85d897333 Mon Sep 17 00:00:00 2001
From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Date: Tue, 21 Apr 2026 14:26:01 +0900
Subject: [PATCH blktests RFC] contrib: add scripts to run blktests with
virtme-ng
It takes rather long time to run blktests repeatedly for debug target
kernels because of the time to install the test target kernel to the
test target system. To reduce the turn around time, virtme-ng [1] is
useful. Add helper scripts to run blktests using virtme-ng per
suggestion by Johaness [2].
After building a kernel in ~/linux, the command lines below will run
blktests on the built kernel. With this, kernel installation step is no
longer required.
$ KDIR=~/linux contrib/run-vng loop/010
virtme: waiting for virtiofsd to start
virtme: use 'microvm' QEMU architecture
early console in setup code
Probing EDD (edd=off to disable)... ok
[ 0.000000][ T0] Linux version 7.0.0 (shin@shinmob) (gcc (GCC) 15.2.1 20260123 (Red Hat 15.2.1-7), GNU ld version 2.45.1-4.fc43) #22 SMP PREEMPT_DYNAMIC Mon Apr 13 15:44:07 JST 2026
[ 0.000000][ T0] Command line: virtme_hostname=vng-blktests-runner nr_open=2147483584 virtme_link_mods=/home/shin/linux/.virtme_mods/lib/modules/0.0.0 virtme_initmount0=tmp virtme_rw_overlay0=/etc virtme_rw_overlay1=/lib virtme_rw_overlay2=/home virtme_rw_overlay3=/opt virtme_rw_overlay4=/srv virtme_rw_overlay5=/usr virtme_rw_overlay6=/var virtme_rw_overlay7=/tmp virtme_console=ttyS0 console=ttyS0 earlyprintk=serial,ttyS0,115200 panic=-1 virtme.exec=`L2hvbWUvc2hpbi9CbGt0ZXN0cy9ibGt0ZXN0cy9jb250cmliL3J1bi1ibGt0ZXN0cy1xdWljayBsb29wLzAwMQ==` virtme.ssh virtme_ssh_channel=vsock virtme_ssh_cache=/home/shin/.cache/virtme-ng/.ssh virtme_chdir=home/shin/Blktests/blktests/contrib debug loglevel=3 init=/usr/lib/python3.14/site-packages/virtme/guest/bin/virtme-ng-init
[ 0.000000][ T0] x86/split lock detection: #DB: warning on user-space bus_locks
[ 0.000000][ T0] BIOS-provided physical RAM map:
[ 0.000000][ T0] BIOS-e820: [mem 0x0000000000000000-0x000000000009fbff] System RAM
[ 0.000000][ T0] BIOS-e820: [mem 0x000000000009fc00-0x000000000009ffff] device reserved
[ 0.000000][ T0] BIOS-e820: [gap 0x00000000000a0000-0x00000000000effff]
[ 0.000000][ T0] BIOS-e820: [mem 0x00000000000f0000-0x00000000000fffff] device reserved
[ 0.000000][ T0] BIOS-e820: [mem 0x0000000000100000-0x00000000bfffefff] System RAM
[ 0.000000][ T0] BIOS-e820: [mem 0x00000000bffff000-0x00000000bfffffff] device reserved
[ 0.000000][ T0] BIOS-e820: [gap 0x00000000c0000000-0x00000000feffbfff]
[ 0.000000][ T0] BIOS-e820: [mem 0x00000000feffc000-0x00000000feffffff] device reserved
[ 0.000000][ T0] BIOS-e820: [gap 0x00000000ff000000-0x00000000fffbffff]
[ 0.000000][ T0] BIOS-e820: [mem 0x00000000fffc0000-0x00000000ffffffff] device reserved
[ 0.000000][ T0] BIOS-e820: [mem 0x0000000100000000-0x000000013fffffff] System RAM
[ 0.000000][ T0] printk: legacy bootconsole [earlyser0] enabled
Poking KASLR using RDRAND RDTSC...
Clean up /tmp/results ...
loop/001 (scan loop device partitions)
loop/001 (scan loop device partitions) [passed]
runtime ... 14.610s
Run logs:
[ 90.927921][ T1] reboot: Power down
When I ran the script on my Fedora 43 testnode for some test groups, I
observed hangs at nvme/010 and nvme/012. Also I observed a failure at
loop/010. Some fixes will be required in blktests and kernel to run the
new script in stable manner.
Of note is that the script does not support test target block device
pass-through yet.
[1] https://github.com/arighi/virtme-ng
[2] https://lore.kernel.org/linux-block/ae47ef06-3f66-4aab-b4ab-f3ae2b634f87@wdc.com/
Suggested-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
contrib/run-blktests-quick | 27 +++++++++++++++++++++++++++
contrib/run-vng | 18 ++++++++++++++++++
2 files changed, 45 insertions(+)
create mode 100755 contrib/run-blktests-quick
create mode 100755 contrib/run-vng
diff --git a/contrib/run-blktests-quick b/contrib/run-blktests-quick
new file mode 100755
index 0000000..9324c38
--- /dev/null
+++ b/contrib/run-blktests-quick
@@ -0,0 +1,27 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 Western Digital Corporation or its affiliates.
+
+declare DIR LOGDIR BLKTESTS_DIR
+
+DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
+BLKTESTS_DIR="${DIR}/../"
+
+LOGDIR="/tmp/results"
+
+if [[ -e $LOGDIR ]]; then
+ echo "Clean up ${LOGDIR} ..."
+ rm -rf $LOGDIR
+fi
+mkdir -p "$LOGDIR"
+
+cd "$BLKTESTS_DIR" || exit
+
+cat > config << EOF
+QUICK_RUN=1
+TIMEOUT=10
+EOF
+
+./check -o "$LOGDIR" "$@"
+echo "Run logs: ${LOGIDR}"
+
diff --git a/contrib/run-vng b/contrib/run-vng
new file mode 100755
index 0000000..9aa4b91
--- /dev/null
+++ b/contrib/run-vng
@@ -0,0 +1,18 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 Western Digital Corporation or its affiliates.
+
+declare DIR
+declare -a BLKTESTS_OPTS
+DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 && pwd)"
+
+if (($#)); then
+ BLKTESTS_OPTS=("$@")
+else
+ BLKTESTS_OPTS=(block nvme loop scsi)
+fi
+
+vng \
+ --user=root --verbose --name vng-blktests-runner --cpus=4 --memory=4G \
+ --append loglevel=3 --run "$KDIR" --ssh \
+ --rwdir=/tmp --exec "$DIR/run-blktests-quick ${BLKTESTS_OPTS[*]}"
--
2.53.0
^ permalink raw reply related
* Re: [LSF/MM/BPF ATTEND][LSF/MM/BPF TOPIC] : blktests: status, expansion plan for the storage stack test framework
From: Shin'ichiro Kawasaki @ 2026-04-21 6:19 UTC (permalink / raw)
To: Nilay Shroff
Cc: Daniel Wagner, Chaitanya Kulkarni, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org,
lsf-pc@lists.linux-foundation.org, Bart Van Assche,
Hannes Reinecke, hch, Jens Axboe, sagi@grimberg.me, tytso@mit.edu,
Johannes Thumshirn, Christian Brauner, Martin K. Petersen,
linux-fsdevel@vger.kernel.org, Javier González,
willy@infradead.org, Jan Kara, amir73il@gmail.com, vbabka@suse.cz,
Damien Le Moal
In-Reply-To: <901f4daf-3226-416f-8741-dd15573e736b@linux.ibm.com>
On Feb 16, 2026 / 00:08, Nilay Shroff wrote:
>
>
> On 2/13/26 4:53 PM, Shinichiro Kawasaki wrote:
> > On Feb 12, 2026 / 08:52, Daniel Wagner wrote:
> >> On Wed, Feb 11, 2026 at 08:35:30PM +0000, Chaitanya Kulkarni wrote:
> >>> For the storage track at LSFMMBPF2026, I propose a session dedicated to
> >>> blktests to discuss expansion plan and CI integration progress.
> >>
> >> Thanks for proposing this topic.
> >
> > Chaitanya, my thank also goes to you.
> >
> Yes thanks for proposing this!
>
> >> Just a few random topics which come to mind we could discuss:
> >>
> >> - blktests has gain a bit of traction and some folks run on regular
> >> basis these tests. Can we gather feedback from them, what is working
> >> good, what is not? Are there feature wishes?
> >
> > Good topic, I also would like to hear about it.
> >
> One improvement I’d like to highlight is related to how blktests are executed
> today. So far, we’ve been running blktests serially, but if it's possible to
> run tests in parallel to improve test turnaround time and make large-scale or
> CI-based testing more efficient? For instance, adding parallel_safe Tags: Marking tests
> that don't modify global kernel state so they can be safely offloaded to parallel
> workers. Marking parallel_safe tags would allow the runner to distinguish:
>
> Safe Tests: Tests that only perform I/O on a specific, non-shared device or
> check static kernel parameters.
>
> Unsafe Tests: Tests that reload kernel modules, modify global /sys or /proc entries,
> or require exclusive access to specific hardware addresses.
>
> Yes adding parallel execution support shall require framework/design changes.
Hi Nilay, thanks for the idea. I understand that shorter test time will make CI
cycles faster and improve the development efficiency.
Said that, the safe/unsafe testing idea may not be enough. I think majority of
test case does kernel module set up using null_blk, scsi_debug, or nvme target
drivers. Then I foresee the majority of the test cases will be "unsafe", and
cannot be run in parallel.
Also, parallel runs on single system will affect dmesg or kmemleak checking.
We cannot tell which run caused a dmesg message or a memory leak.
For the runtime reduction by parallel runs, I guess blktests run on VMs might be
the good approach as Haris pointed out. Anyway, this topic will need more
discussion.
[...]
> > 4. Long standing failures make test result reports dirty
> > - I feel lockdep WARNs are tend to be left unfixed rather long period.
> > How can we gather effort to fix them?
>
> I agree regarding lockdep; recently we did see quite a few lockdep splats.
> That said, I believe the number has dropped significantly and only a small
> set remains. From what I can tell, most of the outstanding lockdep issues
> are related to fs-reclaim paths recursing into the block layer while the
> queue is frozen. We should be able to resolve most of these soon, or at
> least before the conference. If anything is still outstanding after that,
> we can discuss it during the conference and work toward addressing it as
> quickly as possible.
Taking this chance, I'd like to express my appreciation for the effort to
resolve the lockdep issues. It is great that a number of lockdeps are already
fixed. Said that, two lockdep issues are still observed with v7.0 kernel at
nvme/005 and nbd/002 [1]. I would like to gather attentions to the failures.
[1] https://lore.kernel.org/linux-block/ynmi72x5wt5ooljjafebhcarit3pvu6axkslqenikb2p5txe57@ldytqa2t4i2x/
^ permalink raw reply
* Re: [LSF/MM/BPF ATTEND][LSF/MM/BPF TOPIC] : blktests: status, expansion plan for the storage stack test framework
From: Shin'ichiro Kawasaki @ 2026-04-21 6:37 UTC (permalink / raw)
To: Daniel Wagner
Cc: Chaitanya Kulkarni, linux-block@vger.kernel.org,
linux-scsi@vger.kernel.org, linux-nvme@lists.infradead.org,
lsf-pc@lists.linux-foundation.org, Bart Van Assche,
Hannes Reinecke, hch, Jens Axboe, sagi@grimberg.me, tytso@mit.edu,
Johannes Thumshirn, Christian Brauner, Martin K. Petersen,
linux-fsdevel@vger.kernel.org, Javier González,
willy@infradead.org, Jan Kara, amir73il@gmail.com, vbabka@suse.cz,
Damien Le Moal
In-Reply-To: <aY77ogf5nATlJUg_@shinmob>
On Feb 13, 2026 / 11:23, Shinichiro Kawasaki wrote:
[...]
> Here I share the list of improvement
> opportunities from my view point (I already mentioned the first three items).
>
> 1. We can have more CI infra to make the most of blktests
> 2. We can add config examples to help new users
> 3. We can measure code coverage to identify missing test areas
> 4. Long standing failures make test result reports dirty
> - I feel lockdep WARNs are tend to be left unfixed rather long period.
> How can we gather effort to fix them?
> 5. We can refactor and clean up blktests framework for ease of maintainance
> (e.g. trap handling)
> 6. Some users run blktests with built-in kernel modules, which makes a number
> of test cases skipped. We can add more built-in kernel modules support to
> expand test coverage for such use case.
I read through this e-mail thread again, and revised the list of improvement
opportunities. Hope this list helps the discussion at the session.
1. configuration example, documentation, or tool for ease of new users
2. blktests framework capabilitly
- more bug detection capability (such as kmemleak)
- code coverage measurement per test case
- VM integration (virtme-ng? valid use case?)
- parallel execution for shorter run
- built-in module testing
- multi device testing
3. blktests CI
4. longstanding failures
- nvme/005 tcp transport, nbd/002
5. test area coverage expansion (which area do we lack tests?)
- code coverage information as the guide
- new category candidates (rnbd?)
6. blktests framework refactoring
- trap handling
One thing I added was "multi device testing". Most of the test cases use
single test device for single test run, while md/003 and bcache/001 require
multiple devices for single test run. There was a discussion how to specify
multiple devices in the config file [*].
[*] https://lore.kernel.org/linux-block/aXHbPWCf5SfycpBX@shinmob/
^ permalink raw reply
* Re: [PATCH v2 3/3] scsi: use percpu counters for iorequest_cnt and iodone_cnt
From: John Garry @ 2026-04-21 8:45 UTC (permalink / raw)
To: Sumit Saxena, martin.petersen, axboe
Cc: linux-scsi, linux-block, mpi3mr-linuxdrv.pdl, Bart Van Assche
In-Reply-To: <20260420113846.1401374-4-sumit.saxena@broadcom.com>
On 20/04/2026 12:38, Sumit Saxena wrote:
> iorequest_cnt and iodone_cnt are updated on every command dispatch and
> completion, often from different CPUs on high queue depth workloads.
> Using adjacent atomic_t fields caused cache line contention between the
> submission and completion paths.
>
> Represent these statistics with struct percpu_counter so increments are
> mostly local to each CPU, avoiding false sharing without growing
> struct scsi_device further for cache-line padding.
>
> Suggested-by: Bart Van Assche <bvanassche@acm.org>
> Signed-off-by: Sumit Saxena <sumit.saxena@broadcom.com>
> ---
> drivers/scsi/scsi_error.c | 2 +-
> drivers/scsi/scsi_lib.c | 8 ++++----
> drivers/scsi/scsi_scan.c | 9 +++++++++
> drivers/scsi/scsi_sysfs.c | 27 +++++++++++++++++++++++----
> include/scsi/scsi_device.h | 5 +++--
> 5 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 147127fb4db9..c7424ce92f3e 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -370,7 +370,7 @@ enum blk_eh_timer_return scsi_timeout(struct request *req)
> */
> if (test_and_set_bit(SCMD_STATE_COMPLETE, &scmd->state))
> return BLK_EH_DONE;
> - atomic_inc(&scmd->device->iodone_cnt);
> + percpu_counter_inc(&scmd->device->iodone_cnt);
> if (scsi_abort_command(scmd) != SUCCESS) {
> set_host_byte(scmd, DID_TIME_OUT);
> scsi_eh_scmd_add(scmd);
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index 6e8c7a42603e..0b05cb63f630 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1554,7 +1554,7 @@ static void scsi_complete(struct request *rq)
>
> INIT_LIST_HEAD(&cmd->eh_entry);
>
> - atomic_inc(&cmd->device->iodone_cnt);
> + percpu_counter_inc(&cmd->device->iodone_cnt);
> if (cmd->result)
> atomic_inc(&cmd->device->ioerr_cnt);
>
> @@ -1592,7 +1592,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> struct Scsi_Host *host = cmd->device->host;
> int rtn = 0;
>
> - atomic_inc(&cmd->device->iorequest_cnt);
> + percpu_counter_inc(&cmd->device->iorequest_cnt);
>
> /* check if the device is still usable */
> if (unlikely(cmd->device->sdev_state == SDEV_DEL)) {
> @@ -1614,7 +1614,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> */
> SCSI_LOG_MLQUEUE(3, scmd_printk(KERN_INFO, cmd,
> "queuecommand : device blocked\n"));
> - atomic_dec(&cmd->device->iorequest_cnt);
> + percpu_counter_dec(&cmd->device->iorequest_cnt);
> return SCSI_MLQUEUE_DEVICE_BUSY;
> }
>
> @@ -1647,7 +1647,7 @@ static enum scsi_qc_status scsi_dispatch_cmd(struct scsi_cmnd *cmd)
> trace_scsi_dispatch_cmd_start(cmd);
> rtn = host->hostt->queuecommand(host, cmd);
> if (rtn) {
> - atomic_dec(&cmd->device->iorequest_cnt);
> + percpu_counter_dec(&cmd->device->iorequest_cnt);
> trace_scsi_dispatch_cmd_error(cmd, rtn);
> if (rtn != SCSI_MLQUEUE_DEVICE_BUSY &&
> rtn != SCSI_MLQUEUE_TARGET_BUSY)
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 9749a8dbe964..0b4fa89149af 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -351,6 +351,15 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
>
> scsi_sysfs_device_initialize(sdev);
>
> + ret = percpu_counter_init(&sdev->iorequest_cnt, 0, GFP_KERNEL);
> + if (ret)
> + goto out_device_destroy;
> + ret = percpu_counter_init(&sdev->iodone_cnt, 0, GFP_KERNEL);
> + if (ret) {
> + percpu_counter_destroy(&sdev->iorequest_cnt);
> + goto out_device_destroy;
> + }
it could be neater to have:
if (percpu_counter_init(&sdev->iorequest_cnt, 0, GFP_KERNEL) ||
percpu_counter_init(&sdev->iodone_cnt, 0, GFP_KERNEL)) {
err = some err;
goto out_device_destroy;
}
> +
> if (scsi_device_is_pseudo_dev(sdev))
> return sdev;
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index dfc3559e7e04..1f5b2dc156a8 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -516,6 +516,10 @@ static void scsi_device_dev_release(struct device *dev)
> if (vpd_pgb7)
> kfree_rcu(vpd_pgb7, rcu);
> kfree(sdev->inquiry);
> + if (percpu_counter_initialized(&sdev->iodone_cnt))
> + percpu_counter_destroy(&sdev->iodone_cnt);
> + if (percpu_counter_initialized(&sdev->iorequest_cnt))
> + percpu_counter_destroy(&sdev->iorequest_cnt);
Maybe I am wrong, but doesn't percpu_counter_destroy() handle the case
of the percpu counter not being initialized? In other words, do we need
the percpu_counter_initialized() checks?
> kfree(sdev);
>
> if (parent)
> @@ -936,11 +940,26 @@ static ssize_t
> show_iostat_counterbits(struct device *dev, struct device_attribute *attr,
> char *buf)
> {
> - return snprintf(buf, 20, "%d\n", (int)sizeof(atomic_t) * 8);
> + /*
> + * iorequest_cnt and iodone_cnt are per-CPU sums (s64); ioerr_cnt and
> + * iotmo_cnt remain atomic_t. Report the widest counter for tools.
> + */
> + return snprintf(buf, 20, "%zu\n", sizeof(s64) * 8);
> }
>
> static DEVICE_ATTR(iocounterbits, S_IRUGO, show_iostat_counterbits, NULL);
>
> +#define show_sdev_iostat_percpu(field) \
> +static ssize_t \
> +show_iostat_##field(struct device *dev, struct device_attribute *attr, \
> + char *buf) \
> +{ \
> + struct scsi_device *sdev = to_scsi_device(dev); \
> + unsigned long long count = percpu_counter_sum(&sdev->field); \
> + return snprintf(buf, 20, "0x%llx\n", count); \
> +} \
> +static DEVICE_ATTR(field, 0444, show_iostat_##field, NULL)
> +
> #define show_sdev_iostat(field) \
> static ssize_t \
> show_iostat_##field(struct device *dev, struct device_attribute *attr, \
> @@ -950,10 +969,10 @@ show_iostat_##field(struct device *dev, struct device_attribute *attr, \
> unsigned long long count = atomic_read(&sdev->field); \
> return snprintf(buf, 20, "0x%llx\n", count); \
> } \
> -static DEVICE_ATTR(field, S_IRUGO, show_iostat_##field, NULL)
> +static DEVICE_ATTR(field, 0444, show_iostat_##field, NULL)
>
> -show_sdev_iostat(iorequest_cnt);
> -show_sdev_iostat(iodone_cnt);
> +show_sdev_iostat_percpu(iorequest_cnt);
> +show_sdev_iostat_percpu(iodone_cnt);
> show_sdev_iostat(ioerr_cnt);
> show_sdev_iostat(iotmo_cnt);
>
> diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
> index 9c2a7bbe5891..ad80b500ced9 100644
> --- a/include/scsi/scsi_device.h
> +++ b/include/scsi/scsi_device.h
> @@ -8,6 +8,7 @@
> #include <linux/blk-mq.h>
> #include <scsi/scsi.h>
> #include <linux/atomic.h>
> +#include <linux/percpu_counter.h>
> #include <linux/sbitmap.h>
>
> struct bsg_device;
> @@ -271,8 +272,8 @@ struct scsi_device {
> unsigned int max_device_blocked; /* what device_blocked counts down from */
> #define SCSI_DEFAULT_DEVICE_BLOCKED 3
>
> - atomic_t iorequest_cnt;
> - atomic_t iodone_cnt;
> + struct percpu_counter iorequest_cnt;
> + struct percpu_counter iodone_cnt;
> atomic_t ioerr_cnt;
> atomic_t iotmo_cnt;
>
Would it be simpler to make ioerr_cnt and iotmo_cnt also as
percpu_counter? We could then drop some sysfs code for handling atomic_t
counter.
I noticed that there is a percpu_counter_init_many() - maybe we could
use that, and index into the array of counters. Note that I am not
fimilar with that API, so it may not be a good idea.
^ permalink raw reply
* Re: [syzbot] [block?] general protection fault in lo_rw_aio
From: Tetsuo Handa @ 2026-04-21 11:05 UTC (permalink / raw)
To: syzbot, axboe, linux-block, syzkaller-bugs
In-Reply-To: <69e2ca14.a00a0220.1bd0ca.0031.GAE@google.com>
I confirmed that this NULL pointer dereference is triggered by
__loop_clr_fd() clearing lo->lo_backing_file before aio request
is processed. A debug printk() patch at
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/block/loop.c?h=next-20260420&id=7454b88731fd320e7dbfbf1ea72f25705c3befc0
generated the following output.
file == NULL
Last __loop_clr_fd() call was 5 jiffies ago by udevd (16676)
lo_release+0x35b/0x9d0
blkdev_release+0x15/0x20 block/fops.c:705
__fput+0x461/0xa70 fs/file_table.c:510
fput_close_sync+0x11f/0x240 fs/file_table.c:615
__do_sys_close fs/open.c:1507 [inline]
__se_sys_close fs/open.c:1492 [inline]
__x64_sys_close+0x7e/0x110 fs/open.c:1492
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000014: 0000 [#1] SMP KASAN PTI
KASAN: null-ptr-deref in range [0x00000000000000a0-0x00000000000000a7]
CPU: 0 UID: 0 PID: 180 Comm: kworker/u8:6 Tainted: G L syzkaller #0 PREEMPT_{RT,(full)}
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/18/2026
Workqueue: loop4 loop_workfn
RIP: 0010:file_inode include/linux/fs.h:1353 [inline]
RIP: 0010:kiocb_start_write include/linux/fs.h:2756 [inline]
RIP: 0010:lo_rw_aio+0xb88/0x1190 drivers/block/loop.c:429
Code: 1e d8 34 fc 45 85 e4 0f 84 d8 00 00 00 48 8b 44 24 58 48 8d 98 a0 00 00 00 48 89 d8 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <80> 3c 08 00 74 08 48 89 df e8 8a 4c 9e fc 48 8b 1b 48 83 c3 28 49
RSP: 0018:ffffc90003907660 EFLAGS: 00010206
RAX: 0000000000000014 RBX: 00000000000000a0 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: ffffc900039077d0 R08: 0000000000000000 R09: 0000000000000000
R10: ffffc90003907723 R11: fffff52000720ee6 R12: 0000000000000001
R13: ffff888026ca2c90 R14: 0000000000000000 R15: dffffc0000000000
FS: 0000000000000000(0000) GS:ffff888125eb7000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055555b199a28 CR3: 000000003a2c0000 CR4: 00000000003526f0
Call Trace:
<TASK>
do_req_filebacked drivers/block/loop.c:461 [inline]
loop_handle_cmd drivers/block/loop.c:1960 [inline]
loop_process_work+0x647/0x1560 drivers/block/loop.c:1995
process_one_work+0x9a3/0x1710 kernel/workqueue.c:3312
process_scheduled_works kernel/workqueue.c:3403 [inline]
worker_thread+0xba8/0x11e0 kernel/workqueue.c:3489
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:file_inode include/linux/fs.h:1353 [inline]
RIP: 0010:kiocb_start_write include/linux/fs.h:2756 [inline]
RIP: 0010:lo_rw_aio+0xb88/0x1190 drivers/block/loop.c:429
Code: 1e d8 34 fc 45 85 e4 0f 84 d8 00 00 00 48 8b 44 24 58 48 8d 98 a0 00 00 00 48 89 d8 48 c1 e8 03 48 b9 00 00 00 00 00 fc ff df <80> 3c 08 00 74 08 48 89 df e8 8a 4c 9e fc 48 8b 1b 48 83 c3 28 49
RSP: 0018:ffffc90003907660 EFLAGS: 00010206
RAX: 0000000000000014 RBX: 00000000000000a0 RCX: dffffc0000000000
RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000000
RBP: ffffc900039077d0 R08: 0000000000000000 R09: 0000000000000000
R10: ffffc90003907723 R11: fffff52000720ee6 R12: 0000000000000001
R13: ffff888026ca2c90 R14: 0000000000000000 R15: dffffc0000000000
FS: 0000000000000000(0000) GS:ffff888125eb7000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000055555b199a28 CR3: 000000003a2c0000 CR4: 00000000003526f0
Either fput() is called without corresponding fget(), or some recent changes
has changed timing of start processing aio request?
Since this problem started after the merge window opened, the culprit commit
might be in between commit a028739a4330 ("Merge tag 'block-7.0-20260305' of
git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux") and
commit 7fe6ac157b7e ("Merge tag 'for-7.1/block-20260411' of
git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux").
Also, syzbot was not testing changes in linux-next since next-20260403,
and found this problem in next-20260413.
^ permalink raw reply
* [RFC PATCH v2 0/4] mm/zsmalloc: reduce zs_free() latency on swap release path
From: Wenchao Hao @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Nhat Pham, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm
Cc: Barry Song, Xueyuan Chen, Wenchao Hao
Swap freeing can be expensive when unmapping a VMA containing
many swap entries. This has been reported to significantly
delay memory reclamation during Android's low-memory killing,
especially when multiple processes are terminated to free
memory, with slot_free() accounting for more than 80% of
the total cost of freeing swap entries.
Two earlier attempts by Lei and Zhiguo added a new thread in the mm core
to asynchronously collect and free swap entries [1][2], but the
design itself is fairly complex.
When anon folios and swap entries are mixed within a
process, reclaiming anon folios from killed processes
helps return memory to the system as quickly as possible,
so that newly launched applications can satisfy their
memory demands. It is not ideal for swap freeing to block
anon folio freeing. On the other hand, swap freeing can
still return memory to the system, although at a slower
rate due to memory compression.
Therefore, we introduce a GC worker to allow anon
folio freeing and slot_free to run in parallel, since
slot_free is performed asynchronously, maximizing the rate at
which memory is returned to the system.
This series takes two complementary approaches to reduce zs_free()
latency:
- Shrink zs_free() class->lock critical section by moving zspage
freeing outside the lock.
- Defer zs_free() to a workqueue via zs_free_deferred(), benefiting
both zram and zswap.
The deferred free approach builds on Barry Song's earlier RFC [1] with
changes based on community feedback: optimization moved to zsmalloc
layer instead of zram; fixed array storing handles (not indices) with
O(1) enqueue to avoid memory allocation on the exit path and data
consistency issues on slot reuse; size-based capacity scaling with
PAGE_SIZE.
Xueyuan's test on RK3588 with Barry's RFC v1 [3] shows that unmapping
a 256MB swap-filled VMA becomes 3.4x faster when pinning tasks to CPU2,
reducing the execution time from 63,102,982 ns to 18,570,726 ns.
A positive side effect is that async GC also slightly improves
do_swap_page() performance, as it no longer has to wait for
slot_free() to complete.
Xueyuan's test with Barry's RFC v1 [3] shows that swapping in 256MB of
data (each page filled with repeating patterns such as "1024 one",
"1024 two", "1024 three", and "1024 four") reduces execution time from
1,358,133,886 ns to 1,104,315,986 ns, achieving a 1.22x speedup.
[1] https://lore.kernel.org/all/20240805153639.1057-1-justinjiang@vivo.com/
[2] https://lore.kernel.org/all/20250909065349.574894-1-liulei.rjpt@vivo.com/
[3] https://lore.kernel.org/linux-mm/20260412060450.15813-1-baohua@kernel.org/
Xueyuan Chen (1):
mm:zsmalloc: drop class lock before freeing zspage
Barry Song (Xiaomi) (1):
zram: defer zs_free() in swap slot free notification path
Wenchao Hao (2):
mm/zsmalloc: introduce zs_free_deferred() for async handle freeing
mm/zswap: defer zs_free() in zswap_invalidate() path
drivers/block/zram/zram_drv.c | 37 ++++++---
include/linux/zsmalloc.h | 2 +
mm/zsmalloc.c | 141 ++++++++++++++++++++++++++++++++--
mm/zswap.c | 16 ++++-
4 files changed, 177 insertions(+), 19 deletions(-)
--
2.34.1
^ permalink raw reply
* [RFC PATCH v2 1/4] mm:zsmalloc: drop class lock before freeing zspage
From: Wenchao Hao @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Nhat Pham, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm
Cc: Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <20260421121616.3298845-1-haowenchao@xiaomi.com>
From: Xueyuan Chen <xueyuan.chen21@gmail.com>
Currently in zs_free(), the class->lock is held until the zspage is
completely freed and the counters are updated. However, freeing pages back
to the buddy allocator requires acquiring the zone lock.
Under heavy memory pressure, zone lock contention can be severe. When this
happens, the CPU holding the class->lock will stall waiting for the zone
lock, thereby blocking all other CPUs attempting to acquire the same
class->lock.
This patch shrinks the critical section of the class->lock to reduce lock
contention. By moving the actual page freeing process outside the
class->lock, we can improve the concurrency performance of zs_free().
Testing on the RADXA O6 platform shows that with 12 CPUs concurrently
performing zs_free() operations, the execution time is reduced by 20%.
Signed-off-by: Xueyuan Chen <xueyuan.chen21@gmail.com>
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
mm/zsmalloc.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 63128ddb7959..40687c8a7469 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -801,13 +801,10 @@ static int trylock_zspage(struct zspage *zspage)
return 0;
}
-static void __free_zspage(struct zs_pool *pool, struct size_class *class,
- struct zspage *zspage)
+static inline void __free_zspage_lockless(struct zs_pool *pool, struct zspage *zspage)
{
struct zpdesc *zpdesc, *next;
- assert_spin_locked(&class->lock);
-
VM_BUG_ON(get_zspage_inuse(zspage));
VM_BUG_ON(zspage->fullness != ZS_INUSE_RATIO_0);
@@ -823,7 +820,13 @@ static void __free_zspage(struct zs_pool *pool, struct size_class *class,
} while (zpdesc != NULL);
cache_free_zspage(zspage);
+}
+static void __free_zspage(struct zs_pool *pool, struct size_class *class,
+ struct zspage *zspage)
+{
+ assert_spin_locked(&class->lock);
+ __free_zspage_lockless(pool, zspage);
class_stat_sub(class, ZS_OBJS_ALLOCATED, class->objs_per_zspage);
atomic_long_sub(class->pages_per_zspage, &pool->pages_allocated);
}
@@ -1388,6 +1391,7 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
unsigned long obj;
struct size_class *class;
int fullness;
+ struct zspage *zspage_to_free = NULL;
if (IS_ERR_OR_NULL((void *)handle))
return;
@@ -1408,10 +1412,22 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
obj_free(class->size, obj);
fullness = fix_fullness_group(class, zspage);
- if (fullness == ZS_INUSE_RATIO_0)
- free_zspage(pool, class, zspage);
+ if (fullness == ZS_INUSE_RATIO_0) {
+ if (trylock_zspage(zspage)) {
+ remove_zspage(class, zspage);
+ class_stat_sub(class, ZS_OBJS_ALLOCATED,
+ class->objs_per_zspage);
+ zspage_to_free = zspage;
+ } else
+ kick_deferred_free(pool);
+ }
spin_unlock(&class->lock);
+
+ if (likely(zspage_to_free)) {
+ __free_zspage_lockless(pool, zspage_to_free);
+ atomic_long_sub(class->pages_per_zspage, &pool->pages_allocated);
+ }
cache_free_handle(handle);
}
EXPORT_SYMBOL_GPL(zs_free);
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v2 2/4] mm/zsmalloc: introduce zs_free_deferred() for async handle freeing
From: Wenchao Hao @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Nhat Pham, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm
Cc: Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <20260421121616.3298845-1-haowenchao@xiaomi.com>
zs_free() is expensive due to internal locking (pool->lock, class->lock)
and potential zspage freeing. On the process exit path, the slow
zs_free() blocks memory reclamation, delaying overall memory release.
This has been reported to significantly impact Android low-memory
killing where slot_free() accounts for over 80% of the total swap
entry freeing cost.
Introduce zs_free_deferred() which queues handles into a fixed-size
per-pool array for later processing by a workqueue. This allows callers
to defer the expensive zs_free() and return quickly, so the process
exit path can release memory faster. The array capacity is derived from
a 128MB uncompressed data budget (128MB >> PAGE_SHIFT entries), which
scales naturally with PAGE_SIZE. When the array reaches half capacity,
the workqueue is scheduled to drain pending handles.
zs_free_deferred() uses spin_trylock() to access the deferred queue.
If the lock is contended (e.g. drain in progress) or the queue is full,
it falls back to synchronous zs_free() to guarantee correctness.
Also introduce zs_free_deferred_flush() for use during pool teardown to
ensure all pending handles are freed.
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
include/linux/zsmalloc.h | 2 +
mm/zsmalloc.c | 111 +++++++++++++++++++++++++++++++++++++++
2 files changed, 113 insertions(+)
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 478410c880b1..1e5ac1a39d41 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -30,6 +30,8 @@ void zs_destroy_pool(struct zs_pool *pool);
unsigned long zs_malloc(struct zs_pool *pool, size_t size, gfp_t flags,
const int nid);
void zs_free(struct zs_pool *pool, unsigned long obj);
+void zs_free_deferred(struct zs_pool *pool, unsigned long handle);
+void zs_free_deferred_flush(struct zs_pool *pool);
size_t zs_huge_class_size(struct zs_pool *pool);
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 40687c8a7469..defc892555e4 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -53,6 +53,10 @@
#define ZS_HANDLE_SIZE (sizeof(unsigned long))
+#define ZS_DEFERRED_FREE_MAX_BYTES (128 << 20)
+#define ZS_DEFERRED_FREE_CAPACITY (ZS_DEFERRED_FREE_MAX_BYTES >> PAGE_SHIFT)
+#define ZS_DEFERRED_FREE_THRESHOLD (ZS_DEFERRED_FREE_CAPACITY / 2)
+
/*
* Object location (<PFN>, <obj_idx>) is encoded as
* a single (unsigned long) handle value.
@@ -217,6 +221,13 @@ struct zs_pool {
/* protect zspage migration/compaction */
rwlock_t lock;
atomic_t compaction_in_progress;
+
+ /* deferred free support */
+ spinlock_t deferred_lock;
+ unsigned long *deferred_handles;
+ unsigned int deferred_count;
+ unsigned int deferred_capacity;
+ struct work_struct deferred_free_work;
};
static inline void zpdesc_set_first(struct zpdesc *zpdesc)
@@ -579,6 +590,19 @@ static int zs_stats_size_show(struct seq_file *s, void *v)
}
DEFINE_SHOW_ATTRIBUTE(zs_stats_size);
+static int zs_stats_deferred_show(struct seq_file *s, void *v)
+{
+ struct zs_pool *pool = s->private;
+
+ spin_lock(&pool->deferred_lock);
+ seq_printf(s, "pending: %u\n", pool->deferred_count);
+ seq_printf(s, "capacity: %u\n", pool->deferred_capacity);
+ spin_unlock(&pool->deferred_lock);
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(zs_stats_deferred);
+
static void zs_pool_stat_create(struct zs_pool *pool, const char *name)
{
if (!zs_stat_root) {
@@ -590,6 +614,9 @@ static void zs_pool_stat_create(struct zs_pool *pool, const char *name)
debugfs_create_file("classes", S_IFREG | 0444, pool->stat_dentry, pool,
&zs_stats_size_fops);
+ debugfs_create_file("deferred_free", S_IFREG | 0444,
+ pool->stat_dentry, pool,
+ &zs_stats_deferred_fops);
}
static void zs_pool_stat_destroy(struct zs_pool *pool)
@@ -1432,6 +1459,76 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
}
EXPORT_SYMBOL_GPL(zs_free);
+static void zs_deferred_free_work(struct work_struct *work)
+{
+ struct zs_pool *pool = container_of(work, struct zs_pool,
+ deferred_free_work);
+ unsigned long handle;
+
+ while (1) {
+ spin_lock(&pool->deferred_lock);
+ if (pool->deferred_count == 0) {
+ spin_unlock(&pool->deferred_lock);
+ break;
+ }
+ handle = pool->deferred_handles[--pool->deferred_count];
+ spin_unlock(&pool->deferred_lock);
+
+ zs_free(pool, handle);
+ cond_resched();
+ }
+}
+
+/**
+ * zs_free_deferred - queue a handle for asynchronous freeing
+ * @pool: pool to free from
+ * @handle: handle to free
+ *
+ * Place @handle into a deferred free queue for later processing by a
+ * workqueue. This is intended for callers that are in atomic context
+ * (e.g. under a spinlock) and cannot afford the cost of zs_free()
+ * directly. When the queue reaches a threshold the work is scheduled.
+ * Falls back to synchronous zs_free() if the lock is contended (drain
+ * in progress) or if the queue is full.
+ */
+void zs_free_deferred(struct zs_pool *pool, unsigned long handle)
+{
+ if (IS_ERR_OR_NULL((void *)handle))
+ return;
+
+ if (!spin_trylock(&pool->deferred_lock))
+ goto sync_free;
+
+ if (pool->deferred_count >= pool->deferred_capacity) {
+ spin_unlock(&pool->deferred_lock);
+ goto sync_free;
+ }
+
+ pool->deferred_handles[pool->deferred_count++] = handle;
+ if (pool->deferred_count >= ZS_DEFERRED_FREE_THRESHOLD)
+ queue_work(system_wq, &pool->deferred_free_work);
+ spin_unlock(&pool->deferred_lock);
+ return;
+
+sync_free:
+ zs_free(pool, handle);
+}
+EXPORT_SYMBOL_GPL(zs_free_deferred);
+
+/**
+ * zs_free_deferred_flush - flush all pending deferred frees
+ * @pool: pool to flush
+ *
+ * Wait for any scheduled work to complete, then drain any remaining
+ * handles. Must be called from process context.
+ */
+void zs_free_deferred_flush(struct zs_pool *pool)
+{
+ flush_work(&pool->deferred_free_work);
+ zs_deferred_free_work(&pool->deferred_free_work);
+}
+EXPORT_SYMBOL_GPL(zs_free_deferred_flush);
+
static void zs_object_copy(struct size_class *class, unsigned long dst,
unsigned long src)
{
@@ -2099,6 +2196,18 @@ struct zs_pool *zs_create_pool(const char *name)
rwlock_init(&pool->lock);
atomic_set(&pool->compaction_in_progress, 0);
+ spin_lock_init(&pool->deferred_lock);
+ pool->deferred_capacity = ZS_DEFERRED_FREE_CAPACITY;
+ pool->deferred_handles = kvmalloc_array(pool->deferred_capacity,
+ sizeof(unsigned long),
+ GFP_KERNEL);
+ if (!pool->deferred_handles) {
+ kfree(pool);
+ return NULL;
+ }
+ pool->deferred_count = 0;
+ INIT_WORK(&pool->deferred_free_work, zs_deferred_free_work);
+
pool->name = kstrdup(name, GFP_KERNEL);
if (!pool->name)
goto err;
@@ -2201,6 +2310,7 @@ void zs_destroy_pool(struct zs_pool *pool)
int i;
zs_unregister_shrinker(pool);
+ zs_free_deferred_flush(pool);
zs_flush_migration(pool);
zs_pool_stat_destroy(pool);
@@ -2224,6 +2334,7 @@ void zs_destroy_pool(struct zs_pool *pool)
kfree(class);
}
+ kvfree(pool->deferred_handles);
kfree(pool->name);
kfree(pool);
}
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v2 3/4] zram: defer zs_free() in swap slot free notification path
From: Wenchao Hao @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Nhat Pham, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm
Cc: Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <20260421121616.3298845-1-haowenchao@xiaomi.com>
From: "Barry Song (Xiaomi)" <baohua@kernel.org>
zram_slot_free_notify() is called on the process exit path when
unmapping swap entries. The slot_free() it calls internally invokes
zs_free(), which accounts for ~87% of slot_free() cost due to zsmalloc
internal locking (pool->lock, class->lock) and potential zspage freeing.
This blocks the process exit path, delaying overall memory release
during Android low-memory killing.
Split slot_free() into slot_free_extract() and the actual zs_free()
call. slot_free_extract() handles all slot metadata cleanup (clearing
flags, updating stats, zeroing handle/size) and returns the zsmalloc
handle that needs freeing. This separation has two benefits:
1. It makes the two responsibilities of slot_free() explicit: slot
metadata management (must be done under slot lock) vs zsmalloc
memory release (can be deferred).
2. It allows zram_slot_free_notify() to use zs_free_deferred() for
the handle, deferring the expensive zs_free() to a workqueue so
the exit path can release memory faster.
While at it, merge three separate clear_slot_flag() calls for
ZRAM_IDLE, ZRAM_INCOMPRESSIBLE, and ZRAM_PP_SLOT into a single
bitmask operation via clear_slot_flags_on_free(), reducing redundant
read-modify-write cycles on the same flags word.
All other slot_free() callers (write, discard, meta_free) continue
to use synchronous zs_free() through the unchanged slot_free()
wrapper.
Signed-off-by: Barry Song (Xiaomi) <baohua@kernel.org>
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
drivers/block/zram/zram_drv.c | 37 ++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 9 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c2afd1c34f4a..382c4dc57c8d 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -165,6 +165,15 @@ static inline bool slot_allocated(struct zram *zram, u32 index)
test_slot_flag(zram, index, ZRAM_WB);
}
+#define ZRAM_FLAGS_TO_CLEAR_ON_FREE (BIT(ZRAM_IDLE) | \
+ BIT(ZRAM_INCOMPRESSIBLE) | \
+ BIT(ZRAM_PP_SLOT))
+
+static inline void clear_slot_flags_on_free(struct zram *zram, u32 index)
+{
+ zram->table[index].attr.flags &= ~ZRAM_FLAGS_TO_CLEAR_ON_FREE;
+}
+
static inline void set_slot_comp_priority(struct zram *zram, u32 index,
u32 prio)
{
@@ -2000,17 +2009,20 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
return true;
}
-static void slot_free(struct zram *zram, u32 index)
+/*
+ * Clear slot metadata and extract the zsmalloc handle for freeing.
+ * Returns the handle that needs to be freed via zs_free(), or 0 if
+ * no zsmalloc freeing is needed (e.g. same-filled or writeback slots).
+ */
+static unsigned long slot_free_extract(struct zram *zram, u32 index)
{
- unsigned long handle;
+ unsigned long handle = 0;
#ifdef CONFIG_ZRAM_TRACK_ENTRY_ACTIME
zram->table[index].attr.ac_time = 0;
#endif
- clear_slot_flag(zram, index, ZRAM_IDLE);
- clear_slot_flag(zram, index, ZRAM_INCOMPRESSIBLE);
- clear_slot_flag(zram, index, ZRAM_PP_SLOT);
+ clear_slot_flags_on_free(zram, index);
set_slot_comp_priority(zram, index, 0);
if (test_slot_flag(zram, index, ZRAM_HUGE)) {
@@ -2041,9 +2053,7 @@ static void slot_free(struct zram *zram, u32 index)
handle = get_slot_handle(zram, index);
if (!handle)
- return;
-
- zs_free(zram->mem_pool, handle);
+ return 0;
atomic64_sub(get_slot_size(zram, index),
&zram->stats.compr_data_size);
@@ -2051,6 +2061,15 @@ static void slot_free(struct zram *zram, u32 index)
atomic64_dec(&zram->stats.pages_stored);
set_slot_handle(zram, index, 0);
set_slot_size(zram, index, 0);
+
+ return handle;
+}
+
+static void slot_free(struct zram *zram, u32 index)
+{
+ unsigned long handle = slot_free_extract(zram, index);
+
+ zs_free(zram->mem_pool, handle);
}
static int read_same_filled_page(struct zram *zram, struct page *page,
@@ -2794,7 +2813,7 @@ static void zram_slot_free_notify(struct block_device *bdev,
return;
}
- slot_free(zram, index);
+ zs_free_deferred(zram->mem_pool, slot_free_extract(zram, index));
slot_unlock(zram, index);
}
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v2 4/4] mm/zswap: defer zs_free() in zswap_invalidate() path
From: Wenchao Hao @ 2026-04-21 12:16 UTC (permalink / raw)
To: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Nhat Pham, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm
Cc: Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <20260421121616.3298845-1-haowenchao@xiaomi.com>
zswap_invalidate() is called on the same process exit path as
zram_slot_free_notify(). The zswap_entry_free() it calls internally
performs zs_free() which is expensive due to zsmalloc internal locking.
Unlike zram which has a trylock fallback, zswap_invalidate() executes
unconditionally, making the latency impact potentially worse.
Like zram, the expensive zs_free() here blocks the process exit path,
delaying overall memory release. Additionally, zswap_entry_free()
performs extra work beyond zs_free(): list_lru_del() (takes its own
spinlock), obj_cgroup accounting, and kmem_cache_free for the entry
itself.
Use zs_free_deferred() in zswap_invalidate() path to defer the
expensive zsmalloc handle freeing to a workqueue, allowing the exit
path to release memory faster. All other callers (zswap_load,
zswap_writeback_entry, zswap_store error paths) run in process context
and continue to use synchronous zs_free().
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
mm/zswap.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 0823cadd02b6..7291f6deb5b6 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -713,11 +713,16 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
/*
* Carries out the common pattern of freeing an entry's zsmalloc allocation,
* freeing the entry itself, and decrementing the number of stored pages.
+ * When @deferred is true, the zsmalloc handle is queued for async freeing
+ * instead of being freed immediately.
*/
-static void zswap_entry_free(struct zswap_entry *entry)
+static void __zswap_entry_free(struct zswap_entry *entry, bool deferred)
{
zswap_lru_del(&zswap_list_lru, entry);
- zs_free(entry->pool->zs_pool, entry->handle);
+ if (deferred)
+ zs_free_deferred(entry->pool->zs_pool, entry->handle);
+ else
+ zs_free(entry->pool->zs_pool, entry->handle);
zswap_pool_put(entry->pool);
if (entry->objcg) {
obj_cgroup_uncharge_zswap(entry->objcg, entry->length);
@@ -729,6 +734,11 @@ static void zswap_entry_free(struct zswap_entry *entry)
atomic_long_dec(&zswap_stored_pages);
}
+static void zswap_entry_free(struct zswap_entry *entry)
+{
+ __zswap_entry_free(entry, false);
+}
+
/*********************************
* compressed storage functions
**********************************/
@@ -1655,7 +1665,7 @@ void zswap_invalidate(swp_entry_t swp)
entry = xa_erase(tree, offset);
if (entry)
- zswap_entry_free(entry);
+ __zswap_entry_free(entry, true);
}
int zswap_swapon(int type, unsigned long nr_pages)
--
2.34.1
^ permalink raw reply related
* [PATCH 5.10.y] blk-cgroup: Reinit blkg_iostat_set after clearing in blkcg_reset_stats()
From: Vasiliy Kovalev @ 2026-04-21 13:29 UTC (permalink / raw)
To: stable; +Cc: Jens Axboe, Tejun Heo, cgroups, linux-block, lvc-project, kovalev
From: Waiman Long <longman@redhat.com>
[ Upstream commit 3d2af77e31ade05ff7ccc3658c3635ec1bea0979 ]
When blkg_alloc() is called to allocate a blkcg_gq structure
with the associated blkg_iostat_set's, there are 2 fields within
blkg_iostat_set that requires proper initialization - blkg & sync.
The former field was introduced by commit 3b8cc6298724 ("blk-cgroup:
Optimize blkcg_rstat_flush()") while the later one was introduced by
commit f73316482977 ("blk-cgroup: reimplement basic IO stats using
cgroup rstat").
Unfortunately those fields in the blkg_iostat_set's are not properly
re-initialized when they are cleared in v1's blkcg_reset_stats(). This
can lead to a kernel panic due to NULL pointer access of the blkg
pointer. The missing initialization of sync is less problematic and
can be a problem in a debug kernel due to missing lockdep initialization.
Fix these problems by re-initializing them after memory clearing.
Fixes: 3b8cc6298724 ("blk-cgroup: Optimize blkcg_rstat_flush()")
Fixes: f73316482977 ("blk-cgroup: reimplement basic IO stats using cgroup rstat")
Signed-off-by: Waiman Long <longman@redhat.com>
Reviewed-by: Ming Lei <ming.lei@redhat.com>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://lore.kernel.org/r/20230606180724.2455066-1-longman@redhat.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
[ Remove this line: bis -> blkg = blkg for blkg was introduced by commit
3b8cc6298724 ("blk-cgroup: Optimize blkcg_rstat_flush()") since v6.2. ]
Signed-off-by: Alva Lan <alvalan9@foxmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 0561aa6033dd181594116d705c41fc16e97161a2)
[ kovalev: bp to fix CVE-2023-53421 ]
Signed-off-by: Vasiliy Kovalev <kovalev@altlinux.org>
---
block/blk-cgroup.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index dbd18b75ec91..7ffdc3360a6c 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -464,8 +464,12 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
struct blkg_iostat_set *bis =
per_cpu_ptr(blkg->iostat_cpu, cpu);
memset(bis, 0, sizeof(*bis));
+
+ /* Re-initialize the cleared blkg_iostat_set */
+ u64_stats_init(&bis->sync);
}
memset(&blkg->iostat, 0, sizeof(blkg->iostat));
+ u64_stats_init(&blkg->iostat.sync);
for (i = 0; i < BLKCG_MAX_POLS; i++) {
struct blkcg_policy *pol = blkcg_policy[i];
--
2.50.1
^ permalink raw reply related
* Re: [RFC PATCH v2 0/4] mm/zsmalloc: reduce zs_free() latency on swap release path
From: Nhat Pham @ 2026-04-21 15:54 UTC (permalink / raw)
To: Wenchao Hao
Cc: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Sergey Senozhatsky, Yosry Ahmed, linux-block,
linux-kernel, linux-mm, Barry Song, Xueyuan Chen, Wenchao Hao,
Kairui Song
In-Reply-To: <20260421121616.3298845-1-haowenchao@xiaomi.com>
On Tue, Apr 21, 2026 at 5:16 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
>
> Swap freeing can be expensive when unmapping a VMA containing
> many swap entries. This has been reported to significantly
> delay memory reclamation during Android's low-memory killing,
> especially when multiple processes are terminated to free
> memory, with slot_free() accounting for more than 80% of
> the total cost of freeing swap entries.
>
> Two earlier attempts by Lei and Zhiguo added a new thread in the mm core
> to asynchronously collect and free swap entries [1][2], but the
> design itself is fairly complex.
>
> When anon folios and swap entries are mixed within a
> process, reclaiming anon folios from killed processes
> helps return memory to the system as quickly as possible,
> so that newly launched applications can satisfy their
> memory demands. It is not ideal for swap freeing to block
> anon folio freeing. On the other hand, swap freeing can
> still return memory to the system, although at a slower
> rate due to memory compression.
Is this correct? I don't think we do decompression in
zswap_invalidate() path. We do decompression in zswap_load(), but as a
separate step from zswap_invalidate().
zswap/zsmalloc entry freeing is decoupled from decompression. For
example, on process teardown, we free the zsmalloc memory but never
decompress (if we do then it's a bug to be fixed lol, but I doubt it).
Zsmalloc freeing might not be worth as much bang-for-your-buck wise
compared to anon folio freeing, but if it's "expensive", then I think
that points to a different root-cause: zsmalloc's poor scalability in
the free path.
I've stared at this code path for a bit, because my other patch series
(vswap - see [1]) was reported to display regression on the free path
on the usemem benchmark. And one of the issues was the contention
between compaction (both systemwide compaction, i.e zs_page_migrate,
and zsmalloc's internal compaction, but mostly the former).:
* zs_free read-acquires pool->lock, and compaction write-acquires the
same lock. So the compaction thread will make all zs free-ers wait for
it. I saw this read lock delay when I perfed the free step of usemem.
* If this lock has fair queue-ing semantics (I have not checked), then
if there a compaction is behind a bunch of zs_free in the queue, then
all the subsequent zs_free's ers are blocked :)
* I'm also curious about cache-friendliness of this rwlock, bouncing
across CPUs, if you have multiple processes being torn down
concurrently.
Have you perf-ed process teardown yet? Can I ask you for a perf trace
on this part? I'm not against async zs-freeing (might still be
required after all), but if it's something fixable on the zsmalloc
side, we should probably prioritize that :) Otherwise these swap
freeing workers will exhibit the same poor scalability behavior - we
might be better off because we manage to get rid of bigger chunks of
uncompressed memory first, but we will still be slowed in releasing
the system's and cgroup's (in zswap's case) compressed memory
I'd love to hear more about thoughts from Yosry, Johannes, Sergey and
Minchan too.
^ permalink raw reply
* Re: [RFC PATCH v2 4/4] mm/zswap: defer zs_free() in zswap_invalidate() path
From: Nhat Pham @ 2026-04-21 17:03 UTC (permalink / raw)
To: Wenchao Hao
Cc: Andrew Morton, Chengming Zhou, Jens Axboe, Johannes Weiner,
Minchan Kim, Sergey Senozhatsky, Yosry Ahmed, linux-block,
linux-kernel, linux-mm, Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <20260421121616.3298845-5-haowenchao@xiaomi.com>
On Tue, Apr 21, 2026 at 5:16 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
>
> zswap_invalidate() is called on the same process exit path as
> zram_slot_free_notify(). The zswap_entry_free() it calls internally
> performs zs_free() which is expensive due to zsmalloc internal locking.
> Unlike zram which has a trylock fallback, zswap_invalidate() executes
> unconditionally, making the latency impact potentially worse.
Hmmm my understanding is that we don't have contention at this point,
because zswap mainly relies on swap cache to synchronize.
But yeah I can see the effect of slow zsmalloc entry freeing here.
>
> Like zram, the expensive zs_free() here blocks the process exit path,
> delaying overall memory release. Additionally, zswap_entry_free()
> performs extra work beyond zs_free(): list_lru_del() (takes its own
> spinlock), obj_cgroup accounting, and kmem_cache_free for the entry
> itself.
>
> Use zs_free_deferred() in zswap_invalidate() path to defer the
> expensive zsmalloc handle freeing to a workqueue, allowing the exit
> path to release memory faster. All other callers (zswap_load,
> zswap_writeback_entry, zswap_store error paths) run in process context
> and continue to use synchronous zs_free().
I wonder if this approach can speed up zswap_load() (i.e page fault
latency) too?
Code LGTM correctness-wise (assuming zs_free_deferred works) :)
>
> Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
> ---
> mm/zswap.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 0823cadd02b6..7291f6deb5b6 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -713,11 +713,16 @@ static void zswap_entry_cache_free(struct zswap_entry *entry)
> /*
> * Carries out the common pattern of freeing an entry's zsmalloc allocation,
> * freeing the entry itself, and decrementing the number of stored pages.
> + * When @deferred is true, the zsmalloc handle is queued for async freeing
> + * instead of being freed immediately.
> */
> -static void zswap_entry_free(struct zswap_entry *entry)
> +static void __zswap_entry_free(struct zswap_entry *entry, bool deferred)
> {
> zswap_lru_del(&zswap_list_lru, entry);
> - zs_free(entry->pool->zs_pool, entry->handle);
> + if (deferred)
> + zs_free_deferred(entry->pool->zs_pool, entry->handle);
> + else
> + zs_free(entry->pool->zs_pool, entry->handle);
> zswap_pool_put(entry->pool);
> if (entry->objcg) {
> obj_cgroup_uncharge_zswap(entry->objcg, entry->length);
> @@ -729,6 +734,11 @@ static void zswap_entry_free(struct zswap_entry *entry)
> atomic_long_dec(&zswap_stored_pages);
> }
>
> +static void zswap_entry_free(struct zswap_entry *entry)
> +{
> + __zswap_entry_free(entry, false);
> +}
> +
> /*********************************
> * compressed storage functions
> **********************************/
> @@ -1655,7 +1665,7 @@ void zswap_invalidate(swp_entry_t swp)
>
> entry = xa_erase(tree, offset);
> if (entry)
> - zswap_entry_free(entry);
> + __zswap_entry_free(entry, true);
> }
>
> int zswap_swapon(int type, unsigned long nr_pages)
> --
> 2.34.1
>
^ permalink raw reply
* Re: [RFC PATCH v2 0/4] mm/zsmalloc: reduce zs_free() latency on swap release path
From: Kairui Song @ 2026-04-21 17:17 UTC (permalink / raw)
To: Nhat Pham
Cc: Wenchao Hao, Andrew Morton, Chengming Zhou, Jens Axboe,
Johannes Weiner, Minchan Kim, Sergey Senozhatsky, Yosry Ahmed,
linux-block, linux-kernel, linux-mm, Barry Song, Xueyuan Chen,
Wenchao Hao
In-Reply-To: <CAKEwX=M5YpR0cQrryX_y4pm_BuxyUWZ_8MbhWodwbf1Fe=gzew@mail.gmail.com>
On Tue, Apr 21, 2026 at 11:55 PM Nhat Pham <nphamcs@gmail.com> wrote:
>
Thanks for adding me to the Cc list :), Barry started this idea with
ZRAM, which looks very interesting to me.
> On Tue, Apr 21, 2026 at 5:16 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
> >
> > Swap freeing can be expensive when unmapping a VMA containing
> > many swap entries. This has been reported to significantly
> > delay memory reclamation during Android's low-memory killing,
> > especially when multiple processes are terminated to free
> > memory, with slot_free() accounting for more than 80% of
> > the total cost of freeing swap entries.
> >
> > Two earlier attempts by Lei and Zhiguo added a new thread in the mm core
> > to asynchronously collect and free swap entries [1][2], but the
> > design itself is fairly complex.
> >
> > When anon folios and swap entries are mixed within a
> > process, reclaiming anon folios from killed processes
> > helps return memory to the system as quickly as possible,
> > so that newly launched applications can satisfy their
> > memory demands. It is not ideal for swap freeing to block
> > anon folio freeing. On the other hand, swap freeing can
> > still return memory to the system, although at a slower
> > rate due to memory compression.
>
> Is this correct? I don't think we do decompression in
> zswap_invalidate() path. We do decompression in zswap_load(), but as a
> separate step from zswap_invalidate().
It's not about decompression. I think what Wenchao means here is that:
freeing the swap entry also releases the backing compression data, but
compared to freeing an actual folio (which bring back a free folio to
reduce memory pressure), you may need to free a lot of swap entries to
free one whole folio, because the compressed data could be much
smaller than folio and with fragmentation. And swap entry freeing is
still not fast enough to be ignored.
>
> zswap/zsmalloc entry freeing is decoupled from decompression. For
> example, on process teardown, we free the zsmalloc memory but never
> decompress (if we do then it's a bug to be fixed lol, but I doubt it).
>
> Zsmalloc freeing might not be worth as much bang-for-your-buck wise
> compared to anon folio freeing, but if it's "expensive", then I think
> that points to a different root-cause: zsmalloc's poor scalability in
> the free path.
That's a very nice insight. I had an idea previously that can we have
something like a zs free bulk? Freeing handles one by one does seem
expensive.
https://lore.kernel.org/linux-mm/adt3Q_SRToF6fb3W@KASONG-MC4/
It might be tricky to do so though.
It will be best if we can speed up everything, doing things async
doesn't reduce the total amount of work, and might cause more trouble
like worker overhead or delayed freeing causing more memory pressure,
if the workqueue didn't run in time. Or maybe a process is almost
completely swapped out, then this won't help at all.
I'm not against the async idea, they might combine well.
>
> I've stared at this code path for a bit, because my other patch series
> (vswap - see [1]) was reported to display regression on the free path
> on the usemem benchmark. And one of the issues was the contention
> between compaction (both systemwide compaction, i.e zs_page_migrate,
> and zsmalloc's internal compaction, but mostly the former).:
>
> * zs_free read-acquires pool->lock, and compaction write-acquires the
> same lock. So the compaction thread will make all zs free-ers wait for
> it. I saw this read lock delay when I perfed the free step of usemem.
>
> * If this lock has fair queue-ing semantics (I have not checked), then
> if there a compaction is behind a bunch of zs_free in the queue, then
> all the subsequent zs_free's ers are blocked :)
>
> * I'm also curious about cache-friendliness of this rwlock, bouncing
> across CPUs, if you have multiple processes being torn down
> concurrently.
That's interesting, when I mentioned zs free bulk I was thinking that,
if we have a percpu queue, at least we may try read lock that on every
enqueue, free the whole queue if successful, then release the lock.
I'm sure there are more ways to optimize that, just a random idea :)
^ permalink raw reply
* Re: (subset) [PATCH 1/2] sg: don't use GFP_ATOMIC in sg_start_req
From: Jens Axboe @ 2026-04-21 17:42 UTC (permalink / raw)
To: Doug Gilbert, Martin K. Petersen, Christoph Hellwig
Cc: Shin'ichiro Kawasaki, linux-block, linux-scsi
In-Reply-To: <20260415060813.807659-2-hch@lst.de>
On Wed, 15 Apr 2026 08:08:06 +0200, Christoph Hellwig wrote:
> sg_start_req is called from normal user context and can sleep when
> waiting for memory. Switch it to use GFP_KERNEL, which fixes allocation
> failures seend with the bio_alloc rework.
Applied, thanks!
[2/2] block: only restrict bio allocation gfp mask asked to block
commit: b5129bda5bbcceea5b2589c8248d39f77660aa19
Best regards,
--
Jens Axboe
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox