Re: [PATCH mm-hotfixes] mm/zswap: add missing kunmap_local()

[Yosry Ahmed]

On Tue, Mar 17, 2026 at 5:59 AM Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx> wrote:
>
> Hi Andrew,
>
> Please apply the fix-patch enclosed to add a dcache flush which was also
> missing here.
>
> I had assumed that a new folio here combined with the flush that is done at
> the point of setting the PTE would suffice, but it doesn't seem that's
> actually the case, as update_mmu_cache() will in many archtectures only
> actually flush entries where a dcache flush was done on a range previously.
>
> I had also wondered whether kunmap_local() might suffice, but it doesn't
> seem to be the case.
>
> Some arches do seem to actually dcache flush on unmap, parisc does it if
> CONFIG_HIGHMEM is not set by setting ARCH_HAS_FLUSH_ON_KUNMAP and calling
> kunmap_flush_on_unmap() from __kunmap_local(), otherwise non-CONFIG_HIGHMEM
> callers do nothing here.
>
> Otherwise arch_kmap_local_pre_unmap() is called which does:
>
> * sparc - flush_cache_all()
> * arm - if VIVT, __cpuc_flush_dcache_area()
> * otherwise - nothing
>
> Also arch_kmap_local_post_unmap() is called which does:
>
> * arm - local_flush_tlb_kernel_page()
> * csky - kmap_flush_tlb()
> * microblaze, ppc - local_flush_tlb_page()
> * mips - local_flush_tlb_one()
> * sparc - flush_tlb_all() (again)
> * x86 - arch_flush_lazy_mmu_mode()
> * otherwise - nothing
>
> But this is only if it's high memory, and doesn't cover all architectures,
> so is presumably intended to handle other cache consistency concerns.
>
> In any case, VIPT is problematic here whether low or high memory (in spite
> of what the documentation claims, see [0] - 'the kernel did write to a page
> that is in the page cache page and / or in high memory'), because dirty
> cache lines may exist at the set indexed by the kernel direct mapping,
> which won't exist in the set indexed by any subsequent userland mapping,
> meaning userland might read stale data from L2 cache.
>
> Even if the documentation is correct and low memory is fine not to be
> flushed here, we can't be sure as to whether the memory is low or high
> (kmap_local_folio() will be a no-op if low), and this call should be
> harmless if it is low.
>
> VIVT would require more work if the memory were shared and already mapped,
> but this isn't the case here, and would anyway be handled by the dcache
> flush call.
>
> In any case, we definitely need this flush as far as I can tell.
>
> And we should probably consider updating the documentation unless it turns
> out there's somehow dcache synchronisation that happens for low
> memory/64-bit kernels elsewhere?

Yeah the documentation is unclear to me too. Given that the flush
existed before, I agree that we should re-add it here even if it's
just cautionary tbh. This feels like something that should be handled
by the kmap API or memcpy_from_sglist() though, sounds like it's easy
to get it wrong.

>
> Thanks, Lorenzo
>
> [0]:https://docs.kernel.org/core-api/cachetlb.html
>
> ----8<----
> From 94989ab3d97f0cde0dcf59eba6466fee932ec4ba Mon Sep 17 00:00:00 2001
> From: "Lorenzo Stoakes (Oracle)" <ljs@xxxxxxxxxx>
> Date: Tue, 17 Mar 2026 12:29:55 +0000
> Subject: [PATCH] fix
>
> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@xxxxxxxxxx>
> ---
>  mm/zswap.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 499520f65ff0..16b2ef7223e1 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -950,6 +950,7 @@ static bool zswap_decompress(struct zswap_entry *entry, struct folio *folio)
>                 memcpy_from_sglist(dst, input, 0, PAGE_SIZE);
>                 dlen = PAGE_SIZE;
>                 kunmap_local(dst);
> +               flush_dcache_folio(folio);
>         } else {
>                 sg_init_table(&output, 1);
>                 sg_set_folio(&output, folio, PAGE_SIZE, 0);
> --
> 2.53.0

Acked-by: Yosry Ahmed <yosry@xxxxxxxxxx>

Do you mind sending Andrew an updated changelog capturing the
reasoning here, I'd hate for that to be lost. Or maybe a v2 including
the full patch and updated changelog. Up to you/Andrew.

Thank you!