summaryrefslogtreecommitdiffstats
path: root/mm/rmap.c
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2022-10-29 11:45:07 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2022-11-02 16:10:30 -0700
commit28154ddc676efa64e8e792389787eb85199d2772 (patch)
tree6bf1a0ba9189c8e904a91b71e01ec5e44651c8a3 /mm/rmap.c
parent655d4bdee63563392b0e5fb40f973c6d41658070 (diff)
downloadlinux-28154ddc676efa64e8e792389787eb85199d2772.tar.bz2
mm: delay rmap removal until after TLB flushmmu_gather-race-fix
When we remove a page table entry, we are very careful to only free the page after we have flushed the TLB, because other CPUs could still be using the page through stale TLB entries until after the flush. However, we have removed the rmap entry for that page early, which means that functions like folio_mkclean() would end up not serializing with the page table lock because the page had already been made invisible to rmap. And that is a problem, because while the TLB entry exists, we could end up with the following situation: (a) one CPU could come in and clean it, never seeing our mapping of the page (b) another CPU could continue to use the stale and dirty TLB entry and continue to write to said page resulting in a page that has been dirtied, but then marked clean again, all while another CPU might have dirtied it some more. End result: possibly lost dirty data. This commit uses the same old TLB gather array that we use to delay the freeing of the page to also say 'remove from rmap after flush', so that we can keep the rmap entries alive until all TLB entries have been flushed. It might be worth noting that this means that the page_zap_pte_rmap() is now called outside the page table lock. That was never mutual exclusion (since the same page could be mapped under multiple different page tables), but it does mean that it needs to use the more careful version of dec_lruvec_page_state() that doesn't depend on being called in a non-preemptable context. NOTE! While the "possibly lost dirty data" sounds catastrophic, for this all to happen you need to have a user thread doing either madvise() with MADV_DONTNEED or a full re-mmap() of the area concurrently with another thread continuing to use said mapping. So arguably this is about user space doing crazy things, but from a VM consistency standpoint it's better if we track the dirty bit properly even when user space goes off the rails. Reported-by: Nadav Amit <nadav.amit@gmail.com> Link: https://lore.kernel.org/all/B88D3073-440A-41C7-95F4-895D3F657EF2@gmail.com/ Cc: Will Deacon <will@kernel.org> Cc: Aneesh Kumar <aneesh.kumar@linux.ibm.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Nick Piggin <npiggin@gmail.com> Cc: Heiko Carstens <hca@linux.ibm.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Alexander Gordeev <agordeev@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@linux.ibm.com> Cc: Sven Schnelle <svens@linux.ibm.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Gerald Schaefer <gerald.schaefer@linux.ibm.com> # s390 Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm/rmap.c')
-rw-r--r--mm/rmap.c4
1 files changed, 1 insertions, 3 deletions
diff --git a/mm/rmap.c b/mm/rmap.c
index 28b51a31ebb0..416b7078b75f 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1422,8 +1422,6 @@ static void page_remove_anon_compound_rmap(struct page *page)
* separately.
*
* This allows for a much simpler calling convention and code.
- *
- * The caller holds the pte lock.
*/
void page_zap_pte_rmap(struct page *page)
{
@@ -1431,7 +1429,7 @@ void page_zap_pte_rmap(struct page *page)
return;
lock_page_memcg(page);
- __dec_lruvec_page_state(page,
+ dec_lruvec_page_state(page,
PageAnon(page) ? NR_ANON_MAPPED : NR_FILE_MAPPED);
unlock_page_memcg(page);
}