* Re: [RFC PATCH v3 0/4] mm/zsmalloc: per-cpu deferred free to accelerate swap entry release
From: Nhat Pham @ 2026-05-09 0:08 UTC (permalink / raw)
To: Wenchao Hao
Cc: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Sergey Senozhatsky, Yosry Ahmed, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
On Thu, May 7, 2026 at 11:08 PM 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.
>
> This series introduces a callback-based deferred free framework in
> zsmalloc. Callers (zram, zswap) register push/drain callbacks to
> define what gets buffered and how it gets drained. The entire free
> path including caller-side bookkeeping (slot_free, zswap_entry_free)
> is deferred to a background worker.
>
> Implementation:
> - Each CPU owns a single-page buffer. The hot path writes a value
> via the push callback with preemption disabled (no locks).
> - When the buffer fills, it is swapped with a fresh page from a
> pre-allocated page pool. The full page is queued to a WQ_UNBOUND
> worker for drain.
> - The drain callback performs the actual expensive work (zs_free,
> slot_free, zswap_entry_free, etc.) in batch, off the hot path.
> - If no free page is available, the caller falls back to synchronous
> processing.
>
> The speedup comes from moving expensive swap slot freeing off the
> munmap hot path into a background worker, so that intact anonymous
> folios are released back to the system without blocking. The worker
> drains at a slower rate since compressed objects are small and freeing
> a single handle may not release an entire page until the zspage is
> fully empty.
>
> Performance results (Raspberry Pi 4B, ARM64, 8GB RAM):
>
> Test 1: munmap latency for 256MB swap-filled VMA (zram backend)
>
> mode Base Patched Speedup
> single 61.82ms 8.62ms 7.17x
> multi 2p 94.75ms 54.11ms 1.75x
> multi 3p 154.64ms 104.83ms 1.48x
>
> Test 2: munmap latency for different sizes (zram, single process)
>
> Size Base Patched Speedup
> 64MB 14.11ms 2.18ms 6.47x
> 128MB 29.45ms 4.48ms 6.57x
> 192MB 43.85ms 6.62ms 6.62x
> 256MB 57.01ms 9.08ms 6.28x
> 512MB 115.13ms 55.58ms 2.07x
> 1024MB 229.66ms 153.28ms 1.50x
>
> Test 3: munmap latency for 256MB swap-filled VMA (zswap backend)
>
> mode Base Patched Speedup
> single 152.14ms 51.26ms 2.97x
> multi 2p 186.56ms 105.42ms 1.77x
> multi 3p 205.83ms 153.32ms 1.34x
>
> Test 4: munmap latency for different sizes (zswap, single process)
>
> Size Base Patched Speedup
> 64MB 37.83ms 13.26ms 2.85x
> 128MB 75.11ms 26.73ms 2.81x
> 256MB 150.78ms 52.97ms 2.85x
> 512MB 303.04ms 130.38ms 2.32x
> 1024MB 599.95ms 287.10ms 2.09x
>
Hmmm, why are we batching at the zswap/zsmalloc level like this? I
agree with Yosry that this seems like somewhat of an unnecessary
layering violation. For example, do we observe a lot more performance
wins by doing this instead of just simply:
static void zswap_entry_free(swp_entry_t swp, bool deferred)
{
...
if (!deferred || !zs_deferred_free(entry->pool->zs_pool , entry->handle))
zs_free(entry->pool->zs_pool , entry->handle);
}
(basically what you had in the last version).
One weird effect of doing deferred zswap entry freeing like what you
are proposing here, is that the zswap LRU will be littered with stale
zswap entries. Seems like you removed them from the zswap xarray, but
they're still linked into the zswap LRU? At writeback time, that will
throw off the statistics used in the heuristics, and will make
writeback go through a bunch of stale entries, wasting more cycles :)
Seems a bit inelegant, no?
^ permalink raw reply
* Re: [RFC PATCH v2 0/4] mm/zsmalloc: reduce zs_free() latency on swap release path
From: Nhat Pham @ 2026-05-08 23:32 UTC (permalink / raw)
To: Wenchao Hao
Cc: Yosry Ahmed, Andrew Morton, Chengming Zhou, Jens Axboe,
Johannes Weiner, Minchan Kim, Sergey Senozhatsky, linux-block,
linux-kernel, linux-mm, Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <CAOptpSOnRCHo35KK5x3S0f5y9+7+dxfznpjNRH072jZvhYHwww@mail.gmail.com>
On Wed, May 6, 2026 at 6:55 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
>
> On Sat, May 2, 2026 at 3:21 PM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > > With only the class_idx-in-obj encoding (eliminating pool->lock from
> > > zs_free, no deferred freeing), we measured on two platforms.
> > >
> > > Test: each process independently mmap 256MB, write data, madvise
> > > MADV_PAGEOUT to swap out via zram (lzo-rle), then concurrent munmap.
> > >
> > > Raspberry Pi 4B (4-core ARM64 Cortex-A72):
> > >
> > > mode Base ClassIdx-only Speedup
> > > single 59.0ms 56.0ms 1.05x
> > > multi 2p 94.6ms 66.7ms 1.42x
> > > multi 4p 202.9ms 110.6ms 1.83x
> > >
> > > x86 physical machine (4-core Intel i7-12700, 2 rounds averaged):
> > >
> > > mode Base ClassIdx-only Speedup
> > > single 11.7ms 9.8ms 1.19x
> > > multi 2p 24.1ms 17.2ms 1.40x
> > > multi 4p 63.0ms 45.3ms 1.39x
> >
> > Oh man, you are eliminating pool lock here right? This would help my
> > other patch series a lot too :)
> >
> > https://lore.kernel.org/all/CAKEwX=M5YpR0cQrryX_y4pm_BuxyUWZ_8MbhWodwbf1Fe=gzew@mail.gmail.com/
> > https://lore.kernel.org/all/CAKEwX=PkFiP+u+ThrzjTKBi+usQf2uuhTZcfB2BNNA8RboOFDQ@mail.gmail.com/
> >
>
> Yes, exactly. With class_idx encoded in the obj value,
> zs_free() can determine the correct size_class without
> any pool-level lock. The lockless read gives a valid
> class_idx because it's invariant across migration (only
> PFN changes), and we re-read obj under class->lock to
> get a stable PFN.
>
> > Well, the deferred freeing would completely move that contention out
> > of the way lol. But this would benefit all users, regardless of
> > whether we're deferring the free step or not (for instance, this will
> > reduce contention between page fault and compaction, IIUC?) I feel
> > like you'll get some good numbers testing in a system with compaction
> > and THP enabled, with lots of swap activities. Which is... a lot of
> > server setup :)
> >
> > If the deferred freeing is too controversial, this smells like
> > something that should be upstreamed independently.
> >
>
> Agreed. We're planning to split the series so that the
> class_idx encoding + pool->lock elimination can be
> reviewed and merged independently of the deferred free
> framework. It's a pure win with no behavioral change
> — just less lock contention.
>
> > >
> > > Single-process shows modest improvement. With multiple processes,
> > > each read_lock/read_unlock atomically modifies the shared rwlock
> > > reader count, and the cost of these atomic operations increases
> > > with more CPUs accessing the same cacheline concurrently.
> > > Eliminating pool->lock removes this overhead entirely.
> > >
> > > This only works on 64-bit systems where OBJ_INDEX_BITS has enough
> > > spare bits to fit class_idx. 32-bit systems don't have the room.
> > > I'm still working on the compile-time gating to properly enable
> > > this based on architecture and page size configuration.
> >
> > /*
> > * The pool->lock protects the race with zpage's migration
> > * so it's safe to get the page from handle.
> > */
> > read_lock(&pool->lock);
> > obj = handle_to_obj(handle);
> > obj_to_zpdesc(obj, &f_zpdesc);
> > zspage = get_zspage(f_zpdesc);
> > class = zspage_class(pool, zspage);
> > spin_lock(&class->lock);
> > read_unlock(&pool->lock);
> >
> > It's basically just this blob right?
> >
>
> Yes, that's the blob being replaced. On the
> ZS_OBJ_CLASS_IDX path (64-bit systems), it becomes:
>
> obj = handle_to_obj(handle);
> class = pool->size_class[obj_to_class_idx(obj)];
> spin_lock(&class->lock);
> obj = handle_to_obj(handle); /* re-read for stable PFN */
>
> No pool->lock at all. We've also added compile-time
> gating (#if BITS_PER_LONG >= 64) since 32-bit systems
> lack the spare bits in OBJ_INDEX to fit class_idx. On
> 32-bit, it falls back to the original pool->lock path.
>
BTW, I've tested your idea with a hacky prototype, when I was playing
with my vswap series. It absolutely improves free time in the usemem
benchmark :) Idea is very promising - I won't scoop your work of
course, just letting you know that at least in my use case, it works
:) Look forward to seeing it submitted soon!!!
^ permalink raw reply
* Re: [PATCH] lib: free pagelist on error in iov_iter_extract_pages()
From: Andrew Morton @ 2026-05-08 21:48 UTC (permalink / raw)
To: Dmitry Antipov
Cc: Jens Axboe, Christoph Hellwig, linux-block, linux-fsdevel,
lvc-project, Fedor Pchelkin
In-Reply-To: <20260508111329.329943-1-dmantipov@yandex.ru>
On Fri, 8 May 2026 14:13:29 +0300 Dmitry Antipov <dmantipov@yandex.ru> wrote:
> Since 'iov_iter_extract_pages()' may allocate new pagelist if the passed
> one isn't large enough, the worst-case scenario may be:
>
> ...
> struct page *stack_pages[SMALL];
> struct page **pages = stack_pages;
> ...
> if (iov_iter_extract_pages(i..., &pages, ...) <= 0) {
> /* Even in case of error, new pagelist may be allocated */
> if (pages != stack_pages)
> kvfree(pages); [1]
> /* The rest of error handling and return */
> }
> /* Regular flow */
> ...
> if (pages != stack_pages)
> kvfree(pages);
> ...
> return 0;
>
> If you're unlucky so SMALL amount of pages wasn't enough and new
> pagelist was allocated, missing [1] causes the memory leak similar
> to one I've recently observed and fixed for 6.12 in [2]. So adjust
> 'iov_iter_extract_pages()' to make such a cleanup itself rather than
> rely on caller's handling on error paths, thus making [1] not needed.
AI review said things:
https://sashiko.dev/#/patchset/20260508111329.329943-1-dmantipov@yandex.ru
^ permalink raw reply
* Re: [PATCH v2] zram: fix use-after-free in zram_writeback_endio
From: Minchan Kim @ 2026-05-08 21:16 UTC (permalink / raw)
To: Richard Chang
Cc: Sergey Senozhatsky, Jens Axboe, Andrew Morton, bgeffon, liumartin,
linux-kernel, linux-block, linux-mm
In-Reply-To: <20260508084933.3730661-1-richardycc@google.com>
On Fri, May 08, 2026 at 08:49:33AM +0000, Richard Chang wrote:
> A crash was observed in zram_writeback_endio due to a NULL pointer
> dereference in wake_up. The root cause is a race condition between the
> bio completion handler (zram_writeback_endio) and the writeback task.
>
> In zram_writeback_endio, wake_up() is called on &wb_ctl->done_wait after
> releasing wb_ctl->done_lock. This creates a race window where the
> writeback task can see num_inflight become 0, return, and free wb_ctl
> before zram_writeback_endio calls wake_up().
>
> CPU 0 (zram_writeback_endio) CPU 1 (writeback_store)
> ============================ ============================
> zram_writeback_slots
> zram_submit_wb_request
> zram_submit_wb_request
> wait_event(wb_ctl->done_wait)
> spin_lock(&wb_ctl->done_lock);
> list_add(&req->entry, &wb_ctl->done_reqs);
> spin_unlock(&wb_ctl->done_lock);
> wake_up(&wb_ctl->done_wait);
> zram_complete_done_reqs
> spin_lock(&wb_ctl->done_lock);
> list_add(&req->entry, &wb_ctl->done_reqs);
> spin_unlock(&wb_ctl->done_lock);
> while (num_inflight) > 0)
> spin_lock(&wb_ctl->done_lock);
> list_del(&req->entry);
> spin_unlock(&wb_ctl->done_lock);
> // num_inflight becomes 0
> atomic_dec(num_inflight);
>
> // Leave zram_writeback_slots
> // Free wb_ctl
> release_wb_ctl(wb_ctl);
> // UAF crash!
> wake_up(&wb_ctl->done_wait);
>
> This patch fixes this race by using RCU. By protecting wb_ctl with
> rcu_read_lock() in zram_writeback_endio and using kfree_rcu() to free
> it, we ensure that wb_ctl remains valid during the execution of
> zram_writeback_endio.
>
> Fixes: f405066a1f0d ("zram: introduce writeback bio batching")
> Suggested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> Suggested-by: Minchan Kim <minchan@kernel.org>
> Signed-off-by: Richard Chang <richardycc@google.com>
Acked-by: Minchan Kim <minchan@kernel.org>
^ permalink raw reply
* Re: [GIT PULL] Block fixes for 7.1-rc3
From: pr-tracker-bot @ 2026-05-08 20:42 UTC (permalink / raw)
To: Jens Axboe; +Cc: Linus Torvalds, linux-block@vger.kernel.org
In-Reply-To: <36745ebc-0658-48b4-9878-982d5644642a@kernel.dk>
The pull request you sent on Fri, 8 May 2026 13:23:12 -0600:
> https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git tags/block-7.1-20260508
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/cbf457c584b5cbd0d44e8f05edaf3e189e894a68
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [RFC PATCH v3 0/4] mm/zsmalloc: per-cpu deferred free to accelerate swap entry release
From: Yosry Ahmed @ 2026-05-08 20:12 UTC (permalink / raw)
To: Wenchao Hao
Cc: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
()
On Thu, May 7, 2026 at 11:08 PM 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.
>
> This series introduces a callback-based deferred free framework in
> zsmalloc. Callers (zram, zswap) register push/drain callbacks to
> define what gets buffered and how it gets drained. The entire free
> path including caller-side bookkeeping (slot_free, zswap_entry_free)
> is deferred to a background worker.
How much of the speedup comes from avoiding the per-class lock,
free_zspage(), other work in zswap, etc.
I ask because I think the design here is still fairly complex. I don't
like how zswap and zram are registering callbacks into zsmalloc to do
their own freeing work, and they fill the buffers on behalf of
zsmalloc which seems like a layering violation.
I wonder how much of the speedup we get by just deferring
free_zspage()? That part can be done much more simply by just putting
the pages on a per-class list and having an async worker or a kthread
consume them and batch-free them. If the rest of zs_free() is also
expensive, we can do the deferred freeing on that level although it
would be more complicated as we need to have a fixed size buffer to
store them and handle running out of space.
A breakdown of where the slowdown is coming from would be helpful to
understand what to focus on.
>
> Implementation:
> - Each CPU owns a single-page buffer. The hot path writes a value
> via the push callback with preemption disabled (no locks).
> - When the buffer fills, it is swapped with a fresh page from a
> pre-allocated page pool. The full page is queued to a WQ_UNBOUND
> worker for drain.
> - The drain callback performs the actual expensive work (zs_free,
> slot_free, zswap_entry_free, etc.) in batch, off the hot path.
> - If no free page is available, the caller falls back to synchronous
> processing.
>
> The speedup comes from moving expensive swap slot freeing off the
> munmap hot path into a background worker, so that intact anonymous
> folios are released back to the system without blocking. The worker
> drains at a slower rate since compressed objects are small and freeing
> a single handle may not release an entire page until the zspage is
> fully empty.
>
> Performance results (Raspberry Pi 4B, ARM64, 8GB RAM):
>
> Test 1: munmap latency for 256MB swap-filled VMA (zram backend)
>
> mode Base Patched Speedup
> single 61.82ms 8.62ms 7.17x
> multi 2p 94.75ms 54.11ms 1.75x
> multi 3p 154.64ms 104.83ms 1.48x
>
> Test 2: munmap latency for different sizes (zram, single process)
>
> Size Base Patched Speedup
> 64MB 14.11ms 2.18ms 6.47x
> 128MB 29.45ms 4.48ms 6.57x
> 192MB 43.85ms 6.62ms 6.62x
> 256MB 57.01ms 9.08ms 6.28x
> 512MB 115.13ms 55.58ms 2.07x
> 1024MB 229.66ms 153.28ms 1.50x
>
> Test 3: munmap latency for 256MB swap-filled VMA (zswap backend)
>
> mode Base Patched Speedup
> single 152.14ms 51.26ms 2.97x
> multi 2p 186.56ms 105.42ms 1.77x
> multi 3p 205.83ms 153.32ms 1.34x
>
> Test 4: munmap latency for different sizes (zswap, single process)
>
> Size Base Patched Speedup
> 64MB 37.83ms 13.26ms 2.85x
> 128MB 75.11ms 26.73ms 2.81x
> 256MB 150.78ms 52.97ms 2.85x
> 512MB 303.04ms 130.38ms 2.32x
> 1024MB 599.95ms 287.10ms 2.09x
>
> [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/
>
> Changes since v2:
> - Use per-cpu single-page buffers instead of a global list; the hot
> path only writes into the local CPU's buffer with preemption disabled
> - Add a page pool for buffer rotation: when the current buffer is full,
> swap it with a free page from the pool and queue the full page for
> drain
> - Introduce push/drain callback ops so that zram and zswap can each
> define their own element size and drain logic (zram stores u32 slot
> indices, zswap stores unsigned long handles)
> - Drop the lock optimization patches it will be submitted separately
> as part of a dedicated zsmalloc lock contention series
> - Link to v2: https://lore.kernel.org/all/20260421121616.3298845-1-haowenchao@xiaomi.com/
>
> Barry Song (1):
> zram: use zsmalloc deferred free callback for async slot free
>
> Wenchao Hao (3):
> mm/zsmalloc: introduce deferred free framework with callback ops
> mm/zswap: use zsmalloc deferred free callback for async invalidate
> zram: batch clear flags in slot_free with single write
>
> drivers/block/zram/zram_drv.c | 44 ++++++-
> drivers/block/zram/zram_drv.h | 6 +
> include/linux/zsmalloc.h | 16 +++
> mm/zsmalloc.c | 208 +++++++++++++++++++++++++++++++++-
> mm/zswap.c | 38 ++++++-
> 5 files changed, 306 insertions(+), 6 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply
* [GIT PULL] Block fixes for 7.1-rc3
From: Jens Axboe @ 2026-05-08 19:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-block@vger.kernel.org
Hi Linus,
A few fixes for block that should go into the current kernel release.
These are mostly ublk related, in detail:
- Fix for ublk not doing an actual issue from the task_work fallback
path. Any request hitting that should be canceled automatically.
- Fix for uring_cmd prep side handling, for the block side uring_cmd
discard handling.
- Fix for missing validation of the io and physical block size shifts.
- Fix for a use-after-free in ublk's cancel command handling.
Please pull!
The following changes since commit 7e36e03d192a5547757bd8f9d9052a60f64e6d17:
Merge tag 'md-7.1-20260428' of https://git.kernel.org/pub/scm/linux/kernel/git/mdraid/linux into block-7.1 (2026-04-28 08:36:26 -0600)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux.git tags/block-7.1-20260508
for you to fetch changes up to f7700a4415afb3ac1767a556094e4ef8bd440e41:
ublk: fix use-after-free in ublk_cancel_cmd() (2026-05-08 06:44:42 -0600)
----------------------------------------------------------------
block-7.1-20260508
----------------------------------------------------------------
Jens Axboe (2):
ublk: don't issue uring_cmd from fallback task work
block: only read from sqe on initial invocation of blkdev_uring_cmd()
Ming Lei (2):
ublk: validate physical_bs_shift, io_min_shift and io_opt_shift
ublk: fix use-after-free in ublk_cancel_cmd()
block/ioctl.c | 24 +++++++++++++++---------
drivers/block/ublk_drv.c | 42 +++++++++++++++++++++++++++++++++++-------
2 files changed, 50 insertions(+), 16 deletions(-)
--
Jens Axboe
^ permalink raw reply
* Re: [PATCH] lib: free pagelist on error in iov_iter_extract_pages()
From: Caleb Sander Mateos @ 2026-05-08 18:33 UTC (permalink / raw)
To: Dmitry Antipov
Cc: Jens Axboe, Andrew Morton, Christoph Hellwig, linux-block,
linux-fsdevel, lvc-project, Fedor Pchelkin
In-Reply-To: <20260508111329.329943-1-dmantipov@yandex.ru>
On Fri, May 8, 2026 at 4:16 AM Dmitry Antipov <dmantipov@yandex.ru> wrote:
>
> Since 'iov_iter_extract_pages()' may allocate new pagelist if the passed
> one isn't large enough, the worst-case scenario may be:
>
> ...
> struct page *stack_pages[SMALL];
> struct page **pages = stack_pages;
> ...
> if (iov_iter_extract_pages(i..., &pages, ...) <= 0) {
> /* Even in case of error, new pagelist may be allocated */
> if (pages != stack_pages)
> kvfree(pages); [1]
iov_iter_extract_pages() will only allocate a pages array if the
initial struct page ** passed is NULL (see want_pages_array()). So the
condition pages != stack_pages will never be true. Indeed, it looks
like *all* callers of iov_iter_extract_pages() pass a non-NULL struct
page **. Would it make sense for iov_iter_extract_pages() to require a
pre-allocated pages array and remove support for allocating one?
> /* The rest of error handling and return */
> }
> /* Regular flow */
> ...
> if (pages != stack_pages)
> kvfree(pages);
> ...
> return 0;
>
> If you're unlucky so SMALL amount of pages wasn't enough and new
> pagelist was allocated, missing [1] causes the memory leak similar
> to one I've recently observed and fixed for 6.12 in [2]. So adjust
> 'iov_iter_extract_pages()' to make such a cleanup itself rather than
> rely on caller's handling on error paths, thus making [1] not needed.
>
> [2] https://lore.kernel.org/stable/20260505094529.406783-1-dmantipov@yandex.ru/T/#u
>
> Suggested-by: Fedor Pchelkin <pchelkin@ispras.ru>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> lib/iov_iter.c | 54 ++++++++++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 21 deletions(-)
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 243662af1af7..46dd11913df0 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1807,7 +1807,8 @@ static ssize_t iov_iter_extract_user_pages(struct iov_iter *i,
> * (*) Use with ITER_DISCARD is not supported as that has no content.
> *
> * On success, the function sets *@pages to the new pagelist, if allocated, and
> - * sets *offset0 to the offset into the first page.
> + * sets *offset0 to the offset into the first page. On error, new pagelist
> + * is freed if was allocated, and *@pages sets back to its original value.
> *
> * It may also return -ENOMEM and -EFAULT.
> */
> @@ -1818,31 +1819,42 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i,
> iov_iter_extraction_t extraction_flags,
> size_t *offset0)
> {
> + struct page **oldpages = *pages;
> + int ret;
Should be ssize_t to avoid truncation?
> +
> maxsize = min_t(size_t, min_t(size_t, maxsize, i->count), MAX_RW_COUNT);
> if (!maxsize)
> return 0;
>
> if (likely(user_backed_iter(i)))
> - return iov_iter_extract_user_pages(i, pages, maxsize,
> - maxpages, extraction_flags,
> - offset0);
> - if (iov_iter_is_kvec(i))
> - return iov_iter_extract_kvec_pages(i, pages, maxsize,
> - maxpages, extraction_flags,
> - offset0);
> - if (iov_iter_is_bvec(i))
> - return iov_iter_extract_bvec_pages(i, pages, maxsize,
> - maxpages, extraction_flags,
> - offset0);
> - if (iov_iter_is_folioq(i))
> - return iov_iter_extract_folioq_pages(i, pages, maxsize,
> - maxpages, extraction_flags,
> - offset0);
> - if (iov_iter_is_xarray(i))
> - return iov_iter_extract_xarray_pages(i, pages, maxsize,
> - maxpages, extraction_flags,
> - offset0);
> - return -EFAULT;
> + ret = iov_iter_extract_user_pages(i, pages, maxsize,
> + maxpages, extraction_flags,
> + offset0);
> + else if (iov_iter_is_kvec(i))
> + ret = iov_iter_extract_kvec_pages(i, pages, maxsize,
> + maxpages, extraction_flags,
> + offset0);
> + else if (iov_iter_is_bvec(i))
> + ret = iov_iter_extract_bvec_pages(i, pages, maxsize,
> + maxpages, extraction_flags,
> + offset0);
> + else if (iov_iter_is_folioq(i))
> + ret = iov_iter_extract_folioq_pages(i, pages, maxsize,
> + maxpages, extraction_flags,
> + offset0);
> + else if (iov_iter_is_xarray(i))
> + ret = iov_iter_extract_xarray_pages(i, pages, maxsize,
> + maxpages, extraction_flags,
> + offset0);
> + else
> + ret = -EFAULT;
> +
> + if (unlikely(ret) && *pages && *pages != oldpages) {
iov_iter_extract_pages() returns a positive value (number of bytes
extracted) on success (even if all maxsize bytes were extracted), so I
don't think this condition is correct. Callers seem to be inconsistent
about whether they treat ret < 0 or ret <= 0 as the error case, which
is another argument not to handle freeing the pages array inside
iov_iter_extract_pages().
Best,
Caleb
> + kvfree(*pages);
> + *pages = oldpages;
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL_GPL(iov_iter_extract_pages);
>
> --
> 2.54.0
>
>
^ permalink raw reply
* Re: [PATCH blktests] blktrace/001: Skip test when kernel lockdown is enabled
From: Daniel Wagner @ 2026-05-08 17:39 UTC (permalink / raw)
To: Disha Goel; +Cc: Shin'ichiro Kawasaki, linux-block
In-Reply-To: <f3617b18-526e-4006-a692-b704925de24b@linux.ibm.com>
On Fri, May 08, 2026 at 04:51:41PM +0530, Disha Goel wrote:
> After further investigation, I've identified the root cause. This
> failure is seen on SLES 16.x and RHEL 10.x with 6.12-based kernels when
> Secure Boot is enabled (kernel lockdown active).
SLES ships an bunch of additional patches which hardens the lockdown
feature a bit more. IIRC these are the same as RHEL uses. Not sure if
this is the source of the problem though.
https://raw.githubusercontent.com/openSUSE/kernel-source/refs/heads/SL-16.1/series.conf
# Lock down functions for secure boot
patches.suse/0001-security-lockdown-expose-a-hook-to-lock-the-kernel-down.patch
patches.suse/lockdown-fix-kernel-lockdown-enforcement-issue-when-secure.patch
patches.suse/0002-efi-Add-an-EFI_SECURE_BOOT-flag-to-indicate-secure-boot-mode.patch
patches.suse/0003-efi-Lock-down-the-kernel-if-booted-in-secure-boot-mode.patch
patches.suse/0004-efi-Lock-down-the-kernel-at-the-integrity-level-if-b.patch
The patches are in https://github.com/openSUSE/kernel-source/tree/SL-16.1/patches.suse
^ permalink raw reply
* Re: [PATCH] ublk: fix use-after-free in ublk_cancel_cmd()
From: Jens Axboe @ 2026-05-08 16:19 UTC (permalink / raw)
To: linux-block, Ming Lei; +Cc: Caleb Sander Mateos, Uday Shankar
In-Reply-To: <20260508123746.242018-1-tom.leiming@gmail.com>
On Fri, 08 May 2026 20:37:46 +0800, Ming Lei wrote:
> When ublk_reset_ch_dev() clears io->cmd via ublk_queue_reinit()
> concurrently with ublk_cancel_cmd(), ublk_cancel_cmd() can read a
> stale pointer and pass it to io_uring_cmd_done(), causing a
> use-after-free.
>
> Fix by synchronizing the two paths with ubq->cancel_lock:
>
> [...]
Applied, thanks!
[1/1] ublk: fix use-after-free in ublk_cancel_cmd()
(no commit info)
Best regards,
--
Jens Axboe
^ permalink raw reply
* [PATCH] ublk: fix use-after-free in ublk_cancel_cmd()
From: Ming Lei @ 2026-05-08 12:37 UTC (permalink / raw)
To: Jens Axboe, linux-block; +Cc: Caleb Sander Mateos, Uday Shankar, Ming Lei
When ublk_reset_ch_dev() clears io->cmd via ublk_queue_reinit()
concurrently with ublk_cancel_cmd(), ublk_cancel_cmd() can read a
stale pointer and pass it to io_uring_cmd_done(), causing a
use-after-free.
Fix by synchronizing the two paths with ubq->cancel_lock:
- ublk_cancel_cmd(): read and clear io->cmd under cancel_lock,
then call io_uring_cmd_done() on the saved local copy outside
the lock.
- ublk_reset_ch_dev(): hold cancel_lock across ublk_queue_reinit()
so that io->cmd and io->flags are cleared atomically with respect
to ublk_cancel_cmd().
Fixes: 216c8f5ef0f2 ("ublk: replace monitor with cancelable uring_cmd")
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
drivers/block/ublk_drv.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 57ec900f0ce0..6d13f1481de0 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2411,8 +2411,14 @@ static void ublk_reset_ch_dev(struct ublk_device *ub)
{
int i;
- for (i = 0; i < ub->dev_info.nr_hw_queues; i++)
- ublk_queue_reinit(ub, ublk_get_queue(ub, i));
+ for (i = 0; i < ub->dev_info.nr_hw_queues; i++) {
+ struct ublk_queue *ubq = ublk_get_queue(ub, i);
+
+ /* Sync with ublk_cancel_cmd() */
+ spin_lock(&ubq->cancel_lock);
+ ublk_queue_reinit(ub, ubq);
+ spin_unlock(&ubq->cancel_lock);
+ }
/* set to NULL, otherwise new tasks cannot mmap io_cmd_buf */
ub->mm = NULL;
@@ -2753,6 +2759,7 @@ static void ublk_cancel_cmd(struct ublk_queue *ubq, unsigned tag,
{
struct ublk_io *io = &ubq->ios[tag];
struct ublk_device *ub = ubq->dev;
+ struct io_uring_cmd *cmd = NULL;
struct request *req;
bool done;
@@ -2775,12 +2782,15 @@ static void ublk_cancel_cmd(struct ublk_queue *ubq, unsigned tag,
spin_lock(&ubq->cancel_lock);
done = !!(io->flags & UBLK_IO_FLAG_CANCELED);
- if (!done)
+ if (!done) {
io->flags |= UBLK_IO_FLAG_CANCELED;
+ cmd = io->cmd;
+ io->cmd = NULL;
+ }
spin_unlock(&ubq->cancel_lock);
- if (!done)
- io_uring_cmd_done(io->cmd, UBLK_IO_RES_ABORT, issue_flags);
+ if (!done && cmd)
+ io_uring_cmd_done(cmd, UBLK_IO_RES_ABORT, issue_flags);
}
/*
--
2.53.0
^ permalink raw reply related
* Re: [PATCH blktests] blktrace/001: Skip test when kernel lockdown is enabled
From: Disha Goel @ 2026-05-08 11:21 UTC (permalink / raw)
To: Shin'ichiro Kawasaki; +Cc: linux-block
In-Reply-To: <afIGq_uV-l9_98Px@shinmob>
On 29/04/26 7:22 pm, Shin'ichiro Kawasaki wrote:
> On Apr 24, 2026 / 19:41, Disha Goel wrote:
>> The blktrace/001 test fails on systems with Secure Boot enabled due to
>> kernel lockdown preventing access to debugfs. The test attempts to run
>> blktrace which requires access to /sys/kernel/debug/block/*/trace*
>> files, but kernel lockdown (enabled automatically with Secure Boot)
>> blocks this access, resulting in "Operation not permitted" errors.
>
> Hello Disha, thanks for the patch. I tried to recreate the "Operation not
> permitted" error on my test node, but I can not recreate it. I tried the
> command lines below, and saw blktrace worked fine with lockdown=confidentiality
> condition. This means that blktrace can access /sys/kernel/debug/block/*/trace*
> even when the kernel is locked down.
>
> ---------------------------------------------------------------------
> root@testnode1:~# cat /sys/kernel/security/lockdown
> [none] integrity confidentiality
> root@testnode1:~# echo confidentiality > /sys/kernel/security/lockdown
> root@testnode1:~# cat /sys/kernel/security/lockdown
> none integrity [confidentiality]
> root@testnode1:~# cd /tmp
> root@testnode1:/tmp# blktrace -d /dev/sdc &
> [1] 1014
> root@testnode1:/tmp# dd if=/dev/zero of=/dev/sdc bs=4k count=1 oflag=direct
> 1+0 records in
> 1+0 records out
> 4096 bytes (4.1 kB, 4.0 KiB) copied, 0.00274992 s, 1.5 MB/s
> root@testnode1:/tmp# kill 1014
> root@testnode1:/tmp# === sdc ===
> CPU 0: 6 events, 1 KiB data
> CPU 1: 0 events, 0 KiB data
> CPU 2: 658 events, 31 KiB data
> CPU 3: 797 events, 38 KiB data
> Total: 1461 events (dropped 0), 69 KiB data
>
> [1]+ Done blktrace -d /dev/sdc
> root@testnode1:/tmp# blkparse -i sdc | head
> 8,32 2 1 0.000000000 1048 Q WS 0 + 8 [dd]
> 8,32 2 0 0.000013242 1048 1,0 m N bfq [bfq_limit_depth] wr_busy 0 sync 1 depth 256
> 8,32 2 2 0.001495890 1048 G WS 0 + 8 [dd]
> 8,32 2 3 0.001497997 1048 P N [dd]
> 8,32 2 4 0.001499079 1048 U N [dd] 1
> 8,32 2 0 0.001574560 1048 1,0 m N bfq0A new_ioprio 4 new_weight 40
> 8,32 2 0 0.001577177 1048 1,0 m N bfq1048S allocated
> 8,32 2 0 0.001581069 1048 1,0 m N bfq1048S get_request 00000000e21f70ba: bfqq 000000001cef6c8d, 2
> 8,32 2 5 0.001583291 1048 I WS 0 + 8 [dd]
> 8,32 2 0 0.001584861 1048 1,0 m N bfq1048S add_request 1
> ---------------------------------------------------------------------
>
> I would like to understand why the blktrace error happens in your environment
> and does not happen in my environment. It will affect how to judge the skip of
> the test case blktrace/001.
>
> Could you share your system set up conditions? FYI, I used Fedora 43, QEMU VM,
> Intel server and v7.1-rc1 kernel for the trial above. I'm guessing any
> difference between the two environments causes the blktrace behavior difference.
>
Hi Shin'ichiro,
Thank you for the detailed testing and feedback.
After further investigation, I've identified the root cause. This
failure is seen on SLES 16.x and RHEL 10.x with 6.12-based kernels when
Secure Boot is enabled (kernel lockdown active).
I tested on upstream kernel v7.1.0-rc1 with lockdown enabled, and the
test ran fine. My apologies for not testing with upstream earlier.
The issue is caused by missing debugfs fixes in the distro kernels.
I'll work with SUSE and Red Hat to get these patches backported to their
6.12-based kernels.
I'm withdrawing this patch as it's a kernel bug, not a test framework
issue. Thank you for the thorough review.
> P.S. I found that kmemleak does not work when lockdown=confidentiality
> condition. This indicates that the kernel lockdown feature works for kmemleak
> as expected.
>
> ---------------------------------------------------------------------
> root@testnode1:~# cat /sys/kernel/debug/kmemleak
> root@testnode1:~# cat /sys/kernel/security/lockdown
> [none] integrity confidentiality
> root@testnode1:~# echo confidentiality > /sys/kernel/security/lockdown
> root@testnode1:~# cat /sys/kernel/security/lockdown
> none integrity [confidentiality]
> root@testnode1:~# cat /sys/kernel/debug/kmemleak
> cat: /sys/kernel/debug/kmemleak: Operation not permitted
> ---------------------------------------------------------------------
>
--
Regards,
Disha
^ permalink raw reply
* Re: [PATCH 6.12] block: fix memory leak in in bio_map_user_iov()
From: Fedor Pchelkin @ 2026-05-08 11:16 UTC (permalink / raw)
To: Dmitry Antipov
Cc: Greg Kroah-Hartman, stable, Jens Axboe, linux-block,
Christoph Hellwig, lvc-project, netdev
In-Reply-To: <5bd98789901e6bcd2b41d646209deb6e48ffb711.camel@yandex.ru>
On Fri, 08. May 11:30, Dmitry Antipov wrote:
> On Thu, 2026-05-07 at 21:52 +0300, Fedor Pchelkin wrote:
>
> > In some form the issue is present in current upstream as well. For
> > example, there is another callsite of iov_iter_extract_pages() in
> > block/bio-integrity.c where the same pattern still persists.
>
> Good point, and skb_splice_from_iter() looks suspicious as well:
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7dad68e3b518..bf053372acb2 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -7343,12 +7343,16 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter *iter,
>
> len = iov_iter_extract_pages(iter, &ppages, maxsize, nr, 0, &off);
This function does allocate memory for @pages argument only if *@pages is
NULL. I don't think it's NULL here, *@pages points to a stack-allocated
array.
> if (len <= 0) {
> + /* Possible memory leak - ppages should be vfree()'d
> + if reallocated (ppages != pages)? */
> ret = len ?: -EIO;
> break;
> }
>
> i = 0;
> do {
> + /* This looks wrong if reallocated - ppages[i++]
> + should be used instead? */
> struct page *page = pages[i++];
> size_t part = min_t(size_t, PAGE_SIZE - off, len);
>
> This issue likely crosses the boundaries of block subsystem so netdev
> people are encouraged to look as well.
Not in this case. The situations where iov_iter_extract_pages() needs to
allocate memory for @pages on its own happen when *@pages is NULL. In
current mainline it can occur at block/bio-integrity.c and probably
that's all.
Mind to prepare the patch, please? There are better chances to discuss
the problem directly with the patch at hand instead of expecting someone
to look at this [PATCH 6.12] thread.
^ permalink raw reply
* [PATCH] lib: free pagelist on error in iov_iter_extract_pages()
From: Dmitry Antipov @ 2026-05-08 11:13 UTC (permalink / raw)
To: Jens Axboe, Andrew Morton
Cc: Christoph Hellwig, linux-block, linux-fsdevel, lvc-project,
Dmitry Antipov, Fedor Pchelkin
Since 'iov_iter_extract_pages()' may allocate new pagelist if the passed
one isn't large enough, the worst-case scenario may be:
...
struct page *stack_pages[SMALL];
struct page **pages = stack_pages;
...
if (iov_iter_extract_pages(i..., &pages, ...) <= 0) {
/* Even in case of error, new pagelist may be allocated */
if (pages != stack_pages)
kvfree(pages); [1]
/* The rest of error handling and return */
}
/* Regular flow */
...
if (pages != stack_pages)
kvfree(pages);
...
return 0;
If you're unlucky so SMALL amount of pages wasn't enough and new
pagelist was allocated, missing [1] causes the memory leak similar
to one I've recently observed and fixed for 6.12 in [2]. So adjust
'iov_iter_extract_pages()' to make such a cleanup itself rather than
rely on caller's handling on error paths, thus making [1] not needed.
[2] https://lore.kernel.org/stable/20260505094529.406783-1-dmantipov@yandex.ru/T/#u
Suggested-by: Fedor Pchelkin <pchelkin@ispras.ru>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
lib/iov_iter.c | 54 ++++++++++++++++++++++++++++++--------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 243662af1af7..46dd11913df0 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1807,7 +1807,8 @@ static ssize_t iov_iter_extract_user_pages(struct iov_iter *i,
* (*) Use with ITER_DISCARD is not supported as that has no content.
*
* On success, the function sets *@pages to the new pagelist, if allocated, and
- * sets *offset0 to the offset into the first page.
+ * sets *offset0 to the offset into the first page. On error, new pagelist
+ * is freed if was allocated, and *@pages sets back to its original value.
*
* It may also return -ENOMEM and -EFAULT.
*/
@@ -1818,31 +1819,42 @@ ssize_t iov_iter_extract_pages(struct iov_iter *i,
iov_iter_extraction_t extraction_flags,
size_t *offset0)
{
+ struct page **oldpages = *pages;
+ int ret;
+
maxsize = min_t(size_t, min_t(size_t, maxsize, i->count), MAX_RW_COUNT);
if (!maxsize)
return 0;
if (likely(user_backed_iter(i)))
- return iov_iter_extract_user_pages(i, pages, maxsize,
- maxpages, extraction_flags,
- offset0);
- if (iov_iter_is_kvec(i))
- return iov_iter_extract_kvec_pages(i, pages, maxsize,
- maxpages, extraction_flags,
- offset0);
- if (iov_iter_is_bvec(i))
- return iov_iter_extract_bvec_pages(i, pages, maxsize,
- maxpages, extraction_flags,
- offset0);
- if (iov_iter_is_folioq(i))
- return iov_iter_extract_folioq_pages(i, pages, maxsize,
- maxpages, extraction_flags,
- offset0);
- if (iov_iter_is_xarray(i))
- return iov_iter_extract_xarray_pages(i, pages, maxsize,
- maxpages, extraction_flags,
- offset0);
- return -EFAULT;
+ ret = iov_iter_extract_user_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
+ else if (iov_iter_is_kvec(i))
+ ret = iov_iter_extract_kvec_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
+ else if (iov_iter_is_bvec(i))
+ ret = iov_iter_extract_bvec_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
+ else if (iov_iter_is_folioq(i))
+ ret = iov_iter_extract_folioq_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
+ else if (iov_iter_is_xarray(i))
+ ret = iov_iter_extract_xarray_pages(i, pages, maxsize,
+ maxpages, extraction_flags,
+ offset0);
+ else
+ ret = -EFAULT;
+
+ if (unlikely(ret) && *pages && *pages != oldpages) {
+ kvfree(*pages);
+ *pages = oldpages;
+ }
+
+ return ret;
}
EXPORT_SYMBOL_GPL(iov_iter_extract_pages);
--
2.54.0
^ permalink raw reply related
* [PATCH v2] zram: fix use-after-free in zram_writeback_endio
From: Richard Chang @ 2026-05-08 8:49 UTC (permalink / raw)
To: Minchan Kim, Sergey Senozhatsky, Jens Axboe, Andrew Morton
Cc: bgeffon, liumartin, linux-kernel, linux-block, linux-mm,
Richard Chang
In-Reply-To: <af1BNwmyHK6aU_uT@google.com>
A crash was observed in zram_writeback_endio due to a NULL pointer
dereference in wake_up. The root cause is a race condition between the
bio completion handler (zram_writeback_endio) and the writeback task.
In zram_writeback_endio, wake_up() is called on &wb_ctl->done_wait after
releasing wb_ctl->done_lock. This creates a race window where the
writeback task can see num_inflight become 0, return, and free wb_ctl
before zram_writeback_endio calls wake_up().
CPU 0 (zram_writeback_endio) CPU 1 (writeback_store)
============================ ============================
zram_writeback_slots
zram_submit_wb_request
zram_submit_wb_request
wait_event(wb_ctl->done_wait)
spin_lock(&wb_ctl->done_lock);
list_add(&req->entry, &wb_ctl->done_reqs);
spin_unlock(&wb_ctl->done_lock);
wake_up(&wb_ctl->done_wait);
zram_complete_done_reqs
spin_lock(&wb_ctl->done_lock);
list_add(&req->entry, &wb_ctl->done_reqs);
spin_unlock(&wb_ctl->done_lock);
while (num_inflight) > 0)
spin_lock(&wb_ctl->done_lock);
list_del(&req->entry);
spin_unlock(&wb_ctl->done_lock);
// num_inflight becomes 0
atomic_dec(num_inflight);
// Leave zram_writeback_slots
// Free wb_ctl
release_wb_ctl(wb_ctl);
// UAF crash!
wake_up(&wb_ctl->done_wait);
This patch fixes this race by using RCU. By protecting wb_ctl with
rcu_read_lock() in zram_writeback_endio and using kfree_rcu() to free
it, we ensure that wb_ctl remains valid during the execution of
zram_writeback_endio.
Fixes: f405066a1f0d ("zram: introduce writeback bio batching")
Suggested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Suggested-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Richard Chang <richardycc@google.com>
---
V1 -> V2: use RCU to manage the wb_ctl lifetime
drivers/block/zram/zram_drv.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index aebc710f0d6a..07111455eecf 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -33,6 +33,7 @@
#include <linux/cpuhotplug.h>
#include <linux/part_stat.h>
#include <linux/kernel_read_file.h>
+#include <linux/rcupdate.h>
#include "zram_drv.h"
@@ -504,6 +505,7 @@ struct zram_wb_ctl {
wait_queue_head_t done_wait;
spinlock_t done_lock;
atomic_t num_inflight;
+ struct rcu_head rcu;
};
struct zram_wb_req {
@@ -847,7 +849,7 @@ static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
release_wb_req(req);
}
- kfree(wb_ctl);
+ kfree_rcu(wb_ctl, rcu);
}
static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
@@ -964,11 +966,13 @@ static void zram_writeback_endio(struct bio *bio)
struct zram_wb_ctl *wb_ctl = bio->bi_private;
unsigned long flags;
+ rcu_read_lock();
spin_lock_irqsave(&wb_ctl->done_lock, flags);
list_add(&req->entry, &wb_ctl->done_reqs);
spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
wake_up(&wb_ctl->done_wait);
+ rcu_read_unlock();
}
static void zram_submit_wb_request(struct zram *zram,
--
2.54.0.563.g4f69b47b94-goog
^ permalink raw reply related
* Re: [PATCH 6.12] block: fix memory leak in in bio_map_user_iov()
From: Dmitry Antipov @ 2026-05-08 8:30 UTC (permalink / raw)
To: Fedor Pchelkin
Cc: Greg Kroah-Hartman, stable, Jens Axboe, linux-block,
Christoph Hellwig, lvc-project, netdev
In-Reply-To: <20260507212200-2614841ccc112a082cab6938-pchelkin@ispras>
On Thu, 2026-05-07 at 21:52 +0300, Fedor Pchelkin wrote:
> In some form the issue is present in current upstream as well. For
> example, there is another callsite of iov_iter_extract_pages() in
> block/bio-integrity.c where the same pattern still persists.
Good point, and skb_splice_from_iter() looks suspicious as well:
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7dad68e3b518..bf053372acb2 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -7343,12 +7343,16 @@ ssize_t skb_splice_from_iter(struct sk_buff *skb, struct iov_iter *iter,
len = iov_iter_extract_pages(iter, &ppages, maxsize, nr, 0, &off);
if (len <= 0) {
+ /* Possible memory leak - ppages should be vfree()'d
+ if reallocated (ppages != pages)? */
ret = len ?: -EIO;
break;
}
i = 0;
do {
+ /* This looks wrong if reallocated - ppages[i++]
+ should be used instead? */
struct page *page = pages[i++];
size_t part = min_t(size_t, PAGE_SIZE - off, len);
This issue likely crosses the boundaries of block subsystem so netdev
people are encouraged to look as well.
Dmitry
^ permalink raw reply related
* Re: [PATCH 1/2] block: pass a minsize argument to bio_iov_iter_bounce
From: Christoph Hellwig @ 2026-05-08 8:12 UTC (permalink / raw)
To: Pankaj Raghav (Samsung)
Cc: Christoph Hellwig, Jens Axboe, Christian Brauner, Darrick J. Wong,
linux-block, linux-xfs, linux-fsdevel
In-Reply-To: <7a24ulsyoczubsjc2y4rkv7tkgqzpo7ehkkljdytyo6tzqdw24@avwve4kr7tzy>
On Thu, May 07, 2026 at 01:36:11PM +0200, Pankaj Raghav (Samsung) wrote:
> On Thu, May 07, 2026 at 07:01:47AM +0200, Christoph Hellwig wrote:
> > When bouncing for block size > PAGE_SIZE file systems that require
> > file system block size alignment (e.g. zoned XFS), the bio needs to
> > be big enough to fit an entire block.
>
> Not related to the patch but I am wondering why we don't need this for
> non-zoned bs > PS configurations.
Because they only require sector size alignment for both read and
writes. So I guess we could actually hit it with a large sector size
and not just fs block size.
^ permalink raw reply
* [RFC PATCH v3 4/4] zram: batch clear flags in slot_free with single write
From: Wenchao Hao @ 2026-05-08 6:07 UTC (permalink / raw)
To: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Yosry Ahmed
Cc: Wenchao Hao, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
Replace four separate flag clear operations in slot_free() with a
single mask write. This reduces redundant read-modify-write cycles
on the same flags word.
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
drivers/block/zram/zram_drv.c | 5 +----
drivers/block/zram/zram_drv.h | 6 ++++++
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 0d07f0901e55..b1a565d35567 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -2011,10 +2011,7 @@ static void slot_free(struct zram *zram, u32 index)
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);
- set_slot_comp_priority(zram, index, 0);
+ zram->table[index].attr.flags &= ~ZRAM_SLOT_FREE_CLEAR_MASK;
if (test_slot_flag(zram, index, ZRAM_HUGE)) {
/*
diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 08d1774c15db..89a7e39a2f4b 100644
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -57,6 +57,12 @@ enum zram_pageflags {
__NR_ZRAM_PAGEFLAGS,
};
+#define ZRAM_SLOT_FREE_CLEAR_MASK (BIT(ZRAM_IDLE) | \
+ BIT(ZRAM_INCOMPRESSIBLE) | \
+ BIT(ZRAM_PP_SLOT) | \
+ (ZRAM_COMP_PRIORITY_MASK << \
+ ZRAM_COMP_PRIORITY_BIT1))
+
/*
* Allocated for each disk page. We use bit-lock (ZRAM_ENTRY_LOCK bit
* of flags) to save memory. There can be plenty of entries and standard
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v3 3/4] zram: use zsmalloc deferred free callback for async slot free
From: Wenchao Hao @ 2026-05-08 6:07 UTC (permalink / raw)
To: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Yosry Ahmed
Cc: Wenchao Hao, Barry Song, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
From: Barry Song <baohua@kernel.org>
Register zram_deferred_ops with zs_pool_enable_deferred_free() to
defer slot freeing to a WQ_UNBOUND worker. The notify hot path only
stores a u32 slot index into the per-cpu buffer (1024 entries/page).
The drain callback does slot_lock + slot_free + slot_unlock for each
index. On deferred failure (no free page), fallback to synchronous
slot_lock + slot_free + slot_unlock.
Signed-off-by: Barry Song <baohua@kernel.org>
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
drivers/block/zram/zram_drv.c | 39 +++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index aebc710f0d6a..0d07f0901e55 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -56,6 +56,7 @@ static size_t huge_class_size;
static const struct block_device_operations zram_devops;
static void slot_free(struct zram *zram, u32 index);
+static const struct zs_deferred_ops zram_deferred_ops;
#define slot_dep_map(zram, index) (&(zram)->table[(index)].dep_map)
static void slot_lock_init(struct zram *zram, u32 index)
@@ -1994,6 +1995,8 @@ static bool zram_meta_alloc(struct zram *zram, u64 disksize)
if (!huge_class_size)
huge_class_size = zs_huge_class_size(zram->mem_pool);
+ zs_pool_enable_deferred_free(zram->mem_pool, &zram_deferred_ops, zram);
+
for (index = 0; index < num_pages; index++)
slot_lock_init(zram, index);
@@ -2784,6 +2787,39 @@ static void zram_submit_bio(struct bio *bio)
}
}
+static enum zs_push_ret zram_deferred_push(void *buf,
+ unsigned int count, unsigned long value)
+{
+ u32 *indices = buf;
+
+ if (count >= PAGE_SIZE / sizeof(u32))
+ return ZS_PUSH_FULL;
+ indices[count] = (u32)value;
+ if (count + 1 >= PAGE_SIZE / sizeof(u32))
+ return ZS_PUSH_FULL_QUEUED;
+ return ZS_PUSH_OK;
+}
+
+static void zram_deferred_drain(void *private, void *buf, unsigned int count)
+{
+ struct zram *zram = private;
+ u32 *indices = buf;
+ unsigned int i;
+
+ for (i = 0; i < count; i++) {
+ u32 index = indices[i];
+
+ slot_lock(zram, index);
+ slot_free(zram, index);
+ slot_unlock(zram, index);
+ }
+}
+
+static const struct zs_deferred_ops zram_deferred_ops = {
+ .push = zram_deferred_push,
+ .drain = zram_deferred_drain,
+};
+
static void zram_slot_free_notify(struct block_device *bdev,
unsigned long index)
{
@@ -2792,6 +2828,9 @@ static void zram_slot_free_notify(struct block_device *bdev,
zram = bdev->bd_disk->private_data;
atomic64_inc(&zram->stats.notify_free);
+ if (zs_free_deferred(zram->mem_pool, (unsigned long)index))
+ return;
+
if (!slot_trylock(zram, index)) {
atomic64_inc(&zram->stats.miss_free);
return;
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v3 2/4] mm/zswap: use zsmalloc deferred free callback for async invalidate
From: Wenchao Hao @ 2026-05-08 6:07 UTC (permalink / raw)
To: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Yosry Ahmed
Cc: Wenchao Hao, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
Register zswap_deferred_ops to defer the entire zswap_entry_free()
to the WQ_UNBOUND worker. The invalidate hot path only stores the
entry pointer into the per-cpu buffer (512 entries/page).
The drain callback performs the full entry teardown: lru_del, zs_free,
memcg uncharge, cache_free, and stats update. On deferred failure,
fallback to synchronous zswap_entry_free().
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
mm/zswap.c | 38 +++++++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/mm/zswap.c b/mm/zswap.c
index 4b5149173b0e..3f23ddbe525c 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -270,6 +270,8 @@ static void acomp_ctx_free(struct crypto_acomp_ctx *acomp_ctx)
acomp_ctx->buffer = NULL;
}
+static const struct zs_deferred_ops zswap_deferred_ops;
+
static struct zswap_pool *zswap_pool_create(char *compressor)
{
struct zswap_pool *pool;
@@ -289,6 +291,8 @@ static struct zswap_pool *zswap_pool_create(char *compressor)
if (!pool->zs_pool)
goto error;
+ zs_pool_enable_deferred_free(pool->zs_pool, &zswap_deferred_ops, pool);
+
strscpy(pool->tfm_name, compressor, sizeof(pool->tfm_name));
/* Many things rely on the zero-initialization. */
@@ -777,6 +781,36 @@ static void zswap_entry_free(struct zswap_entry *entry)
atomic_long_dec(&zswap_stored_pages);
}
+static enum zs_push_ret zswap_deferred_push(void *buf,
+ unsigned int count, unsigned long value)
+{
+ unsigned long *entries = buf;
+
+ if (count >= PAGE_SIZE / sizeof(unsigned long))
+ return ZS_PUSH_FULL;
+ entries[count] = value;
+ if (count + 1 >= PAGE_SIZE / sizeof(unsigned long))
+ return ZS_PUSH_FULL_QUEUED;
+ return ZS_PUSH_OK;
+}
+
+static void zswap_deferred_drain(void *private, void *buf, unsigned int count)
+{
+ unsigned long *entries = buf;
+ unsigned int i;
+
+ for (i = 0; i < count; i++) {
+ struct zswap_entry *entry = (struct zswap_entry *)entries[i];
+
+ zswap_entry_free(entry);
+ }
+}
+
+static const struct zs_deferred_ops zswap_deferred_ops = {
+ .push = zswap_deferred_push,
+ .drain = zswap_deferred_drain,
+};
+
/*********************************
* compressed storage functions
**********************************/
@@ -1647,7 +1681,9 @@ void zswap_invalidate(swp_entry_t swp)
return;
entry = xa_erase(tree, offset);
- if (entry)
+ if (!entry)
+ return;
+ if (!zs_free_deferred(entry->pool->zs_pool, (unsigned long)entry))
zswap_entry_free(entry);
}
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v3 1/4] mm/zsmalloc: introduce deferred free framework with callback ops
From: Wenchao Hao @ 2026-05-08 6:07 UTC (permalink / raw)
To: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Yosry Ahmed
Cc: Wenchao Hao, Wenchao Hao
In-Reply-To: <20260508060724.3810904-1-haowenchao@xiaomi.com>
Add a per-cpu deferred free mechanism to zsmalloc with a callback
interface that lets callers (zram, zswap) customize push and drain
behavior.
Each CPU owns a single-page buffer. The hot path (zs_free_deferred)
writes a value into the current CPU's buffer via the push callback
with preemption disabled — no locks, no atomics. When the buffer
fills, it is swapped with a fresh page from a pre-allocated page
pool and the full page is queued to a WQ_UNBOUND worker for drain.
The drain worker invokes the drain callback which performs the actual
expensive work (zs_free, slot_free, etc.) in batch, away from the
original hot path.
Page pool management:
- Pool is pre-allocated at enable time (ZS_DEFERRED_POOL_SIZE pages)
- Full buffers are drained and returned to the pool
- If no free page is available when buffer is full, the push falls
back to synchronous processing by the caller
Signed-off-by: Wenchao Hao <haowenchao@xiaomi.com>
---
include/linux/zsmalloc.h | 16 +++
mm/zsmalloc.c | 208 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 223 insertions(+), 1 deletion(-)
diff --git a/include/linux/zsmalloc.h b/include/linux/zsmalloc.h
index 478410c880b1..8d6c675b10dc 100644
--- a/include/linux/zsmalloc.h
+++ b/include/linux/zsmalloc.h
@@ -24,12 +24,28 @@ struct zs_pool_stats {
struct zs_pool;
struct scatterlist;
+enum zs_push_ret {
+ ZS_PUSH_OK = 0,
+ ZS_PUSH_FULL,
+ ZS_PUSH_FULL_QUEUED,
+};
+
+struct zs_deferred_ops {
+ enum zs_push_ret (*push)(void *buf, unsigned int count,
+ unsigned long value);
+ void (*drain)(void *private, void *buf, unsigned int count);
+};
+
struct zs_pool *zs_create_pool(const char *name);
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);
+int zs_pool_enable_deferred_free(struct zs_pool *pool,
+ const struct zs_deferred_ops *ops,
+ void *private);
+bool zs_free_deferred(struct zs_pool *pool, unsigned long value);
size_t zs_huge_class_size(struct zs_pool *pool);
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 63128ddb7959..d8220a8753a7 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -196,6 +196,13 @@ struct link_free {
static struct kmem_cache *handle_cachep;
static struct kmem_cache *zspage_cachep;
+#define ZS_DEFERRED_POOL_SIZE (256 * 1024 / PAGE_SIZE)
+
+struct zs_deferred_percpu {
+ unsigned int count;
+ void *buf;
+};
+
struct zs_pool {
const char *name;
@@ -217,6 +224,18 @@ struct zs_pool {
/* protect zspage migration/compaction */
rwlock_t lock;
atomic_t compaction_in_progress;
+
+ /* per-cpu deferred free */
+ const struct zs_deferred_ops *deferred_ops;
+ void *deferred_private;
+ struct zs_deferred_percpu __percpu *deferred;
+ struct work_struct deferred_work;
+ struct workqueue_struct *deferred_wq;
+ struct list_head deferred_pool;
+ unsigned int deferred_pool_count;
+ spinlock_t deferred_pool_lock;
+ struct list_head deferred_drain_list;
+ spinlock_t deferred_drain_lock;
};
static inline void zpdesc_set_first(struct zpdesc *zpdesc)
@@ -1416,6 +1435,171 @@ void zs_free(struct zs_pool *pool, unsigned long handle)
}
EXPORT_SYMBOL_GPL(zs_free);
+static struct page *deferred_pool_get(struct zs_pool *pool)
+{
+ struct page *page = NULL;
+
+ spin_lock(&pool->deferred_pool_lock);
+ if (!list_empty(&pool->deferred_pool)) {
+ page = list_first_entry(&pool->deferred_pool, struct page, lru);
+ list_del(&page->lru);
+ pool->deferred_pool_count--;
+ }
+ spin_unlock(&pool->deferred_pool_lock);
+ return page;
+}
+
+static void deferred_pool_put(struct zs_pool *pool, struct page *page)
+{
+ spin_lock(&pool->deferred_pool_lock);
+ list_add_tail(&page->lru, &pool->deferred_pool);
+ pool->deferred_pool_count++;
+ spin_unlock(&pool->deferred_pool_lock);
+}
+
+static void zs_deferred_work_fn(struct work_struct *work)
+{
+ struct zs_pool *pool = container_of(work, struct zs_pool, deferred_work);
+ struct page *page;
+
+ while (true) {
+ unsigned int count;
+
+ spin_lock(&pool->deferred_drain_lock);
+ if (list_empty(&pool->deferred_drain_list)) {
+ spin_unlock(&pool->deferred_drain_lock);
+ break;
+ }
+ page = list_first_entry(&pool->deferred_drain_list,
+ struct page, lru);
+ list_del(&page->lru);
+ count = page_private(page);
+ spin_unlock(&pool->deferred_drain_lock);
+
+ pool->deferred_ops->drain(pool->deferred_private,
+ page_address(page), count);
+ deferred_pool_put(pool, page);
+ cond_resched();
+ }
+}
+
+bool zs_free_deferred(struct zs_pool *pool, unsigned long value)
+{
+ struct zs_deferred_percpu *def;
+ struct page *new_page, *full_page;
+ enum zs_push_ret ret;
+
+ if (!pool->deferred)
+ return false;
+
+ def = get_cpu_ptr(pool->deferred);
+
+ ret = pool->deferred_ops->push(def->buf, def->count, value);
+ if (ret == ZS_PUSH_OK) {
+ def->count++;
+ put_cpu_ptr(pool->deferred);
+ return true;
+ }
+
+ if (ret == ZS_PUSH_FULL_QUEUED)
+ def->count++;
+
+ new_page = deferred_pool_get(pool);
+ if (new_page) {
+ full_page = virt_to_page(def->buf);
+ set_page_private(full_page, def->count);
+ def->buf = page_address(new_page);
+ def->count = 0;
+
+ if (ret == ZS_PUSH_FULL) {
+ pool->deferred_ops->push(def->buf, 0, value);
+ def->count = 1;
+ }
+ put_cpu_ptr(pool->deferred);
+
+ spin_lock(&pool->deferred_drain_lock);
+ list_add_tail(&full_page->lru, &pool->deferred_drain_list);
+ spin_unlock(&pool->deferred_drain_lock);
+ queue_work(pool->deferred_wq, &pool->deferred_work);
+ return true;
+ }
+ put_cpu_ptr(pool->deferred);
+
+ /* ret==2: value already queued, will be drained eventually */
+ if (ret == 2)
+ return true;
+
+ /* ret==1: value not queued, caller must fallback */
+ return false;
+}
+EXPORT_SYMBOL_GPL(zs_free_deferred);
+
+int zs_pool_enable_deferred_free(struct zs_pool *pool,
+ const struct zs_deferred_ops *ops,
+ void *private)
+{
+ int cpu;
+ unsigned int pg_idx;
+ struct page *page, *tmp;
+
+ pool->deferred_ops = ops;
+ pool->deferred_private = private;
+
+ INIT_WORK(&pool->deferred_work, zs_deferred_work_fn);
+ pool->deferred_wq = alloc_workqueue("zs_drain", WQ_UNBOUND, 0);
+ if (!pool->deferred_wq)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(&pool->deferred_pool);
+ spin_lock_init(&pool->deferred_pool_lock);
+ pool->deferred_pool_count = 0;
+ INIT_LIST_HEAD(&pool->deferred_drain_list);
+ spin_lock_init(&pool->deferred_drain_lock);
+
+ for (pg_idx = 0; pg_idx < ZS_DEFERRED_POOL_SIZE; pg_idx++) {
+ page = alloc_page(GFP_KERNEL);
+ if (!page)
+ goto err_pages;
+ list_add_tail(&page->lru, &pool->deferred_pool);
+ pool->deferred_pool_count++;
+ }
+
+ pool->deferred = alloc_percpu(struct zs_deferred_percpu);
+ if (!pool->deferred)
+ goto err_pages;
+
+ for_each_possible_cpu(cpu) {
+ struct zs_deferred_percpu *def = per_cpu_ptr(pool->deferred, cpu);
+
+ page = deferred_pool_get(pool);
+ if (!page)
+ goto err_percpu;
+ def->buf = page_address(page);
+ def->count = 0;
+ }
+
+ return 0;
+
+err_percpu:
+ for_each_possible_cpu(cpu) {
+ struct zs_deferred_percpu *def = per_cpu_ptr(pool->deferred, cpu);
+
+ if (def->buf)
+ deferred_pool_put(pool, virt_to_page(def->buf));
+ }
+ free_percpu(pool->deferred);
+ pool->deferred = NULL;
+err_pages:
+ list_for_each_entry_safe(page, tmp, &pool->deferred_pool, lru) {
+ list_del(&page->lru);
+ __free_page(page);
+ }
+ destroy_workqueue(pool->deferred_wq);
+ pool->deferred_wq = NULL;
+ return -ENOMEM;
+}
+EXPORT_SYMBOL_GPL(zs_pool_enable_deferred_free);
+
static void zs_object_copy(struct size_class *class, unsigned long dst,
unsigned long src)
{
@@ -2182,9 +2366,31 @@ EXPORT_SYMBOL_GPL(zs_create_pool);
void zs_destroy_pool(struct zs_pool *pool)
{
- int i;
+ int i, cpu;
+ struct page *page, *tmp;
zs_unregister_shrinker(pool);
+
+ if (pool->deferred) {
+ flush_work(&pool->deferred_work);
+ for_each_possible_cpu(cpu) {
+ struct zs_deferred_percpu *def =
+ per_cpu_ptr(pool->deferred, cpu);
+
+ if (def->buf && def->count)
+ pool->deferred_ops->drain(pool->deferred_private,
+ def->buf, def->count);
+ if (def->buf)
+ deferred_pool_put(pool, virt_to_page(def->buf));
+ }
+ free_percpu(pool->deferred);
+ list_for_each_entry_safe(page, tmp, &pool->deferred_pool, lru) {
+ list_del(&page->lru);
+ __free_page(page);
+ }
+ destroy_workqueue(pool->deferred_wq);
+ }
+
zs_flush_migration(pool);
zs_pool_stat_destroy(pool);
--
2.34.1
^ permalink raw reply related
* [RFC PATCH v3 0/4] mm/zsmalloc: per-cpu deferred free to accelerate swap entry release
From: Wenchao Hao @ 2026-05-08 6:07 UTC (permalink / raw)
To: Andrew Morton, Barry Song, Chengming Zhou, Jens Axboe,
Johannes Weiner, linux-block, linux-kernel, linux-mm, Minchan Kim,
Nhat Pham, Sergey Senozhatsky, Yosry Ahmed
Cc: Wenchao Hao, 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.
This series introduces a callback-based deferred free framework in
zsmalloc. Callers (zram, zswap) register push/drain callbacks to
define what gets buffered and how it gets drained. The entire free
path including caller-side bookkeeping (slot_free, zswap_entry_free)
is deferred to a background worker.
Implementation:
- Each CPU owns a single-page buffer. The hot path writes a value
via the push callback with preemption disabled (no locks).
- When the buffer fills, it is swapped with a fresh page from a
pre-allocated page pool. The full page is queued to a WQ_UNBOUND
worker for drain.
- The drain callback performs the actual expensive work (zs_free,
slot_free, zswap_entry_free, etc.) in batch, off the hot path.
- If no free page is available, the caller falls back to synchronous
processing.
The speedup comes from moving expensive swap slot freeing off the
munmap hot path into a background worker, so that intact anonymous
folios are released back to the system without blocking. The worker
drains at a slower rate since compressed objects are small and freeing
a single handle may not release an entire page until the zspage is
fully empty.
Performance results (Raspberry Pi 4B, ARM64, 8GB RAM):
Test 1: munmap latency for 256MB swap-filled VMA (zram backend)
mode Base Patched Speedup
single 61.82ms 8.62ms 7.17x
multi 2p 94.75ms 54.11ms 1.75x
multi 3p 154.64ms 104.83ms 1.48x
Test 2: munmap latency for different sizes (zram, single process)
Size Base Patched Speedup
64MB 14.11ms 2.18ms 6.47x
128MB 29.45ms 4.48ms 6.57x
192MB 43.85ms 6.62ms 6.62x
256MB 57.01ms 9.08ms 6.28x
512MB 115.13ms 55.58ms 2.07x
1024MB 229.66ms 153.28ms 1.50x
Test 3: munmap latency for 256MB swap-filled VMA (zswap backend)
mode Base Patched Speedup
single 152.14ms 51.26ms 2.97x
multi 2p 186.56ms 105.42ms 1.77x
multi 3p 205.83ms 153.32ms 1.34x
Test 4: munmap latency for different sizes (zswap, single process)
Size Base Patched Speedup
64MB 37.83ms 13.26ms 2.85x
128MB 75.11ms 26.73ms 2.81x
256MB 150.78ms 52.97ms 2.85x
512MB 303.04ms 130.38ms 2.32x
1024MB 599.95ms 287.10ms 2.09x
[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/
Changes since v2:
- Use per-cpu single-page buffers instead of a global list; the hot
path only writes into the local CPU's buffer with preemption disabled
- Add a page pool for buffer rotation: when the current buffer is full,
swap it with a free page from the pool and queue the full page for
drain
- Introduce push/drain callback ops so that zram and zswap can each
define their own element size and drain logic (zram stores u32 slot
indices, zswap stores unsigned long handles)
- Drop the lock optimization patches it will be submitted separately
as part of a dedicated zsmalloc lock contention series
- Link to v2: https://lore.kernel.org/all/20260421121616.3298845-1-haowenchao@xiaomi.com/
Barry Song (1):
zram: use zsmalloc deferred free callback for async slot free
Wenchao Hao (3):
mm/zsmalloc: introduce deferred free framework with callback ops
mm/zswap: use zsmalloc deferred free callback for async invalidate
zram: batch clear flags in slot_free with single write
drivers/block/zram/zram_drv.c | 44 ++++++-
drivers/block/zram/zram_drv.h | 6 +
include/linux/zsmalloc.h | 16 +++
mm/zsmalloc.c | 208 +++++++++++++++++++++++++++++++++-
mm/zswap.c | 38 ++++++-
5 files changed, 306 insertions(+), 6 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH] zram: fix use-after-free in zram_writeback_endio
From: Sergey Senozhatsky @ 2026-05-08 2:40 UTC (permalink / raw)
To: Minchan Kim
Cc: Sergey Senozhatsky, Richard Chang, Jens Axboe, Andrew Morton,
bgeffon, liumartin, linux-kernel, linux-block, linux-mm
In-Reply-To: <af0YtJOLGvO-LJow@google.com>
On (26/05/07 15:56), Minchan Kim wrote:
> > - while (atomic_read(&wb_ctl->num_inflight) > 0) {
> > - wait_event(wb_ctl->done_wait, !list_empty(&wb_ctl->done_reqs));
> > + while (atomic_read(&wb_ctl->num_inflight) ||
> > + !list_empty(&wb_ctl->done_reqs)) {
> > + wait_event_timeout(wb_ctl->done_wait,
> > + !list_empty(&wb_ctl->done_reqs),
> > + HZ);
> > err = zram_complete_done_reqs(zram, wb_ctl);
> > if (err)
> > ret = err;
>
> I understand why you used a timeout here, but I still don't think it's a good
> idea since the user could wait for up to a second unnecessarily during the
> race.
Well, sure, it doesn't have to be a full HZ, we only need to wait
for propagation of atomic_dec() from another CPU. That's very fast,
orders of magniter faster than a full second. Just saying.
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index a324ede6206d..28ab4a24e77f 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -33,6 +33,7 @@
> #include <linux/cpuhotplug.h>
> #include <linux/part_stat.h>
> #include <linux/kernel_read_file.h>
> +#include <linux/kref.h>
>
> #include "zram_drv.h"
>
> @@ -504,6 +505,7 @@ struct zram_wb_ctl {
> wait_queue_head_t done_wait;
> spinlock_t done_lock;
> atomic_t num_inflight;
> + struct kref kref;
> };
Yeah okay, it overlaps with ->num_inflight, but we can live with that.
Maybe can get rod of ->num_inflight in future patches.
[..]
> @@ -864,6 +875,7 @@ static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
> atomic_set(&wb_ctl->num_inflight, 0);
> init_waitqueue_head(&wb_ctl->done_wait);
> spin_lock_init(&wb_ctl->done_lock);
> + kref_init(&wb_ctl->kref);
>
> for (i = 0; i < zram->wb_batch_size; i++) {
> struct zram_wb_req *req;
> @@ -985,6 +997,7 @@ static void zram_writeback_endio(struct bio *bio)
> spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
>
> wake_up(&wb_ctl->done_wait);
> + kref_put(&wb_ctl->kref, release_wb_ctl_kref);
> }
>
>
> static void zram_submit_wb_request(struct zram *zram,
> @@ -996,6 +1009,7 @@ static void zram_submit_wb_request(struct zram *zram,
> * so that we don't over-submit.
> */
> zram_account_writeback_submit(zram);
> + kref_get(&wb_ctl->kref);
> atomic_inc(&wb_ctl->num_inflight);
> req->bio.bi_private = wb_ctl;
> submit_bio(&req->bio);
> @@ -1276,8 +1290,8 @@ static ssize_t writeback_store(struct device *dev,
>
> wb_ctl = init_wb_ctl(zram);
> if (!wb_ctl) {
> - ret = -ENOMEM;
> - goto out;
> + release_pp_ctl(zram, pp_ctl);
> + return -ENOMEM;
> }
>
> args = skip_spaces(buf);
So I think we also need to do kref_put(&wb_ctl->kref, release_wb_ctl_kref)
at the end of writeback_store(), because otherwise it just kfree()
wb_ctl and we have the same race condition:
@@ -1330,7 +1340,7 @@ static ssize_t writeback_store(struct device *dev,
out:
release_pp_ctl(zram, pp_ctl);
- release_wb_ctl(wb_ctl);
+ kref_put(&wb_ctl->kref, release_wb_ctl_kref);
return ret;
}
And indirect release in init_wb_ctl() as well:
@@ -895,7 +903,7 @@ static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
return wb_ctl;
release_wb_ctl:
- release_wb_ctl(wb_ctl);
+ kref_put(&wb_ctl->kref, release_wb_ctl_kref);
return NULL;
}
^ permalink raw reply
* Re: [PATCH] zram: fix use-after-free in zram_writeback_endio
From: Minchan Kim @ 2026-05-07 23:38 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Richard Chang, Jens Axboe, Andrew Morton, bgeffon, liumartin,
linux-kernel, linux-block, linux-mm
In-Reply-To: <af0YtJOLGvO-LJow@google.com>
On Thu, May 07, 2026 at 03:56:52PM -0700, Minchan Kim wrote:
> On Thu, May 07, 2026 at 06:40:37PM +0900, Sergey Senozhatsky wrote:
> > On (26/05/05 09:37), Minchan Kim wrote:
> > > > @@ -966,9 +966,8 @@ static void zram_writeback_endio(struct bio *bio)
> > > >
> > > > spin_lock_irqsave(&wb_ctl->done_lock, flags);
> > > > list_add(&req->entry, &wb_ctl->done_reqs);
> > > > - spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> > > > -
> > > > wake_up(&wb_ctl->done_wait);
> > > > + spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> > > > }
> > > >
> > >
> > > I agree this will fix the issue, but using a lock to extend the lifetime of
> > > an object to avoid a UAF is not a good pattern. Object lifetime shared between
> > > process and interrupt contexts should be managed explicitly using refcount.
> >
> > ->num_inflight is a ref-counter, basically. The problem is that
> > completion is a two-step process, only one part of each is synchronized
> > with the writeback context. I honestly don't want to have two ref-counts:
> > one for requests pending zram completion and one for active endio contexts.
> > Maybe we can repurpose num_inflight instead.
>
> If it can make the code much clearer and simpler, I have no objection.
>
> >
> > > Furthermore, keeping wake_up() outside the critical section minimizes
> > > interrupt-disabled latency
> >
> > So I considered that, but isn't endio already called from IRQ context?
> > Just asking. We wakeup only one waiter (writeback task), so it's not
> > that bad CPU-cycles wise. Do you think it's really a concern?
>
> I don't think it will have any measurable impact; I was just pointing out
> a theoretical one.
>
> >
> > wake_up() under spin-lock solves the problem of a unsynchronized
> > two-stages endio process.
> >
> > > and avoids nesting spinlocks (done_lock -> done_wait.lock), reducing
> > > the risk of future lockdep issues, just in case.
> >
> > I considered lockdep as well but ruled it out as impossible scenario,
> > nesting here is strictly uni-directional, we never call into zram from
> > the scheduler. Just saying.
>
> Sure. I just prefer to avoid adding more lock dependencies without a strong
> justification, to prevent potential locking issues in the future.
>
> >
> > > It definitely will add more overhead for the submission/completion paths to deal
> > > with the refcount, but I think we should go that way at the cost of runtime.
> >
> > Dunno, something like below maybe?
> >
> > ---
> > drivers/block/zram/zram_drv.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index ce2e1c79fc75..27fe50d666d7 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -967,7 +967,7 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
> > static void zram_writeback_endio(struct bio *bio)
> > {
> > struct zram_wb_req *req = container_of(bio, struct zram_wb_req, bio);
> > - struct zram_wb_ctl *wb_ctl = bio->bi_private;
> > + struct zram_wb_ctl *wb_ctl = READ_ONCE(bio->bi_private);
> > unsigned long flags;
> >
> > spin_lock_irqsave(&wb_ctl->done_lock, flags);
> > @@ -975,6 +975,7 @@ static void zram_writeback_endio(struct bio *bio)
> > spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> >
> > wake_up(&wb_ctl->done_wait);
> > + atomic_dec(&wb_ctl->num_inflight);
> > }
> >
> > static void zram_submit_wb_request(struct zram *zram,
> > @@ -998,7 +999,7 @@ static int zram_complete_done_reqs(struct zram *zram,
> > unsigned long flags;
> > int ret = 0, err;
> >
> > - while (atomic_read(&wb_ctl->num_inflight) > 0) {
> > + for (;;) {
> > spin_lock_irqsave(&wb_ctl->done_lock, flags);
> > req = list_first_entry_or_null(&wb_ctl->done_reqs,
> > struct zram_wb_req, entry);
> > @@ -1006,7 +1007,6 @@ static int zram_complete_done_reqs(struct zram *zram,
> > list_del(&req->entry);
> > spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> >
> > - /* ->num_inflight > 0 doesn't mean we have done requests */
> > if (!req)
> > break;
> >
> > @@ -1014,7 +1014,6 @@ static int zram_complete_done_reqs(struct zram *zram,
> > if (err)
> > ret = err;
> >
> > - atomic_dec(&wb_ctl->num_inflight);
> > release_pp_slot(zram, req->pps);
> > req->pps = NULL;
> >
> > @@ -1129,8 +1128,11 @@ static int zram_writeback_slots(struct zram *zram,
> > if (req)
> > release_wb_req(req);
> >
> > - while (atomic_read(&wb_ctl->num_inflight) > 0) {
> > - wait_event(wb_ctl->done_wait, !list_empty(&wb_ctl->done_reqs));
> > + while (atomic_read(&wb_ctl->num_inflight) ||
> > + !list_empty(&wb_ctl->done_reqs)) {
> > + wait_event_timeout(wb_ctl->done_wait,
> > + !list_empty(&wb_ctl->done_reqs),
> > + HZ);
> > err = zram_complete_done_reqs(zram, wb_ctl);
> > if (err)
> > ret = err;
>
> I understand why you used a timeout here, but I still don't think it's a good
> idea since the user could wait for up to a second unnecessarily during the
> race.
>
> What I prefer is simple and explicit lifetime management for wb_ctl using
> refcount. It directly addresses the core issue (UAF of wb_ctl) in a standard,
> robust way without needing workarounds like timeouts. The runtime overhead
> of kref will be negligible.
>
The other standard way to deal with lifetime is RCU.
How about this?
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a324ede6206d..28ab4a24e77f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -33,6 +33,7 @@
#include <linux/cpuhotplug.h>
#include <linux/part_stat.h>
#include <linux/kernel_read_file.h>
+#include <linux/rcupdate.h>
#include "zram_drv.h"
@@ -504,6 +505,7 @@ struct zram_wb_ctl {
wait_queue_head_t done_wait;
spinlock_t done_lock;
atomic_t num_inflight;
+ struct rcu_head rcu;
};
struct zram_wb_req {
@@ -829,14 +831,8 @@ static void release_wb_req(struct zram_wb_req *req)
kfree(req);
}
static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
{
- if (!wb_ctl)
- return;
-
/* We should never have inflight requests at this point */
WARN_ON(atomic_read(&wb_ctl->num_inflight));
WARN_ON(!list_empty(&wb_ctl->done_reqs));
@@ -850,7 +849,7 @@ static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
release_wb_req(req);
}
- kfree(wb_ctl);
+ kfree_rcu(wb_ctl, rcu);
}
static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
@@ -985,6 +997,7 @@ static void zram_writeback_endio(struct bio *bio)
struct zram_wb_ctl *wb_ctl = bio->bi_private;
unsigned long flags;
+ rcu_read_lock();
spin_lock_irqsave(&wb_ctl->done_lock, flags);
list_add(&req->entry, &wb_ctl->done_reqs);
spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
@@ -991,5 +1004,6 @@ static void zram_writeback_endio(struct bio *bio)
wake_up(&wb_ctl->done_wait);
+ rcu_read_unlock();
}
static void zram_submit_wb_request(struct zram *zram,
@@ -1276,8 +1290,8 @@ static ssize_t writeback_store(struct device *dev,
wb_ctl = init_wb_ctl(zram);
if (!wb_ctl) {
- ret = -ENOMEM;
- goto out;
+ release_pp_ctl(zram, pp_ctl);
+ return -ENOMEM;
}
args = skip_spaces(buf);
^ permalink raw reply related
* Re: [PATCH] zram: fix use-after-free in zram_writeback_endio
From: Minchan Kim @ 2026-05-07 22:56 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Richard Chang, Jens Axboe, Andrew Morton, bgeffon, liumartin,
linux-kernel, linux-block, linux-mm
In-Reply-To: <afw2919RiZje9xzq@google.com>
On Thu, May 07, 2026 at 06:40:37PM +0900, Sergey Senozhatsky wrote:
> On (26/05/05 09:37), Minchan Kim wrote:
> > > @@ -966,9 +966,8 @@ static void zram_writeback_endio(struct bio *bio)
> > >
> > > spin_lock_irqsave(&wb_ctl->done_lock, flags);
> > > list_add(&req->entry, &wb_ctl->done_reqs);
> > > - spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> > > -
> > > wake_up(&wb_ctl->done_wait);
> > > + spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
> > > }
> > >
> >
> > I agree this will fix the issue, but using a lock to extend the lifetime of
> > an object to avoid a UAF is not a good pattern. Object lifetime shared between
> > process and interrupt contexts should be managed explicitly using refcount.
>
> ->num_inflight is a ref-counter, basically. The problem is that
> completion is a two-step process, only one part of each is synchronized
> with the writeback context. I honestly don't want to have two ref-counts:
> one for requests pending zram completion and one for active endio contexts.
> Maybe we can repurpose num_inflight instead.
If it can make the code much clearer and simpler, I have no objection.
>
> > Furthermore, keeping wake_up() outside the critical section minimizes
> > interrupt-disabled latency
>
> So I considered that, but isn't endio already called from IRQ context?
> Just asking. We wakeup only one waiter (writeback task), so it's not
> that bad CPU-cycles wise. Do you think it's really a concern?
I don't think it will have any measurable impact; I was just pointing out
a theoretical one.
>
> wake_up() under spin-lock solves the problem of a unsynchronized
> two-stages endio process.
>
> > and avoids nesting spinlocks (done_lock -> done_wait.lock), reducing
> > the risk of future lockdep issues, just in case.
>
> I considered lockdep as well but ruled it out as impossible scenario,
> nesting here is strictly uni-directional, we never call into zram from
> the scheduler. Just saying.
Sure. I just prefer to avoid adding more lock dependencies without a strong
justification, to prevent potential locking issues in the future.
>
> > It definitely will add more overhead for the submission/completion paths to deal
> > with the refcount, but I think we should go that way at the cost of runtime.
>
> Dunno, something like below maybe?
>
> ---
> drivers/block/zram/zram_drv.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index ce2e1c79fc75..27fe50d666d7 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -967,7 +967,7 @@ static int zram_writeback_complete(struct zram *zram, struct zram_wb_req *req)
> static void zram_writeback_endio(struct bio *bio)
> {
> struct zram_wb_req *req = container_of(bio, struct zram_wb_req, bio);
> - struct zram_wb_ctl *wb_ctl = bio->bi_private;
> + struct zram_wb_ctl *wb_ctl = READ_ONCE(bio->bi_private);
> unsigned long flags;
>
> spin_lock_irqsave(&wb_ctl->done_lock, flags);
> @@ -975,6 +975,7 @@ static void zram_writeback_endio(struct bio *bio)
> spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
>
> wake_up(&wb_ctl->done_wait);
> + atomic_dec(&wb_ctl->num_inflight);
> }
>
> static void zram_submit_wb_request(struct zram *zram,
> @@ -998,7 +999,7 @@ static int zram_complete_done_reqs(struct zram *zram,
> unsigned long flags;
> int ret = 0, err;
>
> - while (atomic_read(&wb_ctl->num_inflight) > 0) {
> + for (;;) {
> spin_lock_irqsave(&wb_ctl->done_lock, flags);
> req = list_first_entry_or_null(&wb_ctl->done_reqs,
> struct zram_wb_req, entry);
> @@ -1006,7 +1007,6 @@ static int zram_complete_done_reqs(struct zram *zram,
> list_del(&req->entry);
> spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
>
> - /* ->num_inflight > 0 doesn't mean we have done requests */
> if (!req)
> break;
>
> @@ -1014,7 +1014,6 @@ static int zram_complete_done_reqs(struct zram *zram,
> if (err)
> ret = err;
>
> - atomic_dec(&wb_ctl->num_inflight);
> release_pp_slot(zram, req->pps);
> req->pps = NULL;
>
> @@ -1129,8 +1128,11 @@ static int zram_writeback_slots(struct zram *zram,
> if (req)
> release_wb_req(req);
>
> - while (atomic_read(&wb_ctl->num_inflight) > 0) {
> - wait_event(wb_ctl->done_wait, !list_empty(&wb_ctl->done_reqs));
> + while (atomic_read(&wb_ctl->num_inflight) ||
> + !list_empty(&wb_ctl->done_reqs)) {
> + wait_event_timeout(wb_ctl->done_wait,
> + !list_empty(&wb_ctl->done_reqs),
> + HZ);
> err = zram_complete_done_reqs(zram, wb_ctl);
> if (err)
> ret = err;
I understand why you used a timeout here, but I still don't think it's a good
idea since the user could wait for up to a second unnecessarily during the
race.
What I prefer is simple and explicit lifetime management for wb_ctl using
refcount. It directly addresses the core issue (UAF of wb_ctl) in a standard,
robust way without needing workarounds like timeouts. The runtime overhead
of kref will be negligible.
Something like this:
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index a324ede6206d..28ab4a24e77f 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -33,6 +33,7 @@
#include <linux/cpuhotplug.h>
#include <linux/part_stat.h>
#include <linux/kernel_read_file.h>
+#include <linux/kref.h>
#include "zram_drv.h"
@@ -504,6 +505,7 @@ struct zram_wb_ctl {
wait_queue_head_t done_wait;
spinlock_t done_lock;
atomic_t num_inflight;
+ struct kref kref;
};
struct zram_wb_req {
@@ -829,11 +831,8 @@ static void release_wb_req(struct zram_wb_req *req)
kfree(req);
}
-static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
+static void __release_wb_ctl(struct zram_wb_ctl *wb_ctl)
{
- if (!wb_ctl)
- return;
-
/* We should never have inflight requests at this point */
WARN_ON(atomic_read(&wb_ctl->num_inflight));
WARN_ON(!list_empty(&wb_ctl->done_reqs));
@@ -850,6 +849,18 @@ static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
kfree(wb_ctl);
}
+static void release_wb_ctl_kref(struct kref *kref)
+{
+ struct zram_wb_ctl *wb_ctl = container_of(kref, struct zram_wb_ctl, kref);
+
+ __release_wb_ctl(wb_ctl);
+}
+
+static void release_wb_ctl(struct zram_wb_ctl *wb_ctl)
+{
+ kref_put(&wb_ctl->kref, release_wb_ctl_kref);
+}
+
static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
{
struct zram_wb_ctl *wb_ctl;
@@ -864,6 +875,7 @@ static struct zram_wb_ctl *init_wb_ctl(struct zram *zram)
atomic_set(&wb_ctl->num_inflight, 0);
init_waitqueue_head(&wb_ctl->done_wait);
spin_lock_init(&wb_ctl->done_lock);
+ kref_init(&wb_ctl->kref);
for (i = 0; i < zram->wb_batch_size; i++) {
struct zram_wb_req *req;
@@ -985,6 +997,7 @@ static void zram_writeback_endio(struct bio *bio)
spin_unlock_irqrestore(&wb_ctl->done_lock, flags);
wake_up(&wb_ctl->done_wait);
+ kref_put(&wb_ctl->kref, release_wb_ctl_kref);
}
static void zram_submit_wb_request(struct zram *zram,
@@ -996,6 +1009,7 @@ static void zram_submit_wb_request(struct zram *zram,
* so that we don't over-submit.
*/
zram_account_writeback_submit(zram);
+ kref_get(&wb_ctl->kref);
atomic_inc(&wb_ctl->num_inflight);
req->bio.bi_private = wb_ctl;
submit_bio(&req->bio);
@@ -1276,8 +1290,8 @@ static ssize_t writeback_store(struct device *dev,
wb_ctl = init_wb_ctl(zram);
if (!wb_ctl) {
- ret = -ENOMEM;
- goto out;
+ release_pp_ctl(zram, pp_ctl);
+ return -ENOMEM;
}
args = skip_spaces(buf);
^ permalink raw reply related
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