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

[SeongJae Park]

On Tue, 17 Mar 2026 12:59:35 +0000 "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.

Makes sense, thank you for detailed description!

> 
> 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?
> 
> 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>

Reviewed-by: SeongJae Park <sj@xxxxxxxxxx>


Thanks,
SJ

[...]