mm: various (urgent?) fixes
Subsystems (27)
David Hildenbrand (Arm)
maintainerv3 cover mm: various (urgent?) fixes
Thanks to everyone for the review! Hoping I can get some more? :)
A quick resend, I'll queue these immediately after sending to get them into -next asap. Don't let that discourage you from reviewing ;)
---
Changes in v3:
- Replace "mm/memfd: check for real shmem inodes before accessing shmem state" by "mm/shmem: make shmem_file() check the file's inode"
- "mm/huge_memory: fix zapping migration entries for device-private THPs": drop "!is_present" check
- Pick up tags
- Link to v2: https://patch.msgid.link/[email protected]
Changes in v2:
- "mm/memfd: check for real shmem inodes before accessing shmem state"
- Call it inode_is_shmem
- "mm/truncate: look again if a folio left the mapping"
- Different fix for truncate_inode_pages_range
- Adjust description
- "mm/huge_memory: fix zapping migration entries for device-private THPs"
- Move handling in zap_deposited_table
- "mm/mmu_gather: fix flushing ranges with 4 GiB or larger"
- Adjust description"
- "mm/huge_memory: split huge PUDs when adjusting VMA boundaries"
- Drop split_pud check
- Link to v1: https://patch.msgid.link/[email protected]
---
Chris Mason (11):
mm/huge_memory: fix truncate racing with PMD zapping mm/huge_memory: fix zapping migration entries for device-private THPs mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs mm/gup: fix clearing folios in memfd_pin_folios()
mm/gup: fix out-of-bounds write for nr_pages == 0 in GUP-fast mm/mmu_notifier: set ->mm only after mmgrab()
mm/memory: reject remote COW faults on PFN mappings
mm: fix unkillable fault loop on MAP_DROPPABLE memory
mm: fix deadlock of mremap() against UFFDIO_MOVE
mm/ksm: check large swap-cache folios before mapping them mm/vma: call ->may_split() before a partial merge
David Hildenbrand (Arm) (20):
fork: take mmap_lock before copying mm_struct
mm/shmem: make shmem_file() check the file's inode
mm/truncate: look again if a folio left the mapping
mm/filemap: finish fault cleanup before dropping the fault lock
mm/rmap: handle device-exclusive entries in folio_referenced_one() mm/huge_memory: fix list corruption when deferred_split_scan() races with deferred_split_folio() mm/huge_memory: properly handle refcounted PUD mappings on fork
mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp() mm/gup: recheck the PUD under PTL in follow_pud_mask()
mm/gup: check for overflow when converting nr_pages to bytes in GUP-fast mm/userfaultfd: write-lock the VMA before waking waiters when unregistering mm/userfaultfd: wake waiters when registration modes are removed
mm/mremap: use pudp_huge_get_and_clear() when moving huge PUDs
mm/mremap: recheck PUDs under the page table lock
mm/madvise: fix data loss from MADV_FREE on a KSM page
mm/ksm: reset the unstable trees of all nodes
mm/ksm: fix use of a freed anon_vma at swap-in
mm/huge_memory: split huge PUDs when adjusting VMA boundaries
mm/vma: don't leak the charge for stack growth
mm/memory-tiers: unregister empty tiers outside memory_tier_lock
Gregory Price (1):
mm/userfaultfd: compare exact registration modes before skipping
Josef Bacik (14):
mm/mglru: don't use stack pointers when walking PUD and PMD tables mm/memfd: make f_pos accesses atomic
mm/filemap: read mapping->a_ops->free_folio under i_lock mm/filemap: don't lose a folio lock wakeup to a signal mm/huge_memory: fix livelock on device private folio mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp() mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp() mm/userfaultfd: wake all waiters on unregister mm/mmu_gather: fix flushing ranges with 4 GiB or larger mm/mmu_notifier: finish non-blocking interval invalidation on error mm/mremap: fix moving PUD leaves
mm/memory_hotplug: avoid races in pfn_to_online_page() with dying pgmap
mm: fix unmap_mapping_range() beyond 4 GiB on 32-bit fs/proc/task_mmu: refuse addresses beyond 32 bits on 32bit
Rik van Riel (1):
mm/cma: don't release CMA pages still in use
fs/proc/task_mmu.c | 6 ++ include/asm-generic/tlb.h | 3 +- include/linux/huge_mm.h | 5 +- include/linux/ksm.h | 6 +- include/linux/memremap.h | 6 ++ include/linux/mmu_notifier.h | 3 +- include/linux/shmem_fs.h | 15 ++--- include/linux/userfaultfd_k.h | 2 - kernel/fork.c | 20 ++++-- mm/cma.c | 10 ++- mm/filemap.c | 88 ++++++++++++------------- mm/gup.c | 53 ++++++++++----- mm/huge_memory.c | 146 +++++++++++++++++++++++++++++++----------- mm/hugetlb.c | 14 +--- mm/internal.h | 28 ++++++++ mm/khugepaged.c | 79 +++++++++++------------ mm/ksm.c | 56 ++++++++++++---- mm/madvise.c | 9 +++ mm/memfd.c | 2 + mm/memory-tiers.c | 29 ++++----- mm/memory.c | 30 ++++++--- mm/memory_hotplug.c | 13 ++-- mm/memremap.c | 13 ++++ mm/mmap.c | 8 +-- mm/mmu_notifier.c | 24 +++++-- mm/mremap.c | 103 +++++++++++++---------------- mm/nommu.c | 2 +- mm/rmap.c | 23 +++++-- mm/shmem.c | 5 ++ mm/swapfile.c | 3 +- mm/truncate.c | 13 +++- mm/userfaultfd.c | 75 ++++++++++------------ mm/util.c | 13 ++++ mm/vma.c | 24 +++++-- mm/vmscan.c | 4 +- 35 files changed, 580 insertions(+), 353 deletions(-)
---
base-commit: 65ba8f34ec886ec72b2c07ddc8cf3d815c32869d
change-id: 20261021-fixes_core-bdf638faf023
--
Cheers,
David
David Hildenbrand (Arm)
maintainerPATCH v3 1/47 mm/mglru: don't use stack pointers when walking PUD and PMD tables
From: Josef Bacik <[email protected]>
MGLRU passes stack copies of page-table entries in walk_pud_range() and walk_pmd_range() when walking to the next level. These copies can become stale when khugepaged or MADV_DONTNEED (CONFIG_PT_RECLAIM) removes a PTE table concurrently.
This pattern dates back to commit bd74fdaea146 ("mm: multi-gen LRU:
support page table walks"). Likely, earlier locking masked the latent bug. Later changes exposed it: commit 1d65b771bc08 ("mm/khugepaged:retract_page_tables() without mmap or vma lock") allowed concurrent retraction, and commit cf4a65539c13 ("memcg-v1: no need for memcg locking for MGLRU") dropped the walker's outer RCU protection.
The fix is simple, so just fix the original commit so we don't have to speculate. To fix it, pass the actual PMD/PUD pointers. This restores the RCU-protected lookup and validation against the live PMD. Note that the PUD handling is required as well to handle folded page tables correctly in all cases.
Fix generated by an LLM and sent by Josef Bacik in private.
[ david: handle walk_pud_range() as well, clarify+simplify description,
adjust Fixes: ]
--- mm/vmscan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/vmscan.c +2 -2
@@ -3911,7 +3911,7 @@ static void walk_pmd_range(pud_t *pud, unsigned long start, unsigned long end, walk->mm_stats[MM_NONLEAF_FOUND ]++; -if (!walk_pte_range (&val, addr, next, args)) +if (!walk_pte_range (pmd + i, addr, next, args))continue ; walk->mm_stats[MM_NONLEAF_ADDED ]++;
@@ -3947,7 +3947,7 @@ static int walk_pud_range(p4d_t *p4d, unsigned long start, unsigned long end,if (!pud_present (val) ||WARN_ON_ONCE (pud_leaf (val)))continue ; -walk_pmd_range (&val, addr, next, args); +walk_pmd_range (pud + i, addr, next, args);if (need_resched () || walk->batched >=MAX_LRU_BATCH ) { end = (addr | ~PUD_MASK ) +1 ;
Signature · 1 line
2.43.0
Re: [PATCH v3 01/47] mm/mglru: don't use stack pointers when walking PUD and PMD tables
Quoted 41 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
MGLRU passes stack copies of page-table entries in
walk_pud_range()andwalk_pmd_range()when walking to the next level. These copies can become stale when khugepaged orMADV_DONTNEED(CONFIG_PT_RECLAIM) removes a PTE table concurrently.This pattern dates back to commit
bd74fdaea146("mm: multi-gen LRU:
support page table walks"). Likely, earlier locking masked the latent bug. Later changes exposed it: commit1d65b771bc08("mm/khugepaged:retract_page_tables()without mmap or vma lock") allowed concurrent retraction, and commitcf4a65539c13("memcg-v1: no need for memcg locking for MGLRU") dropped the walker's outer RCU protection.The fix is simple, so just fix the original commit so we don't have to speculate. To fix it, pass the actual PMD/PUD pointers. This restores the RCU-protected lookup and validation against the live PMD. Note that the PUD handling is required as well to handle folded page tables correctly in all cases.
Fix generated by an LLM and sent by Josef Bacik in private.
[ david: handle
walk_pud_range()as well, clarify+simplify description,
adjust Fixes: ]Fixes bd74fdaea146 ("mm: multi-gen LRU: support page table walks")Signed-off-by Josef Bacik <[email protected]>Reviewed-by Rik van Riel <[email protected]>Reviewed-by Qi Zheng <[email protected]>Reviewed-by Dev Jain <[email protected]>Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reviewed-by Kairui Song <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Reviewed-by Harry Yoo (Meta) <[email protected]>Reviewed-by Barry Song <[email protected]>Reviewed-by Baolin Wang <[email protected]>Reviewed-by Pedro Falcato <[email protected]>Reviewed-by Kiryl Shutsemau (Meta) <[email protected]>Reviewed-by SJ Park <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 3 lines
--- mm/vmscan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
LGTM,
Quoted 27 lines
mm/vmscan.c +2
-2@@ -3911,7 +3911,7 @@ static void walk_pmd_range(pud_t *pud, unsigned long start, unsigned long end, walk->mm_stats[MM_NONLEAF_FOUND ]++; -if (!walk_pte_range (&val, addr, next, args)) +if (!walk_pte_range (pmd + i, addr, next, args))continue ; walk->mm_stats[MM_NONLEAF_ADDED ]++;@@ -3947,7 +3947,7 @@ static int walk_pud_range(p4d_t *p4d, unsigned long start, unsigned long end,if (!pud_present (val) ||WARN_ON_ONCE (pud_leaf (val)))continue ; -walk_pmd_range (&val, addr, next, args); +walk_pmd_range (pud + i, addr, next, args);if (need_resched () || walk->batched >=MAX_LRU_BATCH ) { end = (addr | ~PUD_MASK ) +1 ;Signature · 2 lines
-- 2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 2/47 fork: take mmap_lock before copying mm_struct
In reply to David Hildenbrand (Arm)dup_mm() copies the parent's mm_struct before dup_mmap() takes the mmap_lock. A concurrent brk() can leave the child with a stale mm->brk and newer heap mappings, causing later brk() requests to fail.
Other operations (e.g., PR_SET_MM) on the parent's mm_struct can similarly cause inconsistencies.
As a simple and robust fix, grab the parent's mmap_lock earlier. This way we don't need to update individual mm fields in dup_mmap() and keep missing ones. Stop updating the 4 existing members (total_vm, data_vm, exec_vm, stack_vm), as that is now no longer required.
Keep releasing the parent's mmap_lock in dup_mmap(), before userfaultfd completion, to avoid moving the userfaultfd handling into the caller. This should get cleaned up later.
Move uprobe_start_dup_mmap() to preserve the lock ordering.
--- kernel/fork.c | 20 ++++++++++++++------ mm/mmap.c | 8 +------- mm/nommu.c | 2 +- 3 files changed, 16 insertions(+), 14 deletions(-)
kernel/fork.c +14 -6
@@ -1533,12 +1533,19 @@ static struct mm_struct *dup_mm(struct task_struct *tsk,if (!mm) goto fail_nomem; +uprobe_start_dup_mmap (); +if (mmap_write_lock_killable (oldmm)) { +free_mm (mm); + goto fail_uprobe; + } +memcpy (mm, oldmm,sizeof (*mm)); -if (!mm_init (mm, tsk)) - goto fail_nomem; +if (!mm_init (mm, tsk)) { +mmap_write_unlock (oldmm); + goto fail_uprobe; + } -uprobe_start_dup_mmap (); err =dup_mmap (mm, oldmm);if (err) goto free_pt;
@@ -1557,9 +1564,10 @@ static struct mm_struct *dup_mm(struct task_struct *tsk, mm->binfmt =NULL ;mm_init_owner (mm,NULL );mmput (mm); -if (err) -uprobe_end_dup_mmap (); - +if (!err) +return NULL ; +fail_uprobe: +uprobe_end_dup_mmap (); fail_nomem:return NULL ; }
mm/mmap.c +1 -7
@@ -1713,8 +1713,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)LIST_HEAD (uf);VMA_ITERATOR (vmi, mm,0 ); -if (mmap_write_lock_killable (oldmm)) -return -EINTR ; +mmap_assert_write_locked (oldmm);flush_cache_dup_mm (oldmm);uprobe_dup_mmap (oldmm, mm);/*
@@ -1725,11 +1724,6 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)/* No ordering required: file already has been exposed. */ dup_mm_exe_file (mm, oldmm); - mm->total_vm = oldmm->total_vm; - mm->data_vm = oldmm->data_vm; - mm->exec_vm = oldmm->exec_vm; - mm->stack_vm = oldmm->stack_vm; -/* Use __mt_dup() to efficiently build an identical maple tree. */ retval =__mt_dup (&oldmm->mm_mt, &mm->mm_mt,GFP_KERNEL );if (unlikely (retval))
mm/nommu.c +1 -1
@@ -1887,7 +1887,7 @@ subsys_initcall(init_admin_reserve);int dup_mmap (struct mm_struct *mm,struct mm_struct *oldmm) { -mmap_write_lock (oldmm); +mmap_assert_write_locked (oldmm);dup_mm_exe_file (mm, oldmm);mmap_write_unlock (oldmm);return 0 ;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 3/47 mm/shmem: make shmem_file() check the file's inode
In reply to David Hildenbrand (Arm)shmem_file() only checks if file->f_mapping is a shmem mapping. This mapping can belong to another file, however, as with a Coda file backed by a tmpfs container after mmap().
So it does not guarantee that the file inode is an actual shmem inode.
Seal handling and memfd preservation use this check before accessing shmem inode state, so they can access an inode of the wrong type.
Let's make shmem_file() check the inode's file operations instead, to identify shmem inodes reliably. This will identify regular shmem inodes, including memfds and ordinary tmpfs files, independently of wrapper file operations.
Use shmem_mapping() in collapse_file() instead to not change the behavior of collapse_file(): it operates on the page cache and uses mapping->host for shmem inode operations. Focusing on the file does not seem relevant.
--- include/linux/shmem_fs.h | 15 ++++++--------- mm/khugepaged.c | 2 +- mm/shmem.c | 5 +++++ 3 files changed, 12 insertions(+), 10 deletions(-)
include/linux/shmem_fs.h +6 -9
@@ -117,8 +117,14 @@ extern unsigned long shmem_get_unmapped_area(struct file *, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags);extern int shmem_lock (struct file *file,int lock,struct ucounts *ucounts);#ifdef CONFIG_SHMEM +bool shmem_file (struct file *file);bool shmem_mapping (const struct address_space *mapping);#else +static inline bool shmem_file (struct file *file) +{ +return false; +} +static inline bool shmem_mapping (const struct address_space *mapping) {return false;
@@ -193,15 +199,6 @@ static inline struct page *shmem_read_mapping_page( mapping_gfp_mask(mapping)); } -static inline bool shmem_file (struct file *file) -{ -if (!IS_ENABLED (CONFIG_SHMEM )) -return false; -if (!file || !file->f_mapping) -return false; -return shmem_mapping (file->f_mapping); -} -/* Must be called with inode lock taken exclusive. */ static inline void shmem_freeze (struct inode *inode,bool freeze) {
mm/khugepaged.c +1 -1
@@ -2230,6 +2230,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr, struct file *file, pgoff_t start, struct collapse_control *cc) { struct address_space *mapping = file->f_mapping; +const bool is_shmem =shmem_mapping (mapping);struct page *dst;struct folio *folio, *tmp, *new_folio;unsigned long new_pfn =-1 ;
@@ -2238,7 +2239,6 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,XA_STATE_ORDER (xas, &mapping->i_pages, start,HPAGE_PMD_ORDER );enum scan_result result =SCAN_SUCCEED ;int nr_none =0 ; -bool is_shmem =shmem_file (file);/* * MADV_COLLAPSE ignores shmem huge config, so do not check shmem
mm/shmem.c +5 -0
@@ -275,6 +275,11 @@ static const struct vm_operations_struct shmem_vm_ops;static const struct vm_operations_struct shmem_anon_vm_ops;static struct file_system_type shmem_fs_type; +bool shmem_file (struct file *file) +{ +return file &&file_inode (file)->i_fop == &shmem_file_operations; +} +bool shmem_mapping (const struct address_space *mapping) {return mapping->a_ops == &shmem_aops;
Signature · 1 line
2.43.0
Re: [PATCH v3 03/47] mm/shmem: make shmem_file() check the file's inode
Quoted 18 lines · David Hildenbrand (Arm)
shmem_file()only checks iffile->f_mappingis a shmem mapping. This mapping can belong to another file, however, as with a Coda file backed by a tmpfs container aftermmap().So it does not guarantee that the file inode is an actual shmem inode.
Seal handling and memfd preservation use this check before accessing shmem inode state, so they can access an inode of the wrong type.
Let's make
shmem_file()check the inode's file operations instead, to identify shmem inodes reliably. This will identify regular shmem inodes, including memfds and ordinary tmpfs files, independently of wrapper file operations.Use
shmem_mapping()incollapse_file()instead to not change the behavior ofcollapse_file(): it operates on the page cache and usesmapping->hostfor shmem inode operations. Focusing on the file does not seem relevant.
Quoted 8 lines
Fixes c49fcfcda8b5 ("mm/shmem: update file sealing comments and file checking")Fixes b3749f174d68 ("mm: memfd_luo: allow preserving memfd")Reported-by Chris Mason <[email protected]>Suggested-by Pratyush Yadav <[email protected]>Reviewed-by Pratyush Yadav (Google) <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
Make sense to me.
David Hildenbrand (Arm)
maintainerPATCH v3 4/47 mm/memfd: make f_pos accesses atomic
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
Commit 9c225f2655e3 ("vfs: atomic f_pos accesses as per POSIX") made read(), write() and lseek() on a shared open file description of a regular file serialize on ->f_pos_lock. The flag that asks for it, FMODE_ATOMIC_POS, is set in do_dentry_open(). A memfd is a regular file too, but its struct file comes from alloc_file_pseudo() and never passes through do_dentry_open(), so it does not get the flag:
CPU0 CPU1
write(memfd, "a", 1) write(memfd, "b", 1)
pos = file->f_pos (= 0)
pos = file->f_pos (= 0)
vfs_write(..., &pos)
vfs_write(..., &pos)
file->f_pos = pos (= 1)
file->f_pos = pos (= 1)One of the two bytes is overwritten. Two threads writing 100000 bytes each, one at a time, to a memfd end up with a file that is far shorter than 200000 bytes, while the same program on a tmpfs file never loses a byte.
Set the flag when the memfd is created. As for any other regular file the lock is only taken if the file is reachable in more than one way, and nothing a memfd does in read or write waits for another operation on the same file. This cannot be done for all alloc_file_pseudo() files: anon inode files such as eventfd are S_IFREG as well, and a blocked read holding ->f_pos_lock would lock out the write that is supposed to wake it.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/memfd.c | 2 ++ 1 file changed, 2 insertions(+)
mm/memfd.c +2 -0
@@ -482,6 +482,8 @@ struct file *memfd_alloc_file(const char *name, unsigned int flags) } file->f_mode |=FMODE_LSEEK |FMODE_PREAD |FMODE_PWRITE ; +/* a regular file like any other: POSIX.1-2008/SUSv4 Section XSI 2.9.7 */ + file->f_mode |=FMODE_ATOMIC_POS ; file->f_flags |=O_LARGEFILE ;if (flags &MFD_NOEXEC_SEAL ) {
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 5/47 mm/truncate: look again if a folio left the mapping
In reply to David Hildenbrand (Arm)invalidate_inode_pages2_range() skips a folio which no longer belongs to the mapping by the time it has got the folio lock, on the assumption that somebody else has removed it from the page cache. But the folio may have been replaced, and then there is a new folio with the same data at that index.
For most cases, like folio migration, that is impossible. However, collapse_file() only checks the refcount of the old folios, under the i_pages lock, and relies on their folio locks from there on. That works for users which repeat the lookup when they find folio->mapping changed. The following scenario is, however, possible:
CPU0 pwrite(O_DIRECT) CPU1 collapse_file()
kiocb_invalidate_pages()
read the folio (old data), lock it refcount check
the write completes copy to new_folio
kiocb_invalidate_post_direct_write()
find_get_entries() -> old folio
xas_store(new_folio)
folio->mapping = NULL
folio_unlock(folio)
folio_lock()
folio->mapping != mapping
-> continueIn that scenario, the collapsed folio with the data from before the write stays in the page cache after the write has returned.
This used to be impossible, as khugepaged only collapsed files that nobody had open for writing.
Restart the lookup at that index, like shmem_undo_range() does. The same problem seems to exist in truncate_inode_pages_range() as well when truncate_inode_folio() fails, so handle it there as well, but slightly
differently: simply check once more through find_get_entries().
--- mm/truncate.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
mm/truncate.c +11 -2
@@ -475,8 +475,11 @@ void truncate_inode_pages_range(struct address_space *mapping, } } +if (start >= end) +return ; + index = start; -while (index < end) { +while (true) {cond_resched ();if (!find_get_entries (mapping, &index, end -1 , &fbatch, indices)) {
@@ -499,6 +502,7 @@ void truncate_inode_pages_range(struct address_space *mapping,folio_lock (folio);VM_BUG_ON_FOLIO (!folio_contains (folio, indices[i]), folio);folio_wait_writeback (folio); +/* Do not handle errors, we'll do another lookup */ truncate_inode_folio (mapping, folio);folio_unlock (folio); }
@@ -760,7 +764,12 @@ int invalidate_inode_pages2_range(struct address_space *mapping,folio_lock (folio);if (unlikely (folio->mapping != mapping)) {folio_unlock (folio); -continue ; +/* +* Folio might have been replaced: look up +* again. +*/ + index = indices[i]; +break ; }VM_BUG_ON_FOLIO (!folio_contains (folio, indices[i]), folio);folio_wait_writeback (folio);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 6/47 mm/filemap: read mapping->a_ops->free_folio under i_lock
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
memory_failure() -> ... -> filemap_remove_folio() can read mapping->a_ops after the inode has been freed. Once the folio is deleted from the pagecache it no longer pins the mapping, so after i_lock is dropped e.g., a concurrent final iput() can evict and free the inode before filemap_free_folio() looks up mapping->a_ops->free_folio.
Most callers hold a reference on the inode, but not all.
Documentation/filesystems/vfs.rst explicitly states that ->free_folio() must not assume the original address_space mapping still exists.
Fix it by loading ->free_folio while i_lock is still held, similar to what folio_unmap_invalidate() does.
Inline filemap_free_folio() into its remaining caller such that delete_from_page_cache_batch() also just loads the callback before dropping the i_lock.
The bug was found by code inspection and has not been reproduced.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/filemap.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)
mm/filemap.c +14 -15
@@ -227,18 +227,6 @@ void __filemap_remove_folio(struct folio *folio, void *shadow)page_cache_delete (mapping, folio, shadow); } -static void filemap_free_folio (const struct address_space *mapping, -struct folio *folio) -{ -void (*free_folio)(struct folio *); - - free_folio = mapping->a_ops->free_folio; -if (free_folio) -free_folio (folio); - -folio_put_refs (folio,folio_nr_pages (folio)); -} -/** * filemap_remove_folio - Remove folio from page cache. * @folio: The folio.
@@ -250,6 +238,7 @@ static void filemap_free_folio(const struct address_space *mapping,void filemap_remove_folio (struct folio *folio) {struct address_space *mapping = folio->mapping; +void (*free_folio)(struct folio *folio);BUG_ON (!folio_test_locked (folio));spin_lock (&mapping->host->i_lock);
@@ -258,9 +247,12 @@ void filemap_remove_folio(struct folio *folio)xa_unlock_irq (&mapping->i_pages);if (mapping_shrinkable (mapping))inode_lru_list_add (mapping->host); + free_folio = mapping->a_ops->free_folio;spin_unlock (&mapping->host->i_lock); -filemap_free_folio (mapping, folio); +if (free_folio) +free_folio (folio); +folio_put_refs (folio,folio_nr_pages (folio)); }/*
@@ -320,6 +312,7 @@ static void page_cache_delete_batch(struct address_space *mapping,void delete_from_page_cache_batch (struct address_space *mapping,struct folio_batch *fbatch) { +void (*free_folio)(struct folio *folio);int i;if (!folio_batch_count (fbatch))
@@ -337,10 +330,16 @@ void delete_from_page_cache_batch(struct address_space *mapping,xa_unlock_irq (&mapping->i_pages);if (mapping_shrinkable (mapping))inode_lru_list_add (mapping->host); + free_folio = mapping->a_ops->free_folio;spin_unlock (&mapping->host->i_lock); -for (i =0 ; i <folio_batch_count (fbatch); i++) -filemap_free_folio (mapping, fbatch->folios[i]); +for (i =0 ; i <folio_batch_count (fbatch); i++) { +struct folio *folio = fbatch->folios[i]; + +if (free_folio) +free_folio (folio); +folio_put_refs (folio,folio_nr_pages (folio)); + } }int filemap_check_errors (struct address_space *mapping)
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 7/47 mm/filemap: don't lose a folio lock wakeup to a signal
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
An exclusive waiter in folio_wait_bit_common() can leave the wait loop because of a fatal signal, then be woken by folio_unlock() before finish_wait(). If the waker did not hand over the lock, the waiter returns -EINTR without trying to acquire it.
folio_wake_bit() stops after waking the first exclusive waiter, so this loses the wakeup and can leave other waiters asleep indefinitely on an unlocked folio.
After finish_wait(), let a waiter woken without a lock handoff try once more to acquire the lock. If successful, return success so the caller's eventual unlock wakes the next waiter. Otherwise, the current lock owner will do so.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/filemap.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-)
mm/filemap.c +15 -3
@@ -1373,10 +1373,22 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,* return value based on that state without races. * * Also note that WQ_FLAG_WOKEN is sufficient for a non-exclusive -* waiter, but an exclusive one requires WQ_FLAG_DONE. +* waiter, but an exclusive one must also acquire the bit. */ -if (behavior ==EXCLUSIVE ) -return wait->flags &WQ_FLAG_DONE ?0 : -EINTR ; +if (behavior ==EXCLUSIVE ) { +if (wait->flags &WQ_FLAG_DONE ) +return 0 ; +/* +* We might have just been woken up and the waker will not +* wake up anybody else. If we ignore that now, the remaining +* waiters may not get woken up: take the bit now if it is +* still free. +*/ +if ((wait->flags &WQ_FLAG_WOKEN ) && + !test_and_set_bit (bit_nr,folio_flags (folio,0 ))) +return 0 ; +return -EINTR ; + }return wait->flags &WQ_FLAG_WOKEN ?0 : -EINTR ; }
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 8/47 mm/filemap: finish fault cleanup before dropping the fault lock
In reply to David Hildenbrand (Arm)On a retried fault, a fatal signal interrupting the folio lock wait makes lock_folio_maybe_drop_mmap() drop the fault lock without pinning the file. However, filemap_fault() may still need to release mapping->invalidate_lock. A concurrent munmap() can release the last file reference and free the mapping before that unlock.
Defer dropping the fault lock to filemap_fault()'s retry cleanup, after releasing invalidate_lock. If the fault lock was already dropped for I/O, release the existing file reference instead. Keep the fault lock held for FAULT_FLAG_RETRY_NOWAIT as before.
--- mm/filemap.c | 37 +++++++++++++------------------------ 1 file changed, 13 insertions(+), 24 deletions(-)
mm/filemap.c +13 -24
@@ -3308,11 +3308,11 @@ loff_t mapping_seek_hole_data(struct address_space *mapping, loff_t start,* @folio - the folio to lock. * @fpin - the pointer to the file we may pin (or is already pinned). * -* This works similar to lock_folio_or_retry in that it can drop the -* mmap_lock. It differs in that it actually returns the folio locked -* if it returns 1 and 0 if it couldn't lock the folio. If we did have -* to drop the mmap_lock then fpin will point to the pinned file and -* needs to be fput()'ed at a later point. +* Returns 1 with the folio locked, or 0 if locking failed. +* +* On both entry and return, a non-NULL *fpin means the fault lock has +* already been dropped and the caller must eventually fput() the file. +* Otherwise the fault lock is still held, including on failure. */ static int lock_folio_maybe_drop_mmap (struct vm_fault *vmf,struct folio *folio,struct file **fpin)
@@ -3329,21 +3329,9 @@ static int lock_folio_maybe_drop_mmap(struct vm_fault *vmf, struct folio *folio,return 0 ; *fpin =maybe_unlock_mmap_for_io (vmf, *fpin); -if (vmf->flags &FAULT_FLAG_KILLABLE ) { -if (__folio_lock_killable (folio)) { -/* -* We didn't have the right flags to drop the -* fault lock, but all fault_handlers only check -* for fatal signals if we return VM_FAULT_RETRY, -* so we need to drop the fault lock here and -* return 0 if we don't have a fpin. -*/ -if (*fpin ==NULL ) -release_fault_lock (vmf); -return 0 ; - } - }else -__folio_lock (folio); +if (vmf->flags &FAULT_FLAG_KILLABLE ) +return !__folio_lock_killable (folio); +__folio_lock (folio);return 1 ; }
@@ -3572,7 +3560,7 @@ static vm_fault_t filemap_fault_recheck_pte_none(struct vm_fault *vmf)* vma->vm_mm->mmap_lock must be held on entry. * * If our return value has VM_FAULT_RETRY set, it's because the mmap_lock -* may be dropped before doing I/O or by lock_folio_maybe_drop_mmap(). +* may be dropped before doing I/O or when aborting an interrupted wait. * * If our return value does not have VM_FAULT_RETRY set, the mmap_lock * has not been released.
@@ -3728,9 +3716,8 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) out_retry:/* -* We dropped the mmap_lock, we need to return to the fault handler to -* re-find the vma and come back and find our hopefully still populated -* page. +* With fpin set, the fault lock was already dropped. Otherwise it is +* still held. Release invalidate_lock before dropping either protection. */ if (!IS_ERR (folio))folio_put (folio);
@@ -3738,6 +3725,8 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)filemap_invalidate_unlock_shared (mapping);if (fpin)fput (fpin); +else if (!(vmf->flags &FAULT_FLAG_RETRY_NOWAIT )) +release_fault_lock (vmf);return ret |VM_FAULT_RETRY ; }EXPORT_SYMBOL (filemap_fault);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 9/47 mm/rmap: handle device-exclusive entries in folio_referenced_one()
In reply to David Hildenbrand (Arm)page_vma_mapped_walk() can return non-present device-exclusive entries, but folio_referenced_one() passes them to PTE young-bit accessors that require present PTEs.
We must neither call clear_flush_young_ptes_notify() nor folio_pte_batch() with non-present entries. For example, clearing the young bit on non-present PTEs can corrupt the entry: on arm64 PTE_AF overlaps the swap type, so clearing it turns a device-exclusive entry into an ordinary swap entry.
folio_pte_batch() will VM_WARN_ON_FOLIO.
Apply the same approach as the previous fixes to e.g., damon_ptep_mkold():
treat these entries as old from the CPU's perspective and obtain their young state through the MMU notifier.
Skip PTE batching and MGLRU look-around for non-present entries, and don't modify non-present PTEs.
--- mm/rmap.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
mm/rmap.c +19 -4
@@ -924,6 +924,7 @@ static bool folio_referenced_one(struct folio *folio,DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 );int ptes =0 , referenced =0 ;unsigned int nr; +pte_t pteval;while (page_vma_mapped_walk (&pvmw)) { address = pvmw.address;
@@ -968,10 +969,12 @@ static bool folio_referenced_one(struct folio *folio,return false; } -if (pvmw.pte &&folio_test_large (folio)) { +if (pvmw.pte) + pteval =ptep_get (pvmw.pte); + +if (pvmw.pte &&pte_present (pteval) &&folio_test_large (folio)) {const unsigned long end_addr =pmd_addr_end (address, vma->vm_end);const unsigned int max_nr = (end_addr - address) >>PAGE_SHIFT ; -pte_t pteval =ptep_get (pvmw.pte); nr =folio_pte_batch (folio, pvmw.pte, pteval, max_nr); }
@@ -981,11 +984,23 @@ static bool folio_referenced_one(struct folio *folio,* are. —they could be on active/inactive lists or on MGLRU. So the * simplest approach is to disable this look-around optimization. */ -if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte) { +if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte && +pte_present (pteval)) {if (lru_gen_look_around (&pvmw, nr)) referenced++; }else if (pvmw.pte) { -if (clear_flush_young_ptes_notify (vma, address, pvmw.pte, nr)) +bool young = false; + +/* +* PFN swap PTEs, such as device-exclusive ones, that +* actually map pages are "old" from a CPU perspective. +* The MMU notifier takes care of any device aspects. +*/ +if (pte_present (pteval)) + young =clear_flush_young_ptes (vma, address, pvmw.pte, nr); + young |=mmu_notifier_clear_flush_young (vma->vm_mm, address, + address + nr *PAGE_SIZE ); +if (young) referenced++; }else if (IS_ENABLED (CONFIG_TRANSPARENT_HUGEPAGE )) { if (pmdp_clear_flush_young_notify(vma, address,
Signature · 1 line
2.43.0
Re: [PATCH v3 09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one()
Quoted 28 lines · David Hildenbrand (Arm)
page_vma_mapped_walk()can return non-present device-exclusive entries, butfolio_referenced_one()passes them to PTE young-bit accessors that require present PTEs.We must neither call
clear_flush_young_ptes_notify()norfolio_pte_batch()with non-present entries. For example, clearing the young bit on non-present PTEs can corrupt the entry: on arm64PTE_AFoverlaps the swap type, so clearing it turns a device-exclusive entry into an ordinary swap entry.
folio_pte_batch()willVM_WARN_ON_FOLIO.Apply the same approach as the previous fixes to e.g.,
damon_ptep_mkold():
treat these entries as old from the CPU's perspective and obtain their young state through the MMU notifier.Skip PTE batching and MGLRU look-around for non-present entries, and don't modify non-present PTEs.
Fixes b756a3b5e7ea ("mm: device exclusive memory access")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reported-by Josef Bacik <[email protected]>Reviewed-by Harry Yoo (Meta) <[email protected]>Reviewed-by Barry Song <[email protected]>Reviewed-by Dev Jain <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM.
Re: [PATCH v3 09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one()
In reply to David Hildenbrand (Arm)Quoted 60 lines · David Hildenbrand (Arm)
page_vma_mapped_walk()can return non-present device-exclusive entries, butfolio_referenced_one()passes them to PTE young-bit accessors that require present PTEs.We must neither call
clear_flush_young_ptes_notify()norfolio_pte_batch()with non-present entries. For example, clearing the young bit on non-present PTEs can corrupt the entry: on arm64PTE_AFoverlaps the swap type, so clearing it turns a device-exclusive entry into an ordinary swap entry.
folio_pte_batch()willVM_WARN_ON_FOLIO.Apply the same approach as the previous fixes to e.g.,
damon_ptep_mkold():
treat these entries as old from the CPU's perspective and obtain their young state through the MMU notifier.Skip PTE batching and MGLRU look-around for non-present entries, and don't modify non-present PTEs.
Fixes b756a3b5e7ea ("mm: device exclusive memory access")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reported-by Josef Bacik <[email protected]>Reviewed-by Harry Yoo (Meta) <[email protected]>Reviewed-by Barry Song <[email protected]>Reviewed-by Dev Jain <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/rmap.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)mm/rmap.c +5
-2@@ -924,6 +924,7 @@ static bool folio_referenced_one(struct folio *folio,DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 );int ptes =0 , referenced =0 ;unsigned int nr; +pte_t pteval;while (page_vma_mapped_walk (&pvmw)) { address = pvmw.address;@@ -968,10 +969,12 @@ static bool folio_referenced_one(struct folio *folio,return false; } -if (pvmw.pte &&folio_test_large (folio)) { +if (pvmw.pte) + pteval =ptep_get (pvmw.pte); + +if (pvmw.pte &&pte_present (pteval) &&folio_test_large (folio)) {const unsigned long end_addr =pmd_addr_end (address, vma->vm_end);const unsigned int max_nr = (end_addr - address) >>PAGE_SHIFT ; -pte_t pteval =ptep_get (pvmw.pte); nr =folio_pte_batch (folio, pvmw.pte, pteval, max_nr); }
mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 988
Quoted 10 lines
mm/rmap.c +2
-2@@ -981,11 +984,23 @@ static bool folio_referenced_one(struct folio *folio,* are. —they could be on active/inactive lists or on MGLRU. So the * simplest approach is to disable this look-around optimization. */ -if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte) { +if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte && +pte_present (pteval)) {if (lru_gen_look_around (&pvmw, nr)) referenced++; }else if (pvmw.pte) { -if (clear_flush_young_ptes_notify (vma, address, pvmw.pte, nr))
This was the only caller of clear_flush_young_ptes_notify(), so this leaves both definitions in mm/internal.h unused.
Would you prefer to remove them, or keep them for company with their still used test_and_clear_young_ptes_notify friend?
Quoted 15 lines
mm/rmap.c +12
-0+bool young = false; + +/* +* PFN swap PTEs, such as device-exclusive ones, that +* actually map pages are "old" from a CPU perspective. +* The MMU notifier takes care of any device aspects. +*/ +if (pte_present (pteval)) + young =clear_flush_young_ptes (vma, address, pvmw.pte, nr); + young |=mmu_notifier_clear_flush_young (vma->vm_mm, address, + address + nr *PAGE_SIZE ); +if (young) referenced++; }else if (IS_ENABLED (CONFIG_TRANSPARENT_HUGEPAGE )) { if (pmdp_clear_flush_young_notify(vma, address,
David Hildenbrand (Arm)
maintainerRe: [PATCH v3 09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one()
Quoted 75 lines · Shivank
Quoted 60 lines · David Hildenbrand (Arm)
page_vma_mapped_walk()can return non-present device-exclusive entries, butfolio_referenced_one()passes them to PTE young-bit accessors that require present PTEs.We must neither call
clear_flush_young_ptes_notify()norfolio_pte_batch()with non-present entries. For example, clearing the young bit on non-present PTEs can corrupt the entry: on arm64PTE_AFoverlaps the swap type, so clearing it turns a device-exclusive entry into an ordinary swap entry.
folio_pte_batch()willVM_WARN_ON_FOLIO.Apply the same approach as the previous fixes to e.g.,
damon_ptep_mkold():
treat these entries as old from the CPU's perspective and obtain their young state through the MMU notifier.Skip PTE batching and MGLRU look-around for non-present entries, and don't modify non-present PTEs.
Fixes b756a3b5e7ea ("mm: device exclusive memory access")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reported-by Josef Bacik <[email protected]>Reviewed-by Harry Yoo (Meta) <[email protected]>Reviewed-by Barry Song <[email protected]>Reviewed-by Dev Jain <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/rmap.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)mm/rmap.c +5
-2@@ -924,6 +924,7 @@ static bool folio_referenced_one(struct folio *folio,DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 );int ptes =0 , referenced =0 ;unsigned int nr; +pte_t pteval;while (page_vma_mapped_walk (&pvmw)) { address = pvmw.address;@@ -968,10 +969,12 @@ static bool folio_referenced_one(struct folio *folio,return false; } -if (pvmw.pte &&folio_test_large (folio)) { +if (pvmw.pte) + pteval =ptep_get (pvmw.pte); + +if (pvmw.pte &&pte_present (pteval) &&folio_test_large (folio)) {const unsigned long end_addr =pmd_addr_end (address, vma->vm_end);const unsigned int max_nr = (end_addr - address) >>PAGE_SHIFT ; -pte_t pteval =ptep_get (pvmw.pte); nr =folio_pte_batch (folio, pvmw.pte, pteval, max_nr); }mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 988
Quoted 10 lines
mm/rmap.c +2
-2@@ -981,11 +984,23 @@ static bool folio_referenced_one(struct folio *folio,* are. —they could be on active/inactive lists or on MGLRU. So the * simplest approach is to disable this look-around optimization. */ -if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte) { +if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte && +pte_present (pteval)) {if (lru_gen_look_around (&pvmw, nr)) referenced++; }else if (pvmw.pte) { -if (clear_flush_young_ptes_notify (vma, address, pvmw.pte, nr))This was the only caller of
clear_flush_young_ptes_notify(), so this leaves both definitions inmm/internal.hunused.
Quoted 2 lines
Would you prefer to remove them, or keep them for company with their still used
test_and_clear_young_ptes_notifyfriend?
Can you send a cleanup? Thanks!
Signature · 3 lines
Cheers, David
Re: [PATCH v3 09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one()
Quoted 76 lines · David Hildenbrand (Arm)
Quoted 75 lines · Shivank
Quoted 60 lines · David Hildenbrand (Arm)
page_vma_mapped_walk()can return non-present device-exclusive entries, butfolio_referenced_one()passes them to PTE young-bit accessors that require present PTEs.We must neither call
clear_flush_young_ptes_notify()norfolio_pte_batch()with non-present entries. For example, clearing the young bit on non-present PTEs can corrupt the entry: on arm64PTE_AFoverlaps the swap type, so clearing it turns a device-exclusive entry into an ordinary swap entry.
folio_pte_batch()willVM_WARN_ON_FOLIO.Apply the same approach as the previous fixes to e.g.,
damon_ptep_mkold():
treat these entries as old from the CPU's perspective and obtain their young state through the MMU notifier.Skip PTE batching and MGLRU look-around for non-present entries, and don't modify non-present PTEs.
Fixes b756a3b5e7ea ("mm: device exclusive memory access")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reported-by Josef Bacik <[email protected]>Reviewed-by Harry Yoo (Meta) <[email protected]>Reviewed-by Barry Song <[email protected]>Reviewed-by Dev Jain <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/rmap.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)mm/rmap.c +5
-2@@ -924,6 +924,7 @@ static bool folio_referenced_one(struct folio *folio,DEFINE_FOLIO_VMA_WALK (pvmw, folio, vma, address,0 );int ptes =0 , referenced =0 ;unsigned int nr; +pte_t pteval;while (page_vma_mapped_walk (&pvmw)) { address = pvmw.address;@@ -968,10 +969,12 @@ static bool folio_referenced_one(struct folio *folio,return false; } -if (pvmw.pte &&folio_test_large (folio)) { +if (pvmw.pte) + pteval =ptep_get (pvmw.pte); + +if (pvmw.pte &&pte_present (pteval) &&folio_test_large (folio)) {const unsigned long end_addr =pmd_addr_end (address, vma->vm_end);const unsigned int max_nr = (end_addr - address) >>PAGE_SHIFT ; -pte_t pteval =ptep_get (pvmw.pte); nr =folio_pte_batch (folio, pvmw.pte, pteval, max_nr); }mm/rmap.c static bool folio_referenced_one(struct folio *folio, · line 988
Quoted 10 lines
mm/rmap.c +2
-2@@ -981,11 +984,23 @@ static bool folio_referenced_one(struct folio *folio,* are. —they could be on active/inactive lists or on MGLRU. So the * simplest approach is to disable this look-around optimization. */ -if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte) { +if (lru_gen_enabled () && !lru_gen_switching () && pvmw.pte && +pte_present (pteval)) {if (lru_gen_look_around (&pvmw, nr)) referenced++; }else if (pvmw.pte) { -if (clear_flush_young_ptes_notify (vma, address, pvmw.pte, nr))This was the only caller of
clear_flush_young_ptes_notify(), so this leaves both definitions inmm/internal.hunused.
Quoted 4 lines
Quoted 2 lines
Would you prefer to remove them, or keep them for company with their still used
test_and_clear_young_ptes_notifyfriend?Can you send a cleanup? Thanks!
Sure, thanks. Will do.
I'll include it in V2 of my previous series [1].
Do you have any comment on [1] now, before I send V2.
[1]
https://lore.kernel.org/linux-mm/[email protected]
Thanks,
Shivank
David Hildenbrand (Arm)
maintainerRe: [PATCH v3 09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one()
Quoted 6 lines · Shivank
Quoted 5 lines · David Hildenbrand (Arm)
Quoted 4 lines · Shivank
This was the only caller of
clear_flush_young_ptes_notify(), so this leaves both definitions inmm/internal.hunused.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Would you prefer to remove them, or keep them for company with their still used
test_and_clear_young_ptes_notifyfriend?Can you send a cleanup? Thanks!
Sure, thanks. Will do.
I'll include it in V2 of my previous series [1].
Please send this one separately against the core.git/for-next branch. Likely, the other series would best be split up for MM components to pick them (some stuff can go through the page_alloc tree, for example).
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerPATCH v3 10/47 mm/huge_memory: fix list corruption when deferred_split_scan() races with deferred_split_folio()
In reply to David Hildenbrand (Arm)Since the deferred split queue became a list_lru, the isolate callback moves the folios it took a reference on to an on-stack list, linked through folio->_deferred_list, and deferred_split_scan() takes them off that list with list_del_init() without holding the list_lru lock.
deferred_split_folio() does not know about the on-stack list. It only holds the list_lru lock when it looks at the same list_head, and it can run for such a folio: a THP is queued when it is mapped, as a candidate for the underused check, and is queued "again" when a part of it gets unmapped:
CPU0 deferred_split_scan() CPU1 deferred_split_folio()
list_del_init(&folio->_deferred_list)
next->prev = prev
prev->next = next
entry->next = entry
list_lru_lock_irqsave()
folio_set_partially_mapped()
__list_lru_add()
list_empty(item) -> true
list_add_tail(item, &l->list)
item->next = &l->list
item->prev = tail
entry->prev = entryThe folio now sits on the list_lru with ->prev pointing to itself. The next removal leaves the neighbours pointing to it, and they still do so after the folio has been freed. With CONFIG_DEBUG_LIST:
list_del corruption. prev->next should be ffd4000000270090, but was ffd4000000278090. (prev=ffd4000000270090)
Fix it by grabbing the lock around list_del_init() so deferred_split_folio() cannot race with it.
--- mm/huge_memory.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
mm/huge_memory.c +13 -2
@@ -4522,8 +4522,7 @@ int split_folio_to_list(struct folio *folio, struct list_head *list)* * It is unsafe to call folio_unqueue_deferred_split() until folio refcount is * zero: because even when the list_lru lock is held, a non-empty -* _deferred_list might be in use on deferred_split_scan()'s unlocked -* on-stack list. +* _deferred_list might be in use on deferred_split_scan()'s on-stack list. * * The list_lru sublist is determined by folio's memcg: it is therefore * important to unqueue deferred split before changing folio memcg.
@@ -4665,10 +4664,22 @@ static unsigned long deferred_split_scan(struct shrinker *shrink, deferred_split_isolate, &dispose);list_for_each_entry_safe (folio, next, &dispose, _deferred_list) { +struct mem_cgroup *memcg; +struct list_lru_one *lru;bool did_split = false;bool underused = false; +/* +* Sync with deferred_split_folio(), which can requeue the +* folio once list_empty() is true, and sets PG_partially_mapped. +*/ +rcu_read_lock (); + memcg =folio_memcg (folio); + lru =list_lru_lock_irq (&deferred_split_lru,folio_nid (folio), + &memcg);list_del_init (&folio->_deferred_list); +list_lru_unlock_irq (lru); +rcu_read_unlock ();if (!folio_test_partially_mapped (folio)) {/*
Signature · 1 line
2.43.0
Re: [PATCH v3 10/47] mm/huge_memory: fix list corruption when deferred_split_scan() races with deferred_split_folio()
Quoted 44 lines · David Hildenbrand (Arm)
Since the deferred split queue became a
list_lru, the isolate callback moves the folios it took a reference on to an on-stack list, linked throughfolio->_deferred_list, anddeferred_split_scan()takes them off that list withlist_del_init()without holding thelist_lrulock.
deferred_split_folio()does not know about the on-stack list. It only holds thelist_lrulock when it looks at the samelist_head, and it can run for such a folio: a THP is queued when it is mapped, as a candidate for the underused check, and is queued "again" when a part of it gets unmapped:CPU0 deferred_split_scan() CPU1 deferred_split_folio() list_del_init(&folio->_deferred_list) next->prev = prev prev->next = next entry->next = entry list_lru_lock_irqsave() folio_set_partially_mapped() __list_lru_add() list_empty(item) -> true list_add_tail(item, &l->list) item->next = &l->list item->prev = tail entry->prev = entryThe folio now sits on the
list_lruwith ->prev pointing to itself. The next removal leaves the neighbours pointing to it, and they still do so after the folio has been freed. WithCONFIG_DEBUG_LIST:
list_delcorruption.prev->nextshould beffd4000000270090, but wasffd4000000278090. (prev=ffd4000000270090)Fix it by grabbing the lock around
list_del_init()sodeferred_split_folio()cannot race with it.Fixes fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")Reported-by Josef Bacik <[email protected]>Reviewed-by Johannes Weiner <[email protected]>Reviewed-by Mani Tofigh <[email protected]>Tested-by Mani Tofigh <[email protected]>Reviewed-by Lance Yang <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM.
David Hildenbrand (Arm)
maintainerPATCH v3 11/47 mm/huge_memory: properly handle refcounted PUD mappings on fork
In reply to David Hildenbrand (Arm)copy_huge_pud() copies device-DAX entries without taking a folio reference or updating mapcounts and the child's RSS.
For example, MADV_GUARD_INSTALL sets VM_MAYBE_GUARD on a device-DAX VMA, forcing copy_page_range() to traverse the whole VMA on fork. Any remaining huge PUDs are then copied without accounting for the new mappings. Unmapping them in the child decrements accounting that was never added.
Use vm_normal_page_pud() to identify refcounted mappings. Take a folio reference and add the child's PUD rmap and RSS accounting before installing the copied entry. Pass src and dst VMAs consistently instead of src and dst MM. Move the TODO.
--- include/linux/huge_mm.h | 5 ++--- mm/huge_memory.c | 27 +++++++++++++++++++-------- mm/memory.c | 4 +--- 3 files changed, 22 insertions(+), 14 deletions(-)
include/linux/huge_mm.h +2 -3
@@ -12,9 +12,8 @@ int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm, pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr, struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma);bool huge_pmd_set_accessed (struct vm_fault *vmf); -int copy_huge_pud (struct mm_struct *dst_mm,struct mm_struct *src_mm, -pud_t *dst_pud,pud_t *src_pud,unsigned long addr, -struct vm_area_struct *vma); +int copy_huge_pud (struct vm_area_struct *dst_vma,struct vm_area_struct *src_vma, +pud_t *dst_pud,pud_t *src_pud,unsigned long addr);#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD void huge_pud_set_accessed (struct vm_fault *vmf,pud_t orig_pud);
mm/huge_memory.c +19 -8
@@ -2072,11 +2072,13 @@ void touch_pud(struct vm_area_struct *vma, unsigned long addr,update_mmu_cache_pud (vma, addr, pud); } -int copy_huge_pud (struct mm_struct *dst_mm,struct mm_struct *src_mm, -pud_t *dst_pud,pud_t *src_pud,unsigned long addr, -struct vm_area_struct *vma) +int copy_huge_pud (struct vm_area_struct *dst_vma,struct vm_area_struct *src_vma, +pud_t *dst_pud,pud_t *src_pud,unsigned long addr) { +struct mm_struct *dst_mm = dst_vma->vm_mm; +struct mm_struct *src_mm = src_vma->vm_mm;spinlock_t *dst_ptl, *src_ptl; +struct page *page;pud_t pud;int ret;
@@ -2089,11 +2091,20 @@ int copy_huge_pud(struct mm_struct *dst_mm, struct mm_struct *src_mm,if (unlikely (!pud_trans_huge (pud))) goto out_unlock; -/* -* TODO: once we support anonymous pages, use -* folio_try_dup_anon_rmap_*() and split if duplicating fails. -*/ -if (vma_is_cow_mapping (vma) &&pud_write (pud)) { + page =vm_normal_page_pud (src_vma, addr, pud); +if (page) { +struct folio *folio =page_folio (page); + +folio_get (folio); +/* +* TODO: once we support anonymous pages, use +* folio_try_dup_anon_rmap_*() and split if duplicating fails. +*/ +folio_add_file_rmap_pud (folio, page, dst_vma); +add_mm_counter (dst_mm,mm_counter_file (folio),HPAGE_PUD_NR ); + } + +if (vma_is_cow_mapping (src_vma) &&pud_write (pud)) {pudp_set_wrprotect (src_mm, addr, src_pud); pud =pud_wrprotect (pud); }
mm/memory.c +1 -3
@@ -1463,7 +1463,6 @@ copy_pud_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma, unsigned long end) {struct mm_struct *dst_mm = dst_vma->vm_mm; -struct mm_struct *src_mm = src_vma->vm_mm;pud_t *src_pud, *dst_pud;unsigned long next;
@@ -1477,8 +1476,7 @@ copy_pud_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,int err;VM_BUG_ON_VMA (next-addr !=HPAGE_PUD_SIZE , src_vma); - err =copy_huge_pud (dst_mm, src_mm, - dst_pud, src_pud, addr, src_vma); + err =copy_huge_pud (dst_vma, src_vma, dst_pud, src_pud, addr);if (err == -ENOMEM )return -ENOMEM ;if (!err)
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 12/47 mm/huge_memory: fix livelock on device private folio
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
If do_huge_pmd_device_private() cannot lock the folio, it simply returns to try again on the next fault.
Just like we did in commit b570f37a2ce4 ("mm: Fix a hmm_range_fault() livelock / starvation problem"), let's wait for the softleaf entry, which will drop the PTL and wait until the folio was unlocked. This is precisely what we do in do_swap_page() with device-private folios. This avoids possibly livelocking.
Update softleaf_entry_wait_on_locked() documentation to clarify that device-private entries wait until the folio is unlocked.
There seems to be no compelling reason to backport this to stable kernels unless we actually see problems there.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/filemap.c | 4 ++-- mm/huge_memory.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
mm/filemap.c +2 -2
@@ -1395,8 +1395,8 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,#ifdef CONFIG_MIGRATION /** -* softleaf_entry_wait_on_locked - Wait for a migration entry or -* device_private entry to be removed. +* softleaf_entry_wait_on_locked - Wait for a migration entry to be removed +* or a device-private folio in a device-private entry to get unlocked. * @entry: migration or device_private swap entry. * @ptl: already locked ptl. This function will drop the lock. *
mm/huge_memory.c +1 -1
@@ -1500,7 +1500,7 @@ vm_fault_t do_huge_pmd_device_private(struct vm_fault *vmf)folio_unlock (folio);folio_put (folio); }else { -spin_unlock (ptl); +softleaf_entry_wait_on_locked (entry, ptl); }return ret;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 13/47 mm/huge_memory: fix truncate racing with PMD zapping
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
zap_huge_pmd_folio() removes the rmap before calling folio_mark_dirty(). If this was the last mapping, truncation can skip unmapping the folio and proceed without waiting for the PMD lock, racing with dirtying.
ifs_set_range_dirty() can oops if MADV_DONTNEED races with ftruncate() on XFS, when we get the folio set dirty after it was found to not be mapped anymore.
Move folio_mark_dirty() and folio_mark_accessed() before rmap removal, matching the PTE zap path. This keeps the folio mapped while updating its state, so truncation must wait for the page table lock.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/huge_memory.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
mm/huge_memory.c +7 -4
@@ -2495,10 +2495,6 @@ static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma, {const bool is_device_private =folio_is_device_private (folio); -/* Present and device private folios are rmappable. */ -if (is_present || is_device_private) -folio_remove_rmap_pmd (folio, &folio->page, vma); -if (folio_test_anon (folio)) {add_mm_counter (mm,MM_ANONPAGES , -HPAGE_PMD_NR ); }else {
@@ -2512,6 +2508,13 @@ static void zap_huge_pmd_folio(struct mm_struct *mm, struct vm_area_struct *vma,folio_mark_accessed (folio); } +/* +* Present and device private folios are rmappable. Remove the +* rmap after transferring young+dirty to the folio. +*/ +if (is_present || is_device_private) +folio_remove_rmap_pmd (folio, &folio->page, vma); +/* Device private folios are pinned. */ if (is_device_private)folio_put (folio);
Signature · 1 line
2.43.0
Re: [PATCH v3 13/47] mm/huge_memory: fix truncate racing with PMD zapping
Quoted 23 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
zap_huge_pmd_folio()removes the rmap before callingfolio_mark_dirty(). If this was the last mapping, truncation can skip unmapping the folio and proceed without waiting for the PMD lock, racing with dirtying.
ifs_set_range_dirty()can oops ifMADV_DONTNEEDraces withftruncate()on XFS, when we get the folio set dirty after it was found to not be mapped anymore.Move
folio_mark_dirty()andfolio_mark_accessed()before rmap removal, matching the PTE zap path. This keeps the folio mapped while updating its state, so truncation must wait for the page table lock.Fix generated by an LLM and sent by Chris Mason in private.
Fixes fe6cf984939d ("mm/huge_memory: transfer the pmd dirty bit to the folio on zap")Signed-off-by Chris Mason <[email protected]>[ david: simplify comment and description ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 4 lines
--- mm/huge_memory.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH v3 13/47] mm/huge_memory: fix truncate racing with PMD zapping
In reply to David Hildenbrand (Arm)Quoted 16 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
zap_huge_pmd_folio()removes the rmap before callingfolio_mark_dirty(). If this was the last mapping, truncation can skip unmapping the folio and proceed without waiting for the PMD lock, racing with dirtying.
ifs_set_range_dirty()can oops ifMADV_DONTNEEDraces withftruncate()on XFS, when we get the folio set dirty after it was found to not be mapped anymore.Move
folio_mark_dirty()andfolio_mark_accessed()before rmap removal, matching the PTE zap path. This keeps the folio mapped while updating its state, so truncation must wait for the page table lock.Fix generated by an LLM and sent by Chris Mason in private.
Quoted 8 lines
Fixes fe6cf984939d ("mm/huge_memory: transfer the pmd dirty bit to the folio on zap")Signed-off-by Chris Mason <[email protected]>[ david: simplify comment and description ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
I checked the memory ordering issues, and it looks like the implicit barriers provided by the fully ordered atomic operations already ensure the required ordering. So LGTM.
David Hildenbrand (Arm)
maintainerPATCH v3 14/47 mm/huge_memory: fix zapping migration entries for device-private THPs
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
A PMD migration entry can refer to a device-private folio. zap_huge_pmd_folio() checks the folio type instead of the entry type, so it treats such a migration entry as a device-private entry.
Installing the migration entry already removed the reverse mapping and dropped the mapping reference. Zapping it must not repeat those operations. Concurrent MADV_DONTNEED during migration back to system memory can trigger this.
Check the PMD entry type using pmd_is_device_private_entry().
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/huge_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/huge_memory.c +1 -1
@@ -2493,7 +2493,7 @@ static inline void zap_deposited_table(struct mm_struct *mm, pmd_t *pmd)static void zap_huge_pmd_folio (struct mm_struct *mm,struct vm_area_struct *vma,pmd_t pmdval,struct folio *folio,bool is_present) { -const bool is_device_private =folio_is_device_private (folio); +const bool is_device_private =pmd_is_device_private_entry (pmdval);if (folio_test_anon (folio)) {add_mm_counter (mm,MM_ANONPAGES , -HPAGE_PMD_NR );
Signature · 1 line
2.43.0
Re: [PATCH v3 14/47] mm/huge_memory: fix zapping migration entries for device-private THPs
Quoted 21 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
A PMD migration entry can refer to a device-private folio.
zap_huge_pmd_folio()checks the folio type instead of the entry type, so it treats such a migration entry as a device-private entry.Installing the migration entry already removed the reverse mapping and dropped the mapping reference. Zapping it must not repeat those operations. Concurrent
MADV_DONTNEEDduring migration back to system memory can trigger this.Check the PMD entry type using
pmd_is_device_private_entry().Fix generated by an LLM and sent by Chris Mason in private.
Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")Signed-off-by Chris Mason <[email protected]>[ david: simplify description ]Reviewed-by Gregory Price (Meta) <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 4 lines
--- mm/huge_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Signature · 2 lines
Best Regards, Yan, Zi
Re: [PATCH v3 14/47] mm/huge_memory: fix zapping migration entries for device-private THPs
In reply to David Hildenbrand (Arm)Quoted 15 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
A PMD migration entry can refer to a device-private folio.
zap_huge_pmd_folio()checks the folio type instead of the entry type, so it treats such a migration entry as a device-private entry.Installing the migration entry already removed the reverse mapping and dropped the mapping reference. Zapping it must not repeat those operations. Concurrent
MADV_DONTNEEDduring migration back to system memory can trigger this.Check the PMD entry type using
pmd_is_device_private_entry().Fix generated by an LLM and sent by Chris Mason in private.
Quoted 7 lines
Fixes 368076f52ebe ("mm/huge_memory: add device-private THP support to PMD operations")Signed-off-by Chris Mason <[email protected]>[ david: simplify description ]Reviewed-by Gregory Price (Meta) <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
David Hildenbrand (Arm)
maintainerPATCH v3 15/47 mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
try_collapse_pte_mapped_thp() drops the PTE lock after it has cleared the page table, and takes the PMD lock and the PTE lock to remove it.
In between, MADV_DONTNEED with CONFIG_PT_RECLAIM can reclaim the page table. The pmd_same() check is supposed to handle this, but it is only done with CONFIG_SPLIT_PTE_PTLOCKS.
Fix it by performing the check unconditionally.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/khugepaged.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
mm/khugepaged.c +4 -5
@@ -2013,12 +2013,11 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign/* step 4: remove empty page table */ if (!pml) { pml =pmd_lock (mm, pmd); -if (ptl != pml) { +if (ptl != pml)spin_lock_nested (ptl,SINGLE_DEPTH_NESTING ); -if (unlikely (!pmd_same (pgt_pmd,pmdp_get_lockless (pmd)))) { -flush_tlb_mm (mm); - goto unlock; - } +if (unlikely (!pmd_same (pgt_pmd,pmdp_get_lockless (pmd)))) { +flush_tlb_mm (mm); + goto unlock; } } pgt_pmd =pmdp_collapse_flush (vma, haddr, pmd);
Signature · 1 line
2.43.0
Re: [PATCH v3 15/47] mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp()
Quoted 21 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
try_collapse_pte_mapped_thp()drops the PTE lock after it has cleared the page table, and takes the PMD lock and the PTE lock to remove it.In between,
MADV_DONTNEEDwithCONFIG_PT_RECLAIMcan reclaim the page table. Thepmd_same()check is supposed to handle this, but it is only done withCONFIG_SPLIT_PTE_PTLOCKS.Fix it by performing the check unconditionally.
Fix generated by an LLM and sent by Josef Bacik in private.
Fixes 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")Signed-off-by Josef Bacik <[email protected]>[ david: adjust description ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
Nothing jumped out at me, so:
Re: [PATCH v3 15/47] mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)Quoted 21 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
try_collapse_pte_mapped_thp()drops the PTE lock after it has cleared the page table, and takes the PMD lock and the PTE lock to remove it.In between,
MADV_DONTNEEDwithCONFIG_PT_RECLAIMcan reclaim the page table. Thepmd_same()check is supposed to handle this, but it is only done withCONFIG_SPLIT_PTE_PTLOCKS.Fix it by performing the check unconditionally.
Fix generated by an LLM and sent by Josef Bacik in private.
Fixes 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")Signed-off-by Josef Bacik <[email protected]>[ david: adjust description ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM.
David Hildenbrand (Arm)
maintainerPATCH v3 16/47 mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
For a private mapping with userfaultfd, try_collapse_pte_mapped_thp() takes the PMD table lock before it maps the page table for the second time. If that (pte_offset_map_rw_nolock) fails, because the page table vanished in the meantime due to concurrent page table reclaim, it leaves through the abort label, where the PMD table lock is unlocked if it differs from PTE table lock.
In case the PMD table lock and the PTE table lock match, which is the case without CONFIG_SPLIT_PTE_PTLOCKS after pte_offset_map_rw_nolock() failed, we miss to unlock the PMD table.
To fix it, forget the PTE table lock when we unmap+unlock the PTE table.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/khugepaged.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
mm/khugepaged.c +3 -2
@@ -1904,7 +1904,7 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign result =SCAN_FAIL ; start_pte =pte_offset_map_lock (mm, pmd, haddr, &ptl); -if (!start_pte)/* mmap_lock + page lock should prevent this */ +if (!start_pte) goto drop_folio;/* step 1: check all mapped PTEs are to the right huge page */
@@ -1935,6 +1935,7 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign }pte_unmap_unlock (start_pte, ptl); + ptl =NULL ;mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , mm, haddr, haddr +HPAGE_PMD_SIZE );mmu_notifier_invalidate_range_start (&range);
@@ -1952,7 +1953,7 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign pml =pmd_lock (mm, pmd); start_pte =pte_offset_map_rw_nolock (mm, pmd, haddr, &pgt_pmd, &ptl); -if (!start_pte)/* mmap_lock + page lock should prevent this */ +if (!start_pte) goto abort;if (!pml)spin_lock (ptl);
Signature · 1 line
2.43.0
Re: [PATCH v3 16/47] mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp()
Quoted 25 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
For a private mapping with userfaultfd,
try_collapse_pte_mapped_thp()takes the PMD table lock before it maps the page table for the second time. If that (pte_offset_map_rw_nolock) fails, because the page table vanished in the meantime due to concurrent page table reclaim, it leaves through the abort label, where the PMD table lock is unlocked if it differs from PTE table lock.In case the PMD table lock and the PTE table lock match, which is the case without
CONFIG_SPLIT_PTE_PTLOCKSafterpte_offset_map_rw_nolock()failed, we miss to unlock the PMD table.To fix it, forget the PTE table lock when we unmap+unlock the PTE table.
Fix generated by an LLM and sent by Josef Bacik in private.
Fixes 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")Signed-off-by Josef Bacik <[email protected]>[ david: move the ptl = NULL handling; rewrite description, drop wrong comments ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
Re: [PATCH v3 16/47] mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)Quoted 25 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
For a private mapping with userfaultfd,
try_collapse_pte_mapped_thp()takes the PMD table lock before it maps the page table for the second time. If that (pte_offset_map_rw_nolock) fails, because the page table vanished in the meantime due to concurrent page table reclaim, it leaves through the abort label, where the PMD table lock is unlocked if it differs from PTE table lock.In case the PMD table lock and the PTE table lock match, which is the case without
CONFIG_SPLIT_PTE_PTLOCKSafterpte_offset_map_rw_nolock()failed, we miss to unlock the PMD table.To fix it, forget the PTE table lock when we unmap+unlock the PTE table.
Fix generated by an LLM and sent by Josef Bacik in private.
Fixes 6375e95f381e ("mm: pgtable: reclaim empty PTE page in madvise(MADV_DONTNEED)")Signed-off-by Josef Bacik <[email protected]>[ david: move the ptl = NULL handling; rewrite description, drop wrong comments ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM.
David Hildenbrand (Arm)
maintainerPATCH v3 17/47 mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
try_collapse_pte_mapped_thp() clears PTEs without transferring their dirty bits to the folio. A shmem read fault can map a clean folio writable, so a later store only dirties the PTE. Losing that dirty bit allows reclaim to discard modified data.
The assumption that regular files are read-only is also obsolete since commit 1b6444331bebb ("mm/khugepaged: enable clean pagecache folio collapse for writable files"). In any case, transferring PTE dirty bits to the folio when removing mappings (and while the folio is still mapped) is the right thing to do.
Use get_and_clear_ptes() to collect the dirty bits and mark the folio dirty before unmapping it. Update the comment.
This was reproduced with shmem using MADV_COLLAPSE followed by MADV_PAGEOUT.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/khugepaged.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
mm/khugepaged.c +3 -2
@@ -1995,9 +1995,10 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign/* * Must clear entry, or a racing truncate may re-remove it. * TLB flush can be left until pmdp_collapse_flush() does it. -* PTE dirty? Shmem page is already dirty; file is read-only. */ -clear_ptes (mm, addr, pte, nr_batch_ptes); + ptent =get_and_clear_ptes (mm, addr, pte, nr_batch_ptes); +if (pte_dirty (ptent)) +folio_mark_dirty (folio);folio_remove_rmap_ptes (folio, page, nr_batch_ptes, vma); nr_mapped_ptes += nr_batch_ptes; }
Signature · 1 line
2.43.0
Re: [PATCH v3 17/47] mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs
Quoted 40 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
try_collapse_pte_mapped_thp()clears PTEs without transferring their dirty bits to the folio. A shmem read fault can map a clean folio writable, so a later store only dirties the PTE. Losing that dirty bit allows reclaim to discard modified data.The assumption that regular files are read-only is also obsolete since commit
1b6444331bebb("mm/khugepaged: enable clean pagecache folio collapse for writable files"). In any case, transferring PTE dirty bits to the folio when removing mappings (and while the folio is still mapped) is the right thing to do.Use
get_and_clear_ptes()to collect the dirty bits and mark the folio dirty before unmapping it. Update the comment.This was reproduced with shmem using
MADV_COLLAPSEfollowed byMADV_PAGEOUT.Fix generated by an LLM and sent by Chris Mason in private.
Fixes 27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")Fixes 1b6444331bebb ("mm/khugepaged: enable clean pagecache folio collapse for writable files")Signed-off-by Chris Mason <[email protected]>[ david: adjust comment; simplify description; cover file-thp as well ]Reviewed-by Pedro Falcato <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
mm/khugepaged.c| 5 +++--1 file changed, 3 insertions(+), 2 deletions(-)mm/khugepaged.c +0
-0
mm/khugepaged.c static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign · line 1999
Quoted 7 lines
mm/khugepaged.c +1
-2@@ -1995,9 +1995,10 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign/* * Must clear entry, or a racing truncate may re-remove it. * TLB flush can be left until pmdp_collapse_flush() does it. -* PTE dirty? Shmem page is already dirty; file is read-only. */ -clear_ptes (mm, addr, pte, nr_batch_ptes); + ptent =get_and_clear_ptes (mm, addr, pte, nr_batch_ptes);
I went through get_and_clear_ptes() as well. It merges dirty bits across the batch, so we're covered even when the first PTE is clean and a later one is dirty.
LGTM!
Quoted 10 lines
mm/khugepaged.c +2
-0+if (pte_dirty (ptent)) +folio_mark_dirty (folio);folio_remove_rmap_ptes (folio, page, nr_batch_ptes, vma); nr_mapped_ptes += nr_batch_ptes; }Signature · 2 lines
-- 2.43.0
Re: [PATCH v3 17/47] mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs
In reply to David Hildenbrand (Arm)Quoted 31 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
try_collapse_pte_mapped_thp()clears PTEs without transferring their dirty bits to the folio. A shmem read fault can map a clean folio writable, so a later store only dirties the PTE. Losing that dirty bit allows reclaim to discard modified data.The assumption that regular files are read-only is also obsolete since commit
1b6444331bebb("mm/khugepaged: enable clean pagecache folio collapse for writable files"). In any case, transferring PTE dirty bits to the folio when removing mappings (and while the folio is still mapped) is the right thing to do.Use
get_and_clear_ptes()to collect the dirty bits and mark the folio dirty before unmapping it. Update the comment.This was reproduced with shmem using
MADV_COLLAPSEfollowed byMADV_PAGEOUT.Fix generated by an LLM and sent by Chris Mason in private.
Fixes 27e1f8273113 ("khugepaged: enable collapse pmd for pte-mapped THP")Fixes 1b6444331bebb ("mm/khugepaged: enable clean pagecache folio collapse for writable files")Signed-off-by Chris Mason <[email protected]>[ david: adjust comment; simplify description; cover file-thp as well ]Reviewed-by Pedro Falcato <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM.
David Hildenbrand (Arm)
maintainerPATCH v3 18/47 mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.
Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <linux/hugetlb.h> for now, as x86's flush_tlb_range() macro depends on hugetlb helpers. This dependency needs a separate cleanup.
Both issues this fixes are rather corner cases. Let's not CC stable for now.
--- mm/khugepaged.c | 64 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 33 deletions(-)
mm/khugepaged.c +31 -33
@@ -10,6 +10,7 @@#include <linux/mm_inline.h> #include <linux/kthread.h> #include <linux/khugepaged.h> +#include <linux/hugetlb.h> #include <linux/freezer.h> #include <linux/mman.h> #include <linux/hashtable.h>
@@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long areturn SCAN_SUCCEED ; } +static bool pte_table_empty (pte_t *pte) +{ +int i; + +for (i =0 ; i <HPAGE_PMD_NR ; i++, pte++) { +if (!pte_none (ptep_get (pte))) +return false; + } +return true; +} +static enum scan_result try_collapse_pte_mapped_thp (struct mm_struct *mm,unsigned long addr,bool install_pmd) {
@@ -1935,33 +1947,14 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign }pte_unmap_unlock (start_pte, ptl); - ptl =NULL ;mmu_notifier_range_init (&range,MMU_NOTIFY_CLEAR ,0 , mm, haddr, haddr +HPAGE_PMD_SIZE );mmu_notifier_invalidate_range_start (&range); notified = true; -/* -* pmd_lock covers a wider range than ptl, and (if split from mm's -* page_table_lock) ptl nests inside pml. The less time we hold pml, -* the better; but userfaultfd's mfill_atomic_pte() on a private VMA -* inserts a valid as-if-COWed PTE without even looking up page cache. -* So page lock of folio does not protect from it, so we must not drop -* ptl before pgt_pmd is removed, so uffd private needs pml taken now. -*/ -if (userfaultfd_armed (vma) && !(vma->vm_flags &VM_SHARED )) - pml =pmd_lock (mm, pmd); - - start_pte =pte_offset_map_rw_nolock (mm, pmd, haddr, &pgt_pmd, &ptl); + start_pte =pte_offset_map_lock (mm, pmd, haddr, &ptl);if (!start_pte) goto abort; -if (!pml) -spin_lock (ptl); -else if (ptl != pml) -spin_lock_nested (ptl,SINGLE_DEPTH_NESTING ); - -if (unlikely (!pmd_same (pgt_pmd,pmdp_get_lockless (pmd)))) - goto abort;/* step 2: clear page table and adjust rmap */ for (i = 0, addr = haddr, pte = start_pte; i < HPAGE_PMD_NR;
@@ -1992,10 +1985,7 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign nr_batch_ptes =folio_pte_batch (folio, pte, ptent, max_nr_batch_ptes); -/* -* Must clear entry, or a racing truncate may re-remove it. -* TLB flush can be left until pmdp_collapse_flush() does it. -*/ +/* Must clear entry, or a racing truncate may re-remove it. */ ptent =get_and_clear_ptes (mm, addr, pte, nr_batch_ptes);if (pte_dirty (ptent))folio_mark_dirty (folio);
@@ -2003,25 +1993,33 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign nr_mapped_ptes += nr_batch_ptes; } -if (!pml) -spin_unlock (ptl); -/* step 3: set proper refcount and mm_counters. */ if (nr_mapped_ptes) {folio_ref_sub (folio, nr_mapped_ptes);add_mm_counter (mm,mm_counter_file (folio), -nr_mapped_ptes); } -/* step 4: remove empty page table */ -if (!pml) { +/* step 4: acquire the PMD lock and remove the empty page table */ + pml =pmd_lockptr (mm, pmd); +if (ptl != pml && !spin_trylock (pml)) { +/* We must flush before dropping the PTL. */ +if (nr_mapped_ptes) +flush_tlb_range (vma, haddr, end); +pte_unmap_unlock (start_pte, ptl); + ptl =NULL ; + pml =pmd_lock (mm, pmd); + start_pte =pte_offset_map_rw_nolock (mm, pmd, haddr, &pgt_pmd, &ptl); +if (!start_pte) + goto unlock;if (ptl != pml)spin_lock_nested (ptl,SINGLE_DEPTH_NESTING ); -if (unlikely (!pmd_same (pgt_pmd,pmdp_get_lockless (pmd)))) { -flush_tlb_mm (mm); + +if (!pte_table_empty (start_pte)) goto unlock; - } } + +/* This also flushes the zapped folio ptes properly. */ pgt_pmd =pmdp_collapse_flush (vma, haddr, pmd);pmdp_get_lockless_sync ();pte_unmap_unlock (start_pte, ptl);
@@ -2042,7 +2040,7 @@ static enum scan_result try_collapse_pte_mapped_thp(struct mm_struct *mm, unsign goto drop_folio; abort:if (nr_mapped_ptes) { -flush_tlb_mm (mm); +flush_tlb_range (vma, haddr, end);folio_ref_sub (folio, nr_mapped_ptes);add_mm_counter (mm,mm_counter_file (folio), -nr_mapped_ptes); }
Signature · 1 line
2.43.0
Re: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
Quoted 9 lines · David Hildenbrand (Arm)
The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
Yep ;)
Taking the PMD lock up front would make the locking easier to follow, though we'd hold it across all the PTE zapping ;)
Quoted 9 lines
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <
linux/hugetlb.h> for now, as x86'sflush_tlb_range()macro depends on hugetlb helpers. This dependency needs a separate cleanup.Both issues this fixes are rather corner cases. Let's not CC stable for now.
Quoted 7 lines
Fixes f807123d578d ("mm: allow guard regions in file-backed and read-only mappings")Fixes 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()")Tested-by Gregory Price (Meta) <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
LGTM!
Re: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
In reply to David Hildenbrand (Arm)Quoted 42 lines · David Hildenbrand (Arm)
The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <
linux/hugetlb.h> for now, as x86'sflush_tlb_range()macro depends on hugetlb helpers. This dependency needs a separate cleanup.Both issues this fixes are rather corner cases. Let's not CC stable for now.
Fixes f807123d578d ("mm: allow guard regions in file-backed and read-only mappings")Fixes 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with mmap_read_lock()")Tested-by Gregory Price (Meta) <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/khugepaged.c | 64 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 33 deletions(-)mm/khugepaged.c +1
-0@@ -10,6 +10,7 @@#include <linux/mm_inline.h> #include <linux/kthread.h> #include <linux/khugepaged.h> +#include <linux/hugetlb.h> #include <linux/freezer.h> #include <linux/mman.h> #include <linux/hashtable.h>
mm/khugepaged.c static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long a · line 1840
Quoted 8 lines
mm/khugepaged.c +5
-0@@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long areturn SCAN_SUCCEED ; } +static bool pte_table_empty (pte_t *pte) +{ +int i; + +for (i =0 ; i <HPAGE_PMD_NR ; i++, pte++) {
Using PTRS_PER_PTE to iterate PTEs seems more straightforward? Otherwise LGTM.
David Hildenbrand (Arm)
maintainerRe: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
Quoted 49 lines · Baolin Wang
Quoted 46 lines · David Hildenbrand (Arm)
The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <
linux/hugetlb.h> for now, as x86'sflush_tlb_range()macro depends on hugetlb helpers. This dependency needs a separate cleanup.Both issues this fixes are rather corner cases. Let's not CC stable for now.
Fixes f807123d578d ("mm: allow guard regions in file-backed and read-onlymappings")
Fixes 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with
mmap_read_lock()")Tested-by Gregory Price (Meta) <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
mm/khugepaged.c| 64 ++++++++++++++++++++++++++++-----------------------------1 file changed, 31 insertions(+), 33 deletions(-)mm/khugepaged.c +1
-0@@ -10,6 +10,7 @@ #include <linux/mm_inline.h> #include <linux/kthread.h> #include <linux/khugepaged.h> +#include <linux/hugetlb.h> #include <linux/freezer.h> #include <linux/mman.h> #include <linux/hashtable.h>@@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct
vm_area_struct*vma, unsigned long a
returnSCAN_SUCCEED;
Quoted 8 lines
mm/khugepaged.c static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long a · line 1840
Quoted 6 lines
}
+static boolpte_table_empty(pte_t *pte)mm/khugepaged.c +4
-0+{ + int i; + + for (i = 0; i < HPAGE_PMD_NR; i++, pte++) {Using
PTRS_PER_PTEto iterate PTEs seems more straightforward? Otherwise LGTM.
The difference currently shouldn't matter.
I'll probably leave it as is for no such that I can move forward with upstreaming them soonish (having the code as is in -next for a couple of days).
But this might be reasonable to clean up alter, thanks!
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerRe: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
In reply to Baolin WangQuoted 49 lines · Baolin Wang
Quoted 46 lines · David Hildenbrand (Arm)
The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <
linux/hugetlb.h> for now, as x86'sflush_tlb_range()macro depends on hugetlb helpers. This dependency needs a separate cleanup.Both issues this fixes are rather corner cases. Let's not CC stable for now.
Fixes f807123d578d ("mm: allow guard regions in file-backed and read-onlymappings")
Fixes 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with
mmap_read_lock()")Tested-by Gregory Price (Meta) <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
mm/khugepaged.c| 64 ++++++++++++++++++++++++++++-----------------------------1 file changed, 31 insertions(+), 33 deletions(-)mm/khugepaged.c +1
-0@@ -10,6 +10,7 @@ #include <linux/mm_inline.h> #include <linux/kthread.h> #include <linux/khugepaged.h> +#include <linux/hugetlb.h> #include <linux/freezer.h> #include <linux/mman.h> #include <linux/hashtable.h>@@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct
vm_area_struct*vma, unsigned long a
returnSCAN_SUCCEED;
Quoted 8 lines
mm/khugepaged.c static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long a · line 1840
Quoted 6 lines
}
+static boolpte_table_empty(pte_t *pte)mm/khugepaged.c +4
-0+{ + int i; + + for (i = 0; i < HPAGE_PMD_NR; i++, pte++) {Using
PTRS_PER_PTEto iterate PTEs seems more straightforward? Otherwise LGTM.
As I pulled this patch out from the stable fixes, I fixed it up.
I realized that this helper should likely move to some header later and be reused elsewehere (zap_pte_table_if_empty), but that's something for a later cleanup.
Signature · 3 lines
Cheers, David
Baolin Wang
reviewerRe: [PATCH v3 18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp()
Quoted 59 lines · David Hildenbrand (Arm)
Quoted 49 lines · Baolin Wang
Quoted 46 lines · David Hildenbrand (Arm)
The existing code is rather horrible and already resulted in bugs in the
past: we special-case UFFD inserting anonymous folios by grabbing the PMD lock early, but ignore any other markers (guard, poison through uffd) that could have been inserted into the page table in the meantime, effectively dropping them.Let's just unify and simplify the locking: try to grab the PMD table lock and if that fails, flush the TLB, drop the PTE lock, take PT locks in the right order and check if the page table is still empty.
We still perform the heavy lifting (zapping the folio ptes) under PTE table lock only.
Include <
linux/hugetlb.h> for now, as x86'sflush_tlb_range()macro depends on hugetlb helpers. This dependency needs a separate cleanup.Both issues this fixes are rather corner cases. Let's not CC stable for now.
Fixes f807123d578d ("mm: allow guard regions in file-backed and read-onlymappings")
Fixes 1043173eb5eb ("mm/khugepaged: collapse_pte_mapped_thp() with
mmap_read_lock()")Tested-by Gregory Price (Meta) <[email protected]>Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/khugepaged.c | 64 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 31 insertions(+), 33 deletions(-)mm/khugepaged.c +1
-0@@ -10,6 +10,7 @@ #include <linux/mm_inline.h> #include <linux/kthread.h> #include <linux/khugepaged.h> +#include <linux/hugetlb.h> #include <linux/freezer.h> #include <linux/mman.h> #include <linux/hashtable.h>@@ -1832,6 +1833,17 @@ static enum scan_result set_huge_pmd(struct
vm_area_struct*vma, unsigned long a
returnSCAN_SUCCEED;Quoted 8 lines
mm/khugepaged.c static enum scan_result set_huge_pmd(struct vm_area_struct *vma, unsigned long a · line 1840
Quoted 6 lines
}
+static boolpte_table_empty(pte_t *pte)mm/khugepaged.c +4
-0+{ + int i; + + for (i = 0; i < HPAGE_PMD_NR; i++, pte++) {Using
PTRS_PER_PTEto iterate PTEs seems more straightforward? Otherwise LGTM.
Quoted 1 line
As I pulled this patch out from the stable fixes, I fixed it up.
Thanks.
Quoted 3 lines
I realized that this helper should likely move to some header later and be reused elsewehere (
zap_pte_table_if_empty), but that's something for a later cleanup.
Indeed.
David Hildenbrand (Arm)
maintainerPATCH v3 19/47 mm/gup: recheck the PUD under PTL in follow_pud_mask()
In reply to David Hildenbrand (Arm)follow_pud_mask() reads the PUD without a lock. If it is a leaf, it takes the lock and calls follow_huge_pud(), which reads the entry again, but only tests whether it is present.
A leaf can be zapped concurrently and a page fault can fill the empty entry with a page table.
This currently only applies when device-dax makes use of PUD entries, and the fault after e.g., MADV_DONTNEED would find THPs to be disabled.
That requires a sequence like
1) Fault in the PUD
2) Disable THP
3) Have GUP race against concurrent MADV_DONTNEED + refault
Which is rather unlikely to happen in practice, which is why we never received a report about such a problem.
Before commit 1b1676180246 ("mm/gup: handle huge pud for follow_pud_mask()"), we performed that check in follow_devmap_pud() by testing for pud_devmap().
Let's fix it by just structuring follow_pud_mask() similar to follow_pmd_mask(), minus protnone stuff that does not apply and minus FOLL_SPLIT_PMD handling. That makes it much easier to spot any such inconsistencies.
Perform the pud_bad() check early in follow_pmd_mask() and just forward the PUD value. We can drop the pud_present() check in follow_huge_pud(), just like we have in follow_huge_pmd().
Even though we are performing more changes than strictly necessary, the fix should be easy to backport.
--- mm/gup.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-)
mm/gup.c +18 -14
@@ -658,9 +658,6 @@ static struct page *follow_huge_pud(struct vm_area_struct *vma,assert_spin_locked (pud_lockptr (mm, pudp)); -if (!pud_present (pud)) -return NULL ; -if ((flags &FOLL_WRITE ) && !can_follow_write_pud (pud,pfn_to_page (pfn), vma, flags))return NULL ;
@@ -896,15 +893,17 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, }static struct page *follow_pmd_mask (struct vm_area_struct *vma, -unsigned long address,pud_t *pudp, -unsigned int flags, -unsigned long *page_mask) +unsigned long address,pud_t *pudp,pud_t pud, +unsigned int flags,unsigned long *page_mask) {pmd_t *pmd, pmdval;spinlock_t *ptl;struct page *page;struct mm_struct *mm = vma->vm_mm; +if (unlikely (pud_bad (pud))) +return no_page_table (vma, flags, address); + pmd =pmd_offset (pudp, address); pmdval =pmdp_get_lockless (pmd);if (pmd_none (pmdval))
@@ -953,18 +952,23 @@ static struct page *follow_pud_mask(struct vm_area_struct *vma, pud =pudp_get (pudp);if (!pud_present (pud))return no_page_table (vma, flags, address); -if (pud_leaf (pud)) { - ptl =pud_lock (mm, pudp); - page =follow_huge_pud (vma, address, pudp, flags, page_mask); +if (!pud_leaf (pud)) +return follow_pmd_mask (vma, address, pudp, pud, flags, page_mask); + + ptl =pud_lock (mm, pudp); + pud = *pudp; +if (unlikely (!pud_present (pud))) {spin_unlock (ptl); -if (page) -return page;return no_page_table (vma, flags, address); } -if (unlikely (pud_bad (pud))) -return no_page_table (vma, flags, address); +if (unlikely (!pud_leaf (pud))) { +spin_unlock (ptl); +return follow_pmd_mask (vma, address, pudp, pud, flags, page_mask); + } -return follow_pmd_mask (vma, address, pudp, flags, page_mask); + page =follow_huge_pud (vma, address, pudp, flags, page_mask); +spin_unlock (ptl); +return page; } static struct page *follow_p4d_mask(struct vm_area_struct *vma,
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 20/47 mm/gup: fix clearing folios in memfd_pin_folios()
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
memfd_pin_folios() pins folios obtained directly from the page cache without checking whether they are uptodate. However, shmem fallocate() can leave newly allocated folios uncleared and not uptodate. Callers such as udmabuf can then expose uninitialized memory to userspace.
Clear non-uptodate folios and mark them uptodate under the folio lock before returning them to the caller.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/gup.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
mm/gup.c +14 -0
@@ -3527,6 +3527,20 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end, goto err; } +/* +* fallocate() can leave folios in the +* page cache that were never cleared. +*/ +if (!folio_test_uptodate (folio)) { +folio_lock (folio); +if (!folio_test_uptodate (folio)) { +folio_zero_range (folio,0 ,folio_size (folio)); +flush_dcache_folio (folio); +folio_mark_uptodate (folio); + } +folio_unlock (folio); + } +if (nr_folios ==0 ) *offset =offset_in_folio (folio, start);
Signature · 1 line
2.43.0
Re: [PATCH v3 20/47] mm/gup: fix clearing folios in memfd_pin_folios()
Quoted 6 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
memfd_pin_folios()pins folios obtained directly from the page cache without checking whether they are uptodate. However, shmemfallocate()can leave newly allocated folios uncleared and not uptodate. Callers such as udmabuf can then expose uninitialized memory to userspace.
Right, just make sure the folios are initialized before handing them to callers. They shouldn't have to care whether shmem deferred the clearing ;)
Quoted 14 lines
Clear non-uptodate folios and mark them uptodate under the folio lock before returning them to the caller.
Fix generated by an LLM and sent by Chris Mason in private.
Fixes 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")Signed-off-by Chris Mason <[email protected]>[ david: simplify description and comment ]Reviewed-by Pratyush Yadav (Google) <[email protected]>Reviewed-by Samiullah Khawaja <[email protected]>Tested-by Samiullah Khawaja <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
LGTM!
David Hildenbrand (Arm)
maintainerPATCH v3 21/47 mm/gup: fix out-of-bounds write for nr_pages == 0 in GUP-fast
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
With nr_pages == 0, GUP-fast still enters its page table walk and can write past the caller's page array. The old end <= start check caught empty ranges, but was replaced with an overflow check.
Users such as OrangeFS can pass a zero page count derived from a userspace-supplied length. So let's return zero for nr_pages == 0 before starting the walk, just like GUP-slow.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/gup.c | 3 +++ 1 file changed, 3 insertions(+)
mm/gup.c +3 -0
@@ -3215,6 +3215,9 @@ static int gup_fast_fallback(unsigned long start, unsigned long nr_pages, FOLL_PCI_P2PDMA | FOLL_HONOR_NUMA_FAULT)))return -EINVAL ; +if (unlikely (!nr_pages)) +return 0 ; +if (gup_flags &FOLL_PIN )mm_set_has_pinned_flag (current->mm);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 22/47 mm/gup: check for overflow when converting nr_pages to bytes in GUP-fast
In reply to David Hildenbrand (Arm)The overflow check in gup_fast_fallback() only covers the addition to start. The preceding nr_pages << PAGE_SHIFT can already have truncated the length on 32bit.
Users such as RapidIO's rio_dma_transfer() can derive such page counts from a 64-bit userspace length.
Let's just check_shl_overflow() to reject any such overflow with -EOVERFLOW early.
--- mm/gup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/gup.c +2 -2
@@ -3225,8 +3225,8 @@ static int gup_fast_fallback(unsigned long start, unsigned long nr_pages,might_lock_read (¤t->mm->mmap_lock); start =untagged_addr (start) &PAGE_MASK ; - len = nr_pages <<PAGE_SHIFT ; -if (check_add_overflow (start, len, &end)) +if (check_shl_overflow (nr_pages,PAGE_SHIFT , &len) || +check_add_overflow (start, len, &end))return -EOVERFLOW ;if (end >TASK_SIZE_MAX )return -EFAULT ;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 23/47 mm/userfaultfd: write-lock the VMA before waking waiters when unregistering
In reply to David Hildenbrand (Arm)userfaultfd_unregister() wakes pending missing faults before clearing the VMA's userfaultfd registration. With per-VMA fault locking, holding mmap_lock for write does not prevent another fault from queuing after the wakeup but before userfaultfd_clear_vma() write-locks the VMA. That waiter can then remain asleep indefinitely.
Call vma_start_write() before waking waiters. Existing fault handlers queue themselves before dropping the VMA read lock, and new readers are excluded until mmap_write_unlock().
--- mm/userfaultfd.c | 3 +++ 1 file changed, 3 insertions(+)
mm/userfaultfd.c +3 -0
@@ -3991,6 +3991,9 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, start = vma->vm_start; vma_end =min (end, vma->vm_end); +/* Prevent new userfaults from being queued after the wakeup. */ +vma_start_write (vma); +if (userfaultfd_missing (vma)) {/* * Wake any concurrent pending userfault while
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 24/47 mm/userfaultfd: wake all waiters on unregister
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
UFFDIO_UNREGISTER only wakes pending faults if VM_UFFD_MISSING is set. Threads waiting on write-protect, minor or RWP faults can therefore remain asleep after unregistering a range that does not track missing faults.
Wake pending faults regardless of the registration mode so they can retry without an explicit UFFDIO_WAKE or closing the userfaultfd. This matches the existing behavior for missing faults, including removing unread fault messages for the woken waiters.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/userfaultfd.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
mm/userfaultfd.c +10 -12
@@ -3977,6 +3977,8 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, ret =0 ;for_each_vma_range (vmi, vma, end) { +struct userfaultfd_wake_range range; +cond_resched ();/* VMA not registered with userfaultfd. */
@@ -3994,18 +3996,14 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,/* Prevent new userfaults from being queued after the wakeup. */ vma_start_write (vma); -if (userfaultfd_missing (vma)) { -/* -* Wake any concurrent pending userfault while -* we unregister, so they will not hang -* permanently and it avoids userland to call -* UFFDIO_WAKE explicitly. -*/ -struct userfaultfd_wake_range range; - range.start = start; - range.len = vma_end - start; -wake_userfault (vma->vm_userfaultfd_ctx.ctx, &range); - } +/* +* Wake any concurrent pending userfault while we unregister, +* so they will not hang permanently and it avoids userland to +* call UFFDIO_WAKE explicitly. +*/ + range.start = start; + range.len = vma_end - start; +wake_userfault (ctx, &range); vma =userfaultfd_clear_vma (&vmi, prev, vma, start, vma_end);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 25/47 mm/userfaultfd: wake waiters when registration modes are removed
In reply to David Hildenbrand (Arm)UFFDIO_REGISTER can replace MISSING or MINOR tracking without waking faults that are already queued. For example, switching an anonymous range from MISSING to WP leaves a missing-fault waiter asleep until userspace explicitly wakes it.
When a registration change removes a UFFD mode, wake waiters in the affected interval after updating the VMA. The VMA is write-locked at this point, so no new faults can queue under the old modes and woken threads cannot retry until mmap_write_unlock(). Waiters for retained modes in the same interval may also wake and refault.
The hang reproduces for MISSING-to-WP and MINOR-to-MISSING transitions, with both read and unread fault events.
--- mm/userfaultfd.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
mm/userfaultfd.c +20 -0
@@ -2270,6 +2270,9 @@ static struct vm_area_struct *userfaultfd_clear_vma(struct vma_iterator *vmi,return ret; } +static void wake_userfault_range (struct userfaultfd_ctx *ctx, +unsigned long start,unsigned long end); +/* Assumes mmap write lock taken, and mm_struct pinned. */ static int userfaultfd_register_range(struct userfaultfd_ctx *ctx, struct vm_area_struct *vma,
@@ -2287,6 +2290,8 @@ static int userfaultfd_register_range(struct userfaultfd_ctx *ctx, prev = vma;for_each_vma_range (vmi, vma, end) { +const bool any_mode_removed = vma->vm_flags & __VM_UFFD_FLAGS & ~vm_flags; +cond_resched ();VM_WARN_ON_ONCE (!vma_can_userfault (vma, vm_flags, wp_async));
@@ -2332,6 +2337,10 @@ static int userfaultfd_register_range(struct userfaultfd_ctx *ctx,*/ userfaultfd_set_ctx (vma, ctx, vm_flags); +/* Retry faults for removed modes while the VMA is write-locked. */ +if (any_mode_removed) +wake_userfault_range (ctx, start, vma_end); +if (vma_is_hugetlb (vma) &&uffd_disable_huge_pmd_share (vma))hugetlb_unshare_all_pmds (vma);
@@ -3648,6 +3657,17 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx,__wake_userfault (ctx, range); } +static void wake_userfault_range (struct userfaultfd_ctx *ctx, +unsigned long start,unsigned long end) +{ +struct userfaultfd_wake_range range = { + .start = start, + .len = end - start, + }; + +wake_userfault (ctx, &range); +} + static __always_inline int validate_unaligned_range(struct mm_struct *mm,__u64 start,__u64 len) {
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 26/47 mm/userfaultfd: compare exact registration modes before skipping
In reply to David Hildenbrand (Arm)From: Gregory Price <[email protected]>
Changing registration from MISSING|WP to WP returns success but leaves MISSING enabled. The no-op check sees that WP is already set and skips the update, so later accesses to missing pages still wait for userspace.
Skip the update only when the current and requested UFFD modes match exactly.
--- mm/userfaultfd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
mm/userfaultfd.c +2 -1
@@ -2291,6 +2291,7 @@ static int userfaultfd_register_range(struct userfaultfd_ctx *ctx,for_each_vma_range (vmi, vma, end) {const bool any_mode_removed = vma->vm_flags & __VM_UFFD_FLAGS & ~vm_flags; +const vma_flags_t curr_vma_flags =vma_flags_and_mask (&vma->flags, __VMA_UFFD_FLAGS);cond_resched ();
@@ -2304,7 +2305,7 @@ static int userfaultfd_register_range(struct userfaultfd_ctx *ctx,* userfaultfd and with the right tracking mode too. */ if (vma->vm_userfaultfd_ctx.ctx == ctx && -vma_test_all_mask (vma, vma_flags)) +vma_flags_same_pair (&curr_vma_flags, &vma_flags)) goto skip;/*
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 27/47 mm/mmu_gather: fix flushing ranges with 4 GiB or larger
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
__tlb_adjust_range() takes the size of the range which has to be flushed as an unsigned int. Its callers, tlb_flush_{pte,pmd,pud,p4d}_range(), get and pass an unsigned long. Consequently, a range that is 4 GiB or larger gets truncated, possibly to 0.
On arm64 with hugetlb, a 64k kernel can have 16 GiB hugetlb folios mapped contiguously at the PMD level and flush them when unmapping, and a 16k kernel can flush 64 GiB PUDs through tlb_unshare_pmd_ptdesc(). So at least these two scenarios are affected.
Fix it by flushing the proper range instead of the truncated one by properly using "unsigned long" for range_size.
Fix generated by an LLM and sent by Josef Bacik in private.
--- include/asm-generic/tlb.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
include/asm-generic/tlb.h +1 -2
@@ -350,8 +350,7 @@ struct mmu_gather {void tlb_flush_mmu (struct mmu_gather *tlb);static inline void __tlb_adjust_range (struct mmu_gather *tlb, -unsigned long address, -unsigned int range_size) +unsigned long address,unsigned long range_size) { tlb->start =min (tlb->start, address); tlb->end =max (tlb->end, address + range_size);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 28/47 mm/mmu_notifier: finish non-blocking interval invalidation on error
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
When an ordinary MMU notifier rejects a non-blocking invalidation, the interval invalidation started beforehand is left unfinished: the caller skips mmu_notifier_invalidate_range_end(). Subsequent interval notifier operations can then wait forever.
The only in-tree trigger is nouveau SVM, which uses both notifier types and rejects non-blocking invalidations. Reaping via the OOM reaper or process_mrelease() can therefore leave the GPU fault worker stuck, holding a reference that prevents the dying process's address space and memory from being freed.
End the interval invalidation when an ordinary notifier fails a non-blocking invalidation, like mn_itree_invalidate() already does when an interval notifier fails. Make that handling clearer in the documentation of mmu_notifier_invalidate_range_start_nonblock().
Fix generated by an LLM and sent by Josef Bacik in private.
--- include/linux/mmu_notifier.h | 3 ++- mm/mmu_notifier.c | 17 +++++++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-)
include/linux/mmu_notifier.h +2 -1
@@ -483,7 +483,8 @@ mmu_notifier_invalidate_range_start(struct mmu_notifier_range *range)/* * This version of mmu_notifier_invalidate_range_start() avoids blocking, but it * can return an error if a notifier can't proceed without blocking, in which -* case you're not allowed to modify PTEs in the specified range. +* case you're not allowed to modify PTEs in the specified range or call +* mmu_notifier_invalidate_range_end(). * * This is mainly intended for OOM handling. */
mm/mmu_notifier.c +13 -4
@@ -569,16 +569,25 @@ int __mmu_notifier_invalidate_range_start(struct mmu_notifier_range *range) {struct mmu_notifier_subscriptions *subscriptions = range->mm->notifier_subscriptions; -int ret; +const bool has_itree = subscriptions->has_itree; +int ret =0 ; -if (subscriptions->has_itree) { +if (has_itree) { ret =mn_itree_invalidate (subscriptions, range);if (ret)return ret; }if (!hlist_empty (&subscriptions->list)) -return mn_hlist_invalidate_range_start (subscriptions, range); -return 0 ; + ret =mn_hlist_invalidate_range_start (subscriptions, range); + +/* +* On error, the non-blocking caller is not allowed to call +* invalidate_range_end(), as in mn_itree_invalidate(). +*/ +if (ret && has_itree && !mmu_notifier_range_blockable (range)) +mn_itree_inv_end (subscriptions); + +return ret; } static void
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 29/47 mm/mmu_notifier: set ->mm only after mmgrab()
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
Radeon uses a non-NULL notifier->mm to decide whether to call mmu_interval_notifier_remove() when freeing a buffer object. __mmu_interval_notifier_insert() sets ->mm before validating the range, so a wrapping userptr range can leave ->mm set after insertion fails. Cleanup then tries to remove an unregistered notifier for which no mm reference was acquired.
Set ->mm only after mmgrab(), once insertion can no longer fail. While we could fix radeon's code instead, setting ->mm on success only makes the code more robust.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/mmu_notifier.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
mm/mmu_notifier.c +5 -2
@@ -947,7 +947,6 @@ static int __mmu_interval_notifier_insert( struct mmu_notifier_subscriptions *subscriptions, unsigned long start, unsigned long length, const struct mmu_interval_notifier_ops *ops) { - interval_sub->mm = mm; interval_sub->ops = ops;RB_CLEAR_NODE (&interval_sub->interval_tree.rb); interval_sub->interval_tree.start = start;
@@ -964,8 +963,12 @@ static int __mmu_interval_notifier_insert(if (WARN_ON (atomic_read (&mm->mm_users) <=0 ))return -EINVAL ; -/* pairs with mmdrop in mmu_interval_notifier_remove() */ +/* +* Pairs with mmdrop in mmu_interval_notifier_remove(). Drivers take a +* non-NULL ->mm for "registered", so set it only with the reference. +*/ mmgrab (mm); + interval_sub->mm = mm;/* * If some invalidate_range_start/end region is going on in parallel
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 30/47 mm/mremap: use pudp_huge_get_and_clear() when moving huge PUDs
In reply to David Hildenbrand (Arm)move_huge_pud() reads the old PUD and clears it in separate operations. Hardware accessed or dirty updates between those operations can be lost when the saved entry is installed at the new address.
The raw pud_clear() also bypasses page_table_check_pud_clear(), while set_pud_at() accounts for the destination mapping. This leaves the checker counts unbalanced for mappings tracked by page table checking.
To fix it, simply use pudp_huge_get_and_clear().
--- mm/mremap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
mm/mremap.c +1 -2
@@ -509,8 +509,7 @@ static bool move_huge_pud(struct pagetable_move_control *pmc,spin_lock_nested (new_ptl,SINGLE_DEPTH_NESTING );/* Clear the pud */ - pud = *old_pud; -pud_clear (old_pud); + pud =pudp_huge_get_and_clear (mm, pmc->old_addr, old_pud);VM_BUG_ON (!pud_none (*new_pud));
Signature · 1 line
2.43.0
Re: [PATCH v3 30/47] mm/mremap: use pudp_huge_get_and_clear() when moving huge PUDs
Quoted 3 lines · David Hildenbrand (Arm)
move_huge_pud()reads the old PUD and clears it in separate operations. Hardware accessed or dirty updates between those operations can be lost when the saved entry is installed at the new address.
Yep. HW can still update the A/D bits while the PUD lock is held :)
Quoted 22 lines
The raw
pud_clear()also bypassespage_table_check_pud_clear(), whileset_pud_at()accounts for the destination mapping. This leaves the checker counts unbalanced for mappings tracked by page table checking.To fix it, simply use
pudp_huge_get_and_clear().Fixes 7d846db7d0e2 ("mm/mremap: convert huge PUD move to separate helper")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reviewed-by Pedro Falcato <[email protected]>Reviewed-by Dev Jain <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
mm/mremap.c| 3 +--1 file changed, 1 insertion(+), 2 deletions(-)mm/mremap.c +0
-0
mm/mremap.c static bool move_huge_pud(struct pagetable_move_control *pmc, · line 512
Quoted 6 lines
mm/mremap.c +1
-2@@ -509,8 +509,7 @@ static bool move_huge_pud(struct pagetable_move_control *pmc,spin_lock_nested (new_ptl,SINGLE_DEPTH_NESTING );/* Clear the pud */ - pud = *old_pud; -pud_clear (old_pud); + pud =pudp_huge_get_and_clear (mm, pmc->old_addr, old_pud);
Quoted 8 lines
mm/mremap.c +0
-0VM_BUG_ON (!pud_none (*new_pud));Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 31/47 mm/mremap: recheck PUDs under the page table lock
In reply to David Hildenbrand (Arm)Once get_old_pud() is fixed to accept PUD leaves, unmap_mapping_range() can clear a leaf between the lookup and the move. Holding mmap_lock in write mode does not prevent this.
If the zap happens before move_page_tables() tests the entry, an empty PUD can reach move_normal_pud(), where pud_populate() turns it into a present page table entry. If it happens after the test, move_huge_pud() can instead try to move an empty entry, triggering CONFIG_DEBUG_VM warnings on powerpc.
Recheck the source under the page table lock in both helpers. Treat an empty source in move_normal_pud() as a completed move, and use __pud_trans_huge_lock() in move_huge_pud().
In the future we should likely have the code lookup a PUD/PMD lockless once, instead of multiple times, because one of the problematic bits here is the:
pud_trans_huge(*old_pud)
--- mm/mremap.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
mm/mremap.c +8 -2
@@ -461,14 +461,18 @@ static bool move_normal_pud(struct pagetable_move_control *pmc,if (new_ptl != old_ptl)spin_lock_nested (new_ptl,SINGLE_DEPTH_NESTING ); -/* Clear the pud */ pud = *old_pud; +if (unlikely (pud_none (pud))) + goto out_unlock; + +/* Clear the pud */ pud_clear (old_pud);VM_BUG_ON (!pud_none (*new_pud));pud_populate (mm, new_pud,pud_pgtable (pud));flush_tlb_range (vma, pmc->old_addr, pmc->old_addr +PUD_SIZE ); +out_unlock:if (new_ptl != old_ptl)spin_unlock (new_ptl);spin_unlock (old_ptl);
@@ -503,7 +507,9 @@ static bool move_huge_pud(struct pagetable_move_control *pmc,* We don't have to worry about the ordering of src and dst * ptlocks because exclusive mmap_lock prevents deadlock. */ - old_ptl =pud_lock (mm, old_pud); + old_ptl =__pud_trans_huge_lock (old_pud, vma); +if (unlikely (!old_ptl)) +return false; new_ptl =pud_lockptr (mm, new_pud);if (new_ptl != old_ptl)spin_lock_nested (new_ptl,SINGLE_DEPTH_NESTING );
Signature · 1 line
2.43.0
Re: [PATCH v3 31/47] mm/mremap: recheck PUDs under the page table lock
Quoted 20 lines · David Hildenbrand (Arm)
Once
get_old_pud()is fixed to accept PUD leaves,unmap_mapping_range()can clear a leaf between the lookup and the move. Holdingmmap_lockin write mode does not prevent this.If the zap happens before
move_page_tables()tests the entry, an empty PUD can reachmove_normal_pud(), wherepud_populate()turns it into a present page table entry. If it happens after the test,move_huge_pud()can instead try to move an empty entry, triggeringCONFIG_DEBUG_VMwarnings on powerpc.Recheck the source under the page table lock in both helpers. Treat an empty source in
move_normal_pud()as a completed move, and use__pud_trans_huge_lock()inmove_huge_pud().In the future we should likely have the code lookup a PUD/PMD lockless once, instead of multiple times, because one of the problematic bits here is the:
pud_trans_huge(*old_pud)
Quoted 7 lines
Fixes 0881ace292b6 ("mm/mremap: use pmd/pud_poplulate to update page table entries")Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Reviewed-by Pedro Falcato <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
David Hildenbrand (Arm)
maintainerPATCH v3 32/47 mm/mremap: fix moving PUD leaves
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
pud_none_or_clear_bad() only really works on page tables. PUD leaves confuse the check and make us clear the page table entry and print a warning.
Since commit aed877c2b4257 ("device/dax: properly refcount device dax pages when mapping"), this also leaks device-DAX folio references, mapcounts and RSS accounting, resulting in "Bad rss-counter state" warnings on exit.
Allow PUD leaves in get_old_pud(). If a PUD leaf cannot be moved as a whole or the move fails, split it. This unmaps it with proper accounting instead of treating the leaf entry as a PMD table.
Keep using pud_trans_huge() for now, in the future this code should be cleaned up to just check for PUD leaves.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/mremap.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
mm/mremap.c +4 -5
@@ -87,7 +87,7 @@ static pud_t *get_old_pud(struct mm_struct *mm, unsigned long addr)return NULL ; pud =pud_offset (p4d, addr); -if (pud_none_or_clear_bad (pud)) +if (!pud_trans_huge (*pud) &&pud_none_or_clear_bad (pud))return NULL ;return pud;
@@ -847,11 +847,10 @@ unsigned long move_page_tables(struct pagetable_move_control *pmc)if (!new_pud)break ;if (pud_trans_huge (*old_pud)) { -if (extent ==HPAGE_PUD_SIZE ) { -move_pgt_entry (pmc,HPAGE_PUD , old_pud, new_pud); -/* We ignore and continue on error? */ +if (extent ==HPAGE_PUD_SIZE && +move_pgt_entry (pmc,HPAGE_PUD , old_pud, new_pud))continue ; - } +split_huge_pud (pmc->old, old_pud, pmc->old_addr); }else if (IS_ENABLED (CONFIG_HAVE_MOVE_PUD ) && extent ==PUD_SIZE ) {if (move_pgt_entry (pmc,NORMAL_PUD , old_pud, new_pud))continue ;
Signature · 1 line
2.43.0
Re: [PATCH v3 32/47] mm/mremap: fix moving PUD leaves
Quoted 46 lines · David Hildenbrand (Arm)
From: Josef Bacik <[email protected]>
pud_none_or_clear_bad()only really works on page tables. PUD leaves confuse the check and make us clear the page table entry and print a warning.Since commit
aed877c2b4257("device/dax: properly refcount device dax pages when mapping"), this also leaks device-DAX folio references, mapcounts and RSS accounting, resulting in "Bad rss-counter state" warnings on exit.Allow PUD leaves in
get_old_pud(). If a PUD leaf cannot be moved as a whole or the move fails, split it. This unmaps it with proper accounting instead of treating the leaf entry as a PMD table.Keep using
pud_trans_huge()for now, in the future this code should be cleaned up to just check for PUD leaves.Fix generated by an LLM and sent by Josef Bacik in private.
Fixes a00cc7d9dd93 ("mm, x86: add support for PUD-sized transparent hugepages")Signed-off-by Josef Bacik <[email protected]>Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>[ david: fallthrough after splitting ]Reviewed-by Pedro Falcato <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
mm/mremap.c| 9 ++++-----1 file changed, 4 insertions(+), 5 deletions(-)mm/mremap.c +1
-1@@ -87,7 +87,7 @@ static pud_t *get_old_pud(struct mm_struct *mm, unsigned long addr)return NULL ; pud =pud_offset (p4d, addr); -if (pud_none_or_clear_bad (pud)) +if (!pud_trans_huge (*pud) &&pud_none_or_clear_bad (pud))return NULL ;return pud;@@ -847,11 +847,10 @@ unsigned long move_page_tables(struct pagetable_move_control *pmc)if (!new_pud)
mm/mremap.c unsigned long move_page_tables(struct pagetable_move_control *pmc) · line 853
Quoted 10 lines
mm/mremap.c +3
-4break ;if (pud_trans_huge (*old_pud)) { -if (extent ==HPAGE_PUD_SIZE ) { -move_pgt_entry (pmc,HPAGE_PUD , old_pud, new_pud); -/* We ignore and continue on error? */ +if (extent ==HPAGE_PUD_SIZE && +move_pgt_entry (pmc,HPAGE_PUD , old_pud, new_pud))continue ; - } +split_huge_pud (pmc->old, old_pud, pmc->old_addr);
Right, copy_vma() already sets up the file and offset at the new address, so we can let subsequent faults rebuild the mapping.
No need to build a PMD table just for the move ;)
Quoted 8 lines
mm/mremap.c +0
-0}else if (IS_ENABLED (CONFIG_HAVE_MOVE_PUD ) && extent ==PUD_SIZE ) {if (move_pgt_entry (pmc,NORMAL_PUD , old_pud, new_pud))continue ;Signature · 2 lines
-- 2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 33/47 mm/memory_hotplug: avoid races in pfn_to_online_page() with dying pgmap
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
If ZONE_DEVICE memory shares a memory section with ordinary memory, the section is online and the pfns of the device are valid. pfn_to_online_page() tells them apart by asking get_dev_pagemap() whether the pfn belongs to a dev_pagemap.
get_dev_pagemap() only returns a dev_pagemap whose reference is alive. memunmap_pages() kills the reference first, waits until all pages are idle, and then removes the memmap. During all that time the function finds nothing, and pfn_to_online_page() hands out the struct pages of the device as if they were online memory. A pfn walker reads them while they are poisoned and freed.
The entry in pgmap_array is only deleted after the pages have been removed. Look at that instead of trying to get a reference.
This is probably not worth CCing stable.
Fix generated by an LLM and sent by Josef Bacik in private.
--- include/linux/memremap.h | 6 ++++++ mm/memory_hotplug.c | 13 +++++-------- mm/memremap.c | 13 +++++++++++++ 3 files changed, 24 insertions(+), 8 deletions(-)
include/linux/memremap.h +6 -0
@@ -231,6 +231,7 @@ void memunmap_pages(struct dev_pagemap *pgmap);void *devm_memremap_pages (struct device *dev,struct dev_pagemap *pgmap);void devm_memunmap_pages (struct device *dev,struct dev_pagemap *pgmap);struct dev_pagemap *get_dev_pagemap (unsigned long pfn); +bool pfn_has_dev_pagemap (unsigned long pfn);bool pgmap_pfn_valid (struct dev_pagemap *pgmap,unsigned long pfn);unsigned long memremap_compat_align (void );
@@ -284,6 +285,11 @@ static inline struct dev_pagemap *get_dev_pagemap(unsigned long pfn)return NULL ; } +static inline bool pfn_has_dev_pagemap (unsigned long pfn) +{ +return false; +} +static inline bool pgmap_pfn_valid (struct dev_pagemap *pgmap,unsigned long pfn) {return false;
mm/memory_hotplug.c +5 -8
@@ -342,7 +342,6 @@ static int check_pfn_span(unsigned long pfn, unsigned long nr_pages)struct page *pfn_to_online_page (unsigned long pfn) {unsigned long nr =pfn_to_section_nr (pfn); -struct dev_pagemap *pgmap;struct mem_section *ms;BUILD_BUG_ON (IS_ENABLED (CONFIG_HAVE_ARCH_PFN_VALID ));
@@ -363,14 +362,12 @@ struct page *pfn_to_online_page(unsigned long pfn)/* * Slowpath: when ZONE_DEVICE collides with * ZONE_{NORMAL,MOVABLE} within the same section some pfns in -* the section may be 'offline' but 'valid'. Only -* get_dev_pagemap() can determine sub-section online status. +* the section may be 'offline' but 'valid'. Only the presence of a +* pgmap can determine sub-section online status: it indicates a +* ZONE_DEVICE offline pfn. That includes a pgmap whose reference has +* been killed, its memmap is about to go away. */ - pgmap =get_dev_pagemap (pfn); -put_dev_pagemap (pgmap); - -/* The presence of a pgmap indicates ZONE_DEVICE offline pfn */ -if (pgmap) +if (pfn_has_dev_pagemap (pfn))return NULL ;return pfn_to_page (pfn);
mm/memremap.c +13 -0
@@ -413,6 +413,19 @@ struct dev_pagemap *get_dev_pagemap(unsigned long pfn) }EXPORT_SYMBOL_GPL (get_dev_pagemap); +/** +* pfn_has_dev_pagemap() - does @pfn belong to a dev_pagemap, dead or alive +* @pfn: page frame number to lookup page_map +* +* In contrast to get_dev_pagemap() this also finds a dev_pagemap which is on +* its way out: it is registered before its pages are added, and until they +* have been removed. +*/ +bool pfn_has_dev_pagemap (unsigned long pfn) +{ +return xa_load (&pgmap_array, pfn); +} +void free_zone_device_folio (struct folio *folio) {struct dev_pagemap *pgmap = folio->pgmap;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 34/47 mm/madvise: fix data loss from MADV_FREE on a KSM page
In reply to David Hildenbrand (Arm)MADV_FREE can clear the dirty flag of a singly mapped KSM folio. The folio remains in the stable tree and can gain new mappings without being dirtied again. Once marked lazyfree, reclaim can discard data that those mappings still need.
KSM always marks a folio dirty when converting it to a KSM folio in try_to_merge_one_page(). Marking a folio lazyfree clears PG_swapbacked but leaves PG_dirty unchanged. Preserving PG_dirty makes reclaim restore PG_swapbacked instead of discarding the folio. This also protects folios that were already lazyfree when KSM converted them.
Skip KSM folios before clearing their dirty flag, checking under the folio lock to serialize with KSM conversion.
--- mm/madvise.c | 9 +++++++++ 1 file changed, 9 insertions(+)
mm/madvise.c +9 -0
@@ -756,6 +756,15 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,if (folio_test_swapcache (folio) ||folio_test_dirty (folio)) {if (!folio_trylock (folio))continue ; +/* +* KSM folios can gain new mappings at any time, so we must +* not clear their dirty flag. +*/ +if (folio_test_ksm (folio)) { +folio_unlock (folio); +continue ; + } +/* * If we have a large folio at this point, we know it is * fully mapped so if its mapcount is the same as its
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 35/47 mm/memory: reject remote COW faults on PFN mappings
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
When handling a COW fault without a normal source page (PFN mappings), __wp_page_copy_user() copies from the faulting user address. For remote faults, this accesses the wrong address space, resulting in wrong data getting copied.
For example, user_events updates an enable bit in the registering process's memory. If that memory needs to be faulted in, user_event_enabler_fault_fixup() calls fixup_user_fault() from a work item. Placing the enable bit in a private PFN mapping can trigger this remote COW path.
Reject these faults with VM_FAULT_SIGBUS when the target mm differs from current->mm: remote faults. Ordinary (non-PFN) mappings are not affected.
The user-address copy dates back to commit 6aab341e0a28 ("mm: re-architect the VM_UNPAGED logic"); likely, actual ways to get remote faults in there on COW PFN mappings appeared afterwards. But the fix is simple and straight forward.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/memory.c | 9 +++++++++ 1 file changed, 9 insertions(+)
mm/memory.c +9 -0
@@ -3761,6 +3761,13 @@ static inline int __wp_page_copy_user(struct page *dst, struct page *src,return 0 ; } +/* +* __copy_from_user_inatomic() only works as expected for local faults. +* Reject remote faults. +*/ +if (unlikely (mm != current->mm)) +return -EFAULT ; +/* * If the source page was a PFN mapping, we don't have * a "struct page" for it. We do a best-effort copy by
@@ -4086,6 +4093,8 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)folio_put (old_folio);delayacct_wpcopy_end (); +if (err == -EFAULT ) +return VM_FAULT_SIGBUS ;return err == -EHWPOISON ?VM_FAULT_HWPOISON :0 ; }kmsan_copy_page_meta (&new_folio->page, vmf->page);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 36/47 mm: fix unmap_mapping_range() beyond 4 GiB on 32-bit
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
Commit 9eab0421fa94 ("mm: fix unmap_mapping_range high bits shift bug") made holebegin and holelen unsigned before they are shifted, by casting them to pgoff_t. That is an unsigned long: on a 32-bit kernel the cast cuts off the upper half of the two loff_t values, so a hole which begins at 4 GiB or beyond is taken for one in the first 4 GiB of the file.
truncate_pagecache() passes the new size of the file and asks for private COWed pages to be unmapped as well. If a file of 6 GiB is truncated to 5 GiB, everything from 1 GiB on is unmapped, and what a process has written to a private mapping of the range between 1 GiB and 5 GiB is
lost: it reads the contents of the file again.
Where the range has an end, the wrong range is unmapped instead of the right one. The page cache copes with that, as it unmaps every folio that it removes. The callers which only have unmap_mapping_range() to get rid of user mappings do not, for example drm_vma_node_unmap(), whose fake offsets reach beyond 4 GiB on 32-bit, and revoke_iomem() with physical addresses of more than 32 bits.
Cast to u64 instead. Nothing changes for 64-bit kernels.
Fix generated by an LLM and sent by Josef Bacik in private.
--- mm/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
mm/memory.c +2 -2
@@ -4649,8 +4649,8 @@ EXPORT_SYMBOL_GPL(unmap_mapping_pages);void unmap_mapping_range (struct address_space *mapping,loff_t const holebegin,loff_t const holelen,int even_cows) { -pgoff_t hba = (pgoff_t)(holebegin) >>PAGE_SHIFT ; -pgoff_t hlen = ((pgoff_t)(holelen) +PAGE_SIZE -1 ) >>PAGE_SHIFT ; +pgoff_t hba = (u64 )holebegin >>PAGE_SHIFT ; +pgoff_t hlen = ((u64 )holelen +PAGE_SIZE -1 ) >>PAGE_SHIFT ;/* Check for overflow. */ if (sizeof (holelen) >sizeof (hlen)) {
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 37/47 mm: fix unkillable fault loop on MAP_DROPPABLE memory
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
handle_mm_fault() suppresses VM_FAULT_OOM for VM_DROPPABLE mappings.
For user-mode faults, pending signals are processed before retrying the access. Kernel-mode faults on user buffers instead return directly to the faulting instruction.
With persistent allocation failures (esp. memcg limits), a syscall, such as write(), can therefore loop while not dropping locks.
Only suppress VM_FAULT_OOM when FAULT_FLAG_USER is set, allowing kernel-mode faults and GUP to fail and signal allocation errors.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/memory.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
mm/memory.c +6 -2
@@ -6957,8 +6957,12 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,lru_gen_exit_fault (); -/* If the mapping is droppable, then errors due to OOM aren't fatal. */ -if (is_droppable) +/* +* If the mapping is droppable, then errors due to OOM aren't fatal: the +* task returns to user mode, handles its signals and tries again. A +* fault from kernel mode has no such return, so it must see the error. +*/ +if (is_droppable && (flags &FAULT_FLAG_USER )) ret &= ~VM_FAULT_OOM ;if (flags &FAULT_FLAG_USER ) {
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 38/47 mm: fix deadlock of mremap() against UFFDIO_MOVE
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
With per-VMA locks, UFFDIO_MOVE can run concurrently with mremap() on different VMAs in the same mm. Those VMAs can share page tables, so both operations can acquire the same pair of page-table locks.
UFFDIO_MOVE orders the locks by address, while mremap() takes the source lock first. These conflicting orders can deadlock; mremap() holding mmap_lock for writing no longer helps.
Move double_pt_lock() and double_pt_unlock() to mm/internal.h and use them for all page-table moves in mremap(). Preserve the source entry checks after acquiring both locks. Remove the comments claiming that exclusive mmap_lock makes lock ordering irrelevant.
In move_ptes(), make sure to just recheck source and destination PMD after taking the locks and drop the overly-long comment.
Fix generated by an LLM and sent by Chris Mason in private.
[ david: use new helpers consistently; simplify description and comments;
move_ptes() fixup ]
--- include/linux/userfaultfd_k.h | 2 - mm/huge_memory.c | 19 ++++------ mm/hugetlb.c | 14 ++----- mm/internal.h | 28 ++++++++++++++ mm/mremap.c | 87 +++++++++++++++++-------------------------- mm/userfaultfd.c | 27 -------------- 6 files changed, 72 insertions(+), 105 deletions(-)
include/linux/userfaultfd_k.h +0 -2
@@ -153,8 +153,6 @@ extern int mrwprotect_range(struct userfaultfd_ctx *ctx, unsigned long start, unsigned long len, bool enable_rwp);/* move_pages */ -void double_pt_lock (spinlock_t *ptl1,spinlock_t *ptl2); -void double_pt_unlock (spinlock_t *ptl1,spinlock_t *ptl2); int move_pages_huge_pmd(struct mm_struct *mm, pmd_t *dst_pmd, pmd_t *src_pmd, pmd_t dst_pmdval, struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
mm/huge_memory.c +7 -12
@@ -2646,15 +2646,11 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,return false; } -/* -* We don't have to worry about the ordering of src and dst -* ptlocks because exclusive mmap_lock prevents deadlock. -*/ - old_ptl =__pmd_trans_huge_lock (old_pmd, vma); -if (old_ptl) { - new_ptl =pmd_lockptr (mm, new_pmd); -if (new_ptl != old_ptl) -spin_lock_nested (new_ptl,SINGLE_DEPTH_NESTING ); + old_ptl =pmd_lockptr (mm, old_pmd); + new_ptl =pmd_lockptr (mm, new_pmd); + +double_pt_lock (old_ptl, new_ptl); +if (likely (pmd_is_huge (*old_pmd))) { pmd =pmdp_huge_get_and_clear (mm, old_addr, old_pmd);if (pmd_present (pmd)) force_flush = true;
@@ -2683,11 +2679,10 @@ bool move_huge_pmd(struct vm_area_struct *vma, unsigned long old_addr,set_pmd_at (mm, new_addr, new_pmd, pmd);if (force_flush)flush_pmd_tlb_range (vma, old_addr, old_addr +PMD_SIZE ); -if (new_ptl != old_ptl) -spin_unlock (new_ptl); -spin_unlock (old_ptl); +double_pt_unlock (old_ptl, new_ptl);return true; } +double_pt_unlock (old_ptl, new_ptl);return false; }
mm/hugetlb.c +3 -11
@@ -5153,15 +5153,9 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,spinlock_t *src_ptl, *dst_ptl;pte_t pte; - dst_ptl =huge_pte_lock (h, mm, dst_pte); + dst_ptl =huge_pte_lockptr (h, mm, dst_pte); src_ptl =huge_pte_lockptr (h, mm, src_pte); - -/* -* We don't have to worry about the ordering of src and dst ptlocks -* because exclusive mmap_lock (or the i_mmap_lock) prevents deadlock. -*/ -if (src_ptl != dst_ptl) -spin_lock_nested (src_ptl,SINGLE_DEPTH_NESTING ); +double_pt_lock (dst_ptl, src_ptl); pte =huge_ptep_get_and_clear (mm, old_addr, src_pte, sz);
@@ -5190,9 +5184,7 @@ static void move_huge_pte(struct vm_area_struct *vma, unsigned long old_addr,set_huge_pte_at (mm, new_addr, dst_pte, pte, sz); } -if (src_ptl != dst_ptl) -spin_unlock (src_ptl); -spin_unlock (dst_ptl); +double_pt_unlock (dst_ptl, src_ptl); } int move_hugetlb_page_tables(struct vm_area_struct *vma,
mm/internal.h +28 -0
@@ -624,6 +624,34 @@ static inline void force_page_cache_readahead(struct address_space *mapping,force_page_cache_ra (&ractl, nr_to_read); } +/* +* Take two page table locks at the same level in address order such that +* we cannot deadlock. +*/ +static inline void double_pt_lock (spinlock_t *ptl1,spinlock_t *ptl2) +__acquires (ptl1) +__acquires (ptl2) +{ +if (ptl1 > ptl2) +swap (ptl1, ptl2); +spin_lock (ptl1); +if (ptl1 != ptl2) +spin_lock_nested (ptl2,SINGLE_DEPTH_NESTING ); +else +__acquire (ptl2); +} + +static inline void double_pt_unlock (spinlock_t *ptl1,spinlock_t *ptl2) +__releases (ptl1) +__releases (ptl2) +{ +spin_unlock (ptl1); +if (ptl1 != ptl2) +spin_unlock (ptl2); +else +__release (ptl2); +} +unsigned find_lock_entries (struct address_space *mapping,pgoff_t *start,pgoff_t end,struct folio_batch *fbatch,pgoff_t *indices); unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
mm/mremap.c +34 -53
153 lines not on this page · raw message
mm/userfaultfd.c +0 -27
@@ -1206,33 +1206,6 @@ int mrwprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,return 0 ; } -void double_pt_lock (spinlock_t *ptl1, -spinlock_t *ptl2) -__acquires (ptl1) -__acquires (ptl2) -{ -if (ptl1 > ptl2) -swap (ptl1, ptl2); -/* lock in virtual address order to avoid lock inversion */ -spin_lock (ptl1); -if (ptl1 != ptl2) -spin_lock_nested (ptl2,SINGLE_DEPTH_NESTING ); -else -__acquire (ptl2); -} - -void double_pt_unlock (spinlock_t *ptl1, -spinlock_t *ptl2) -__releases (ptl1) -__releases (ptl2) -{ -spin_unlock (ptl1); -if (ptl1 != ptl2) -spin_unlock (ptl2); -else -__release (ptl2); -} -static inline bool is_pte_pages_stable (pte_t *dst_pte,pte_t *src_pte,pte_t orig_dst_pte,pte_t orig_src_pte,pmd_t *dst_pmd,pmd_t dst_pmdval)
Signature · 1 line
2.43.0
Re: [PATCH v3 38/47] mm: fix deadlock of mremap() against UFFDIO_MOVE
Quoted 23 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
With per-VMA locks,
UFFDIO_MOVEcan run concurrently withmremap()on different VMAs in the same mm. Those VMAs can share page tables, so both operations can acquire the same pair of page-table locks.
UFFDIO_MOVEorders the locks by address, whilemremap()takes the source lock first. These conflicting orders can deadlock;mremap()holdingmmap_lockfor writing no longer helps.Move
double_pt_lock()anddouble_pt_unlock()tomm/internal.hand use them for all page-table moves inmremap(). Preserve the source entry checks after acquiring both locks. Remove the comments claiming that exclusivemmap_lockmakes lock ordering irrelevant.In
move_ptes(), make sure to just recheck source and destination PMD after taking the locks and drop the overly-long comment.Fix generated by an LLM and sent by Chris Mason in private.
[ david: use new helpers consistently; simplify description and comments;
move_ptes()fixup ]
Quoted 8 lines
Fixes 867a43a34ff8 ("userfaultfd: use per-vma locks in userfaultfd operations")Signed-off-by Chris Mason <[email protected]>Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Acked-by Muchun Song <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
Nothing jumped out at me, so:
David Hildenbrand (Arm)
maintainerPATCH v3 39/47 mm/ksm: reset the unstable trees of all nodes
In reply to David Hildenbrand (Arm)Each full scan resets ksm_nr_node_ids unstable tree roots. Enabling merge_across_nodes resets this count to 1, even if per-node roots were previously allocated.
So when toggling "merge_across_nodes" 0 -> 1 -> 0, a scan that starts while the setting is 1 and continues after switching back to 0 can encounter stale unstable tree roots, resulting in UAF.
Only root is allowed to change "merge_across_nodes", and the documented sequence is: "merge_across_nodes setting can be changed only when there are no ksm shared pages in the system: set run 2 to unmerge pages first, then to 1 after changing merge_across_nodes, to remerge according to the new setting."
Let's fix it by tracking in ksm_nr_node_ids the actual number of allocated tree roots.
remove_all_stable_nodes() and ksm_check_stable_tree() will now scan more tree roots (skipping the empty ones), which is harmless.
--- mm/ksm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
mm/ksm.c +3 -3
@@ -475,6 +475,7 @@ static void advisor_stop_scan(void)#ifdef CONFIG_NUMA /* Zeroed when merging across nodes is not allowed */ static unsigned int ksm_merge_across_nodes =1 ; +/* Number of allocated stable and unstable tree roots. */ static int ksm_nr_node_ids =1 ;#else #define ksm_merge_across_nodes 1U
@@ -3636,14 +3637,13 @@ static ssize_t merge_across_nodes_store(struct kobject *kobj,else { root_stable_tree = buf; root_unstable_tree = buf + nr_node_ids; + ksm_nr_node_ids = nr_node_ids;/* Stable tree is empty but not the unstable */ root_unstable_tree[0 ] = one_unstable_tree[0 ]; } } -if (!err) { +if (!err) ksm_merge_across_nodes = knob; - ksm_nr_node_ids = knob ?1 : nr_node_ids; - } }mutex_unlock (&ksm_thread_mutex);
Signature · 1 line
2.43.0
Re: [PATCH v3 39/47] mm/ksm: reset the unstable trees of all nodes
Quoted 49 lines
Each full scan resets
ksm_nr_node_idsunstable tree roots. Enablingmerge_across_nodesresets this count to 1, even if per-node roots were previously allocated.So when toggling "
merge_across_nodes" 0 -> 1 -> 0, a scan that starts while the setting is 1 and continues after switching back to 0 can encounter stale unstable tree roots, resulting in UAF.Only root is allowed to change "
merge_across_nodes", and the documented sequence is: "merge_across_nodessetting can be changed only when there are no ksm shared pages in the system: set run 2 to unmerge pages first, then to 1 after changingmerge_across_nodes, to remerge according to the new setting."Let's fix it by tracking in
ksm_nr_node_idsthe actual number of allocated tree roots.
remove_all_stable_nodes()andksm_check_stable_tree()will now scan more tree roots (skipping the empty ones), which is harmless.Fixes ef53d16cded7 ("ksm: allocate roots when needed")Reported-by Josef Bacik <[email protected]>Reviewed-by Pasha Tatashin <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>--- mm/ksm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)mm/ksm.c +2
-0@@ -475,6 +475,7 @@ static void advisor_stop_scan(void)#ifdef CONFIG_NUMA /* Zeroed when merging across nodes is not allowed */ static unsigned int ksm_merge_across_nodes =1 ; +/* Number of allocated stable and unstable tree roots. */ static int ksm_nr_node_ids =1 ;#else #define ksm_merge_across_nodes 1U@@ -3636,14 +3637,13 @@ static ssize_t merge_across_nodes_store(struct kobject *kobj,else { root_stable_tree = buf; root_unstable_tree = buf + nr_node_ids; + ksm_nr_node_ids = nr_node_ids;/* Stable tree is empty but not the unstable */
mm/ksm.c static ssize_t merge_across_nodes_store(struct kobject *kobj, · line 3648
Quoted 10 lines
mm/ksm.c +1
-3root_unstable_tree[0 ] = one_unstable_tree[0 ]; } } -if (!err) { +if (!err) ksm_merge_across_nodes = knob; - ksm_nr_node_ids = knob ?1 : nr_node_ids; - } }mutex_unlock (&ksm_thread_mutex);
David Hildenbrand (Arm)
maintainerPATCH v3 40/47 mm/ksm: fix use of a freed anon_vma at swap-in
In reply to David Hildenbrand (Arm)A folio read from a swap slot previously shared by KSM can remain in the swap cache after the process that first mapped it exits. Its anon_vma can then be freed, so ksm_might_need_to_copy() must not dereference it to compare roots.
Doing so can result in some false-positives, resulting in the reverse mapping missing the new folio mapping, preventing migration and reclaim.
SLAB_TYPESAFE_BY_RCU allows individual anon_vma objects to be reused immediately after freeing. A stale pointer can then refer to another anon_vma whose root matches, even though the target VMA is not linked to that anon_vma.
Move the anonymous rmap compatibility checks into folio_anon_rmap_compatible(). Keep the page-index check and compare the folio's anon_vma pointer against the VMA's own anon_vma and root first.
If neither matches, bite the bullet and walk the VMA's anon_vma_chain. The VMA keeps these anon_vmas alive, and finding a match guarantees that reverse mapping can find the new mapping.
Lorenzo Stoakes says in a private discussion:
"This isn't quite equivalent to the existing check.
Every anon_vma in an anon_vma_chain shares the same root, but if you split a VMA then fork both of them like this:
VMA A
/ \ <split>
VMA B VMA C
\ /
root <- anon_vma 1
----^ ^---- <fork>
/ parent \
anon_vma 2 anon_vma 3
| |
VMA D VMA E
VMA's D and E both get their own new
anon_vma's such that:
VMA D: anon_vma 2 -> anon_vma 1 -> root
VMA E: anon_vma 3 -> anon_vma 1 -> root
And if you found a folio referencing anon_vma 2, the existing check:
anon_vma->root == vma->anon-vma->root
Would be true for VMA's B, C, D & E.
However after your change it'd be true for VMA D only, as it's the only one with anon_vma 2 in its anon_vma_chain.
Whether or not that could actually happen in practice... 🙂 I don't know.
[...] after your change it'd be true for VMA D only, as it's the only one with anon_vma 2 in its anon_vma_chain. [...] I would [...] say that you are making the test stricter but more correct.
"
If this affects performance when KSM was never even involved, we could just track whether KSM ever merged anything and just skip these checks.
There isn't really a lot we can do otherwise: for mapped folios we might be able to use folio_get_anon_vma(), but for unmapped folios there really is no easy way to see if the anon_vma is still valid.
Document the (confusing) lifetime requirements of folio_anon_vma().
--- mm/ksm.c | 36 +++++++++++++++++++++++++++++++----- mm/util.c | 13 +++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-)
mm/ksm.c +31 -5
@@ -3137,11 +3137,40 @@ void __ksm_exit(struct mm_struct *mm)trace_ksm_exit (mm); } +static bool folio_anon_rmap_compatible (const struct folio *folio, +struct vm_area_struct *vma,unsigned long addr) +{ +const struct anon_vma *anon_vma =folio_anon_vma (folio); +struct anon_vma_chain *avc; + +VM_WARN_ON_ONCE_FOLIO (!folio_test_locked (folio), folio); +VM_WARN_ON_ONCE_FOLIO (folio_test_large (folio), folio); +VM_WARN_ON_ONCE_FOLIO (folio_test_ksm (folio), folio); +vma_assert_stabilised (vma); + +if (!anon_vma) +return true; +if (folio->index !=linear_anon_page_index (vma, addr)) +return false; + +/* +* Careful: anon_vma might have been freed, so we cannot safely +* dereference it. Add a fast path before we walk the anon_vma chain. +*/ +if (anon_vma == vma->anon_vma || anon_vma == vma->anon_vma->root) +return true; + +list_for_each_entry (avc, &vma->anon_vma_chain, same_vma) { +if (avc->anon_vma == anon_vma) +return true; + } +return false; +} +struct folio *ksm_might_need_to_copy (struct folio *folio,struct vm_area_struct *vma,unsigned long addr) {struct page *page =folio_page (folio,0 ); -struct anon_vma *anon_vma =folio_anon_vma (folio);struct folio *new_folio;if (folio_test_large (folio))
@@ -3151,10 +3180,7 @@ struct folio *ksm_might_need_to_copy(struct folio *folio,if (folio_stable_node (folio) && !(ksm_run &KSM_RUN_UNMERGE ))return folio;/* no need to copy it */ - }else if (!anon_vma) { -return folio;/* no need to copy it */ - }else if (folio->index ==linear_anon_page_index (vma, addr) && - anon_vma->root == vma->anon_vma->root) { + }else if (folio_anon_rmap_compatible (folio, vma, addr)) {return folio;/* still no need to copy it */ }if (PageHWPoison (page))
mm/util.c +13 -0
@@ -697,6 +697,19 @@ void *vcalloc_noprof(size_t n, size_t size) }EXPORT_SYMBOL (vcalloc_noprof); +/** +* folio_anon_vma - Get a folio's raw anon_vma pointer. +* @folio: The folio. +* +* The returned pointer may be stale for an unmapped folio. The caller +* must ensure the anon_vma stays alive before dereferencing it. A folio +* reference or lock alone is insufficient; use folio_get_anon_vma() when +* the anon_vma's lifetime is not otherwise protected. Note that +* folio_get_anon_vma() is only useful for folios that are still mapped. +* +* Return: The raw anon_vma pointer for an anonymous, non-KSM folio, or +* %NULL otherwise. +*/ struct anon_vma *folio_anon_vma (const struct folio *folio) {unsigned long mapping = (unsigned long )folio->mapping;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 41/47 mm/ksm: check large swap-cache folios before mapping them
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
KSM folios are always small, but their shared swap slots can be read back as part of a large folio in the swap cache. ksm_might_need_to_copy() skips large folios, so another process can map a page with an incompatible anon_vma or page index. Reverse mapping then cannot find the new mapping, preventing migration or reclaim.
Apply the same compatibility checks to large folios as to small ones. Pass the page corresponding to the swap entry and copy only that page when necessary. In do_swap_page()'s copy path, decrement only that slot's swap count, because the source swap-cache folio can now be large.
Fix generated by an LLM and sent by Chris Mason in private.
--- include/linux/ksm.h | 6 +++--- mm/ksm.c | 22 ++++++++++++---------- mm/memory.c | 5 +++-- mm/swapfile.c | 3 ++- 4 files changed, 20 insertions(+), 16 deletions(-)
include/linux/ksm.h +3 -3
@@ -91,8 +91,8 @@ static inline void ksm_exit(struct mm_struct *mm)* We'd like to make this conditional on vma->vm_flags & VM_MERGEABLE, * but what if the vma was unmerged while the page was swapped out? */ -struct folio *ksm_might_need_to_copy (struct folio *folio, -struct vm_area_struct *vma,unsigned long addr); +struct folio *ksm_might_need_to_copy (struct folio *folio,struct page *page, +struct vm_area_struct *vma,unsigned long addr);void rmap_walk_ksm (struct folio *folio,struct rmap_walk_control *rwc);void folio_migrate_ksm (struct folio *newfolio,struct folio *folio);
@@ -145,7 +145,7 @@ static inline int ksm_madvise(struct vm_area_struct *vma, unsigned long start, }static inline struct folio *ksm_might_need_to_copy (struct folio *folio, -struct vm_area_struct *vma,unsigned long addr) +struct page *page,struct vm_area_struct *vma,unsigned long addr) {return folio; }
mm/ksm.c +12 -10
@@ -3138,19 +3138,20 @@ void __ksm_exit(struct mm_struct *mm) }static bool folio_anon_rmap_compatible (const struct folio *folio, -struct vm_area_struct *vma,unsigned long addr) +const struct page *page,struct vm_area_struct *vma, +unsigned long addr) {const struct anon_vma *anon_vma =folio_anon_vma (folio);struct anon_vma_chain *avc;VM_WARN_ON_ONCE_FOLIO (!folio_test_locked (folio), folio); -VM_WARN_ON_ONCE_FOLIO (folio_test_large (folio), folio); +VM_WARN_ON_ONCE_FOLIO (page_folio (page) != folio, folio);VM_WARN_ON_ONCE_FOLIO (folio_test_ksm (folio), folio);vma_assert_stabilised (vma);if (!anon_vma)return true; -if (folio->index !=linear_anon_page_index (vma, addr)) +if (page_pgoff (folio, page) !=linear_anon_page_index (vma, addr))return false;/*
@@ -3167,20 +3168,21 @@ static bool folio_anon_rmap_compatible(const struct folio *folio,return false; } -struct folio *ksm_might_need_to_copy (struct folio *folio, -struct vm_area_struct *vma,unsigned long addr) +struct folio *ksm_might_need_to_copy (struct folio *folio,struct page *page, +struct vm_area_struct *vma,unsigned long addr) { -struct page *page =folio_page (folio,0 );struct folio *new_folio; -if (folio_test_large (folio)) -return folio; - +/* +* A KSM folio is small, but the slot that it left behind can be swapped +* in as a part of a large folio by one of the processes that shared it. +* So test, and copy, the page of the slot. +*/ if (folio_test_ksm (folio)) {if (folio_stable_node (folio) && !(ksm_run &KSM_RUN_UNMERGE ))return folio;/* no need to copy it */ - }else if (folio_anon_rmap_compatible (folio, vma, addr)) { + }else if (folio_anon_rmap_compatible (folio, page, vma, addr)) {return folio;/* still no need to copy it */ }if (PageHWPoison (page))
mm/memory.c +3 -2
@@ -5096,7 +5096,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)* folio->index of non-ksm folios would be nonlinear inside the * anon VMA -- the ksm flag is lost on actual swapout. */ - folio =ksm_might_need_to_copy (folio, vma, vmf->address); + folio =ksm_might_need_to_copy (folio, page, vma, vmf->address);if (unlikely (!folio)) { ret =VM_FAULT_OOM ; folio = swapcache;
@@ -5278,7 +5278,8 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)if (unlikely (folio != swapcache)) {folio_add_new_anon_rmap (folio, vma, address,RMAP_EXCLUSIVE );folio_add_lru_vma (folio, vma); -folio_put_swap (swapcache,NULL ); +folio_put_swap (swapcache, +folio_file_page (swapcache,swp_offset (entry))); }else if (!folio_test_anon (folio)) {/* * We currently only expect !anon folios that are fully
mm/swapfile.c +2 -1
@@ -2514,7 +2514,8 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,return 0 ; swapcache = folio; - folio =ksm_might_need_to_copy (folio, vma, addr); + folio =ksm_might_need_to_copy (folio, +folio_file_page (folio,swp_offset (entry)), vma, addr);if (unlikely (!folio))return -ENOMEM ;else if (unlikely (folio ==ERR_PTR (-EHWPOISON ))) {
Signature · 1 line
2.43.0
Re: [PATCH v3 41/47] mm/ksm: check large swap-cache folios before mapping them
Quoted 15 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
KSM folios are always small, but their shared swap slots can be read back as part of a large folio in the swap cache.
ksm_might_need_to_copy()skips large folios, so another process can map a page with an incompatibleanon_vmaor page index. Reverse mapping then cannot find the new mapping, preventing migration or reclaim.Apply the same compatibility checks to large folios as to small ones. Pass the page corresponding to the swap entry and copy only that page when necessary. In do_swap_page()'s copy path, decrement only that slot's swap count, because the source swap-cache folio can now be large.
Fix generated by an LLM and sent by Chris Mason in private.
Quoted 7 lines
Fixes f1879e8a0c60 ("mm, swap: never bypass the swap cache even for SWP_SYNCHRONOUS_IO")Signed-off-by Chris Mason <[email protected]>Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
LGTM1
David Hildenbrand (Arm)
maintainerPATCH v3 42/47 mm/vma: call ->may_split() before a partial merge
In reply to David Hildenbrand (Arm)From: Chris Mason <[email protected]>
A driver can use the ->may_split() hook to forbid a VMA boundary at certain addresses.
A device-dax device is a character device that gives raw access to a range of memory through mmap(). device-dax refuses any address that is not a multiple of the device's alignment through vm_ops->may_split.
vma_merge_existing_range() can move boundaries, but does not call vm_ops->may_split.
If a range at the edge of a VMA gets the attributes of the adjacent VMA, the function moves only that range over to the neighbour: one VMA shrinks and the other grows. This is a partial merge, and vma_modify() tries it before any split.
Take a device-dax VMA that lies next to another VMA of the same file. The user calls mprotect() on the one page of the first VMA that touches the second VMA, and asks for the protection that the second VMA already has. The result is a partial merge. It puts the boundary inside a 1 GiB mapping, which is a single leaf entry at the PUD level.
As part of the merge, the kernel looks for a huge PMD to split at the new boundary. That code does not expect a PUD leaf:
vma_merge_existing_range()
commit_merge()
vma_adjust_trans_huge()
split_huge_pmd_address()
mm_find_pmd()
treats the PUD leaf as a pointer to a PMD table, so the "table" is the device memory that the leaf maps
__split_huge_pmd()
reads a PMD that the user wrote to the device memory folio_put() on the folio that this PMD names
Bad. While the code must at some point be taught to split PUD entries as well, the issue here is that we move VMA boundaries such that they are incompatible with the driver.
Fix this by calling ->may_split() on the address that a partial merge would move the boundary to, and by abandoning the merge if the driver refuses. vma_modify() then tries the split, which fails with -EINVAL.
Factor out existing split handling into check_split() and reuse it.
Fix generated by an LLM and sent by Chris Mason in private.
--- mm/vma.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)
mm/vma.c +16 -5
@@ -516,6 +516,15 @@ void unmap_region(struct unmap_desc *unmap)tlb_finish_mmu (&tlb); } +static int check_split (struct vm_area_struct *vma,unsigned long addr) +{ +if (!vma->vm_ops) +return 0 ; +if (!vma->vm_ops->may_split) +return 0 ; +return vma->vm_ops->may_split (vma, addr); +} +/* * __split_vma() bypasses sysctl_max_map_count checking. We use this where it * has already been checked or doesn't make sense to fail.
@@ -532,11 +541,9 @@ __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,WARN_ON (vma->vm_start >= addr);WARN_ON (vma->vm_end <= addr); -if (vma->vm_ops && vma->vm_ops->may_split) { - err = vma->vm_ops->may_split (vma, addr); -if (err) -return err; - } + err =check_split (vma, addr); +if (err) +return err; new =vm_area_dup (vma);if (!new)
@@ -948,6 +955,10 @@ static __must_check struct vm_area_struct *vma_merge_existing_range(if (vmg->__remove_middle && !can_merge_remove_vma (middle))return NULL ; +/* If middle isn't removed then it's resized, i.e. splits. */ +if (!vmg->__remove_middle &&check_split (middle, left_side ? end : start)) +return NULL ; +/* * If we merge both VMAs, then next is also deleted. This implies * merge_will_delete_vma also.
Signature · 1 line
2.43.0
Re: [PATCH v3 42/47] mm/vma: call ->may_split() before a partial merge
Quoted 59 lines · David Hildenbrand (Arm)
From: Chris Mason <[email protected]>
A driver can use the ->may_split() hook to forbid a VMA boundary at certain addresses.
A device-dax device is a character device that gives raw access to a range of memory through
mmap(). device-dax refuses any address that is not a multiple of the device's alignment throughvm_ops->may_split.
vma_merge_existing_range()can move boundaries, but does not callvm_ops->may_split.If a range at the edge of a VMA gets the attributes of the adjacent VMA, the function moves only that range over to the neighbour: one VMA shrinks and the other grows. This is a partial merge, and
vma_modify()tries it before any split.Take a device-dax VMA that lies next to another VMA of the same file. The user calls
mprotect()on the one page of the first VMA that touches the second VMA, and asks for the protection that the second VMA already has. The result is a partial merge. It puts the boundary inside a 1 GiB mapping, which is a single leaf entry at the PUD level.As part of the merge, the kernel looks for a huge PMD to split at the new boundary. That code does not expect a PUD leaf:
vma_merge_existing_range()
commit_merge()
vma_adjust_trans_huge()
split_huge_pmd_address()
mm_find_pmd()
treats the PUD leaf as a pointer to a PMD table, so the "table" is the device memory that the leaf maps
__split_huge_pmd()
reads a PMD that the user wrote to the device memoryfolio_put()on the folio that this PMD namesBad. While the code must at some point be taught to split PUD entries as well, the issue here is that we move VMA boundaries such that they are incompatible with the driver.
Fix this by calling ->may_split() on the address that a partial merge would move the boundary to, and by abandoning the merge if the driver refuses.
vma_modify()then tries the split, which fails with-EINVAL.Factor out existing split handling into
check_split()and reuse it.Fix generated by an LLM and sent by Chris Mason in private.
Fixes 38607c62b34b ("fs/dax: properly refcount fs dax pages")Signed-off-by Chris Mason <[email protected]>Reviewed-by Lorenzo Stoakes (ARM) <[email protected]>[ david: improve description; cleanup the code ]Reviewed-by Gregory Price (Meta) <[email protected]>Tested-by Gregory Price (Meta) <[email protected]>Signed-off-by Gregory Price (Meta) <[email protected]>Tested-by Muhammad Usama Anjum <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>
Quoted 1 line
---
David Hildenbrand (Arm)
maintainerPATCH v3 43/47 mm/huge_memory: split huge PUDs when adjusting VMA boundaries
In reply to David Hildenbrand (Arm)PFN mappings such as VFIO PCI BAR mappings can use PUD leaves while allowing VMA splits at page granularity. However, vma_adjust_trans_huge() only handles PMDs.
After faulting a writable PUD-sized VFIO BAR mapping, changing its first page to read-only with mprotect() makes the PMD walk treat the PUD leaf as a PMD table. Applying MADV_DONTFORK to the first PMD-sized chunk leaves the PUD spanning two VMAs: the PMD-aligned boundary skips splitting.
Walk the page tables once at each adjusted boundary and split a huge PUD or PMD leaf as needed. Handle PUD leaves even when the boundary is PMD-aligned.
--- mm/huge_memory.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 60 insertions(+), 10 deletions(-)
mm/huge_memory.c +60 -10
@@ -3504,16 +3504,66 @@ void split_huge_pmd_address(struct vm_area_struct *vma, unsigned long address,__split_huge_pmd (vma, pmd, address, freeze); } -static inline void split_huge_pmd_if_needed (struct vm_area_struct *vma,unsigned long address) +static bool pud_split_required (const struct vm_area_struct *vma, +unsigned long address) { +if (!IS_ENABLED (CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD )) +return false; +if (IS_ALIGNED (address,HPAGE_PUD_SIZE )) +return false;/* -* If the new address isn't hpage aligned and it could previously -* contain an hugepage: check if we need to split an huge pmd. +* Only selected VMAs support mapping PUDs for now. Lift the +* restriction once this changes. */ -if (!IS_ALIGNED (address,HPAGE_PMD_SIZE ) && -range_in_vma (vma,ALIGN_DOWN (address,HPAGE_PMD_SIZE ), -ALIGN (address,HPAGE_PMD_SIZE ))) -split_huge_pmd_address (vma, address, false); +if (!vma_is_dax (vma) && !vma_test_any (vma,VMA_PFNMAP_BIT ,VMA_MIXEDMAP_BIT )) +return false; +return range_in_vma (vma,ALIGN_DOWN (address,HPAGE_PUD_SIZE ), +ALIGN (address,HPAGE_PUD_SIZE )); +} + +static bool pmd_split_required (const struct vm_area_struct *vma, +unsigned long address) +{ +if (IS_ALIGNED (address,HPAGE_PMD_SIZE )) +return false; + +return range_in_vma (vma,ALIGN_DOWN (address,HPAGE_PMD_SIZE ), +ALIGN (address,HPAGE_PMD_SIZE )); +} + +static void split_huge_leaf (struct vm_area_struct *vma, +unsigned long address) +{ +const bool split_pud =pud_split_required (vma, address); +const bool split_pmd =pmd_split_required (vma, address); +pgd_t *pgd; +p4d_t *p4d; +pud_t *pud; +pmd_t *pmd; + +/* +* mmap, VMA and rmap write locks protect the walk; the split helpers +* take the page table locks. +*/ +if (!split_pud && !split_pmd) +return ; + + pgd =pgd_offset (vma->vm_mm, address); +if (!pgd_present (*pgd)) +return ; + + p4d =p4d_offset (pgd, address); +if (!p4d_present (*p4d)) +return ; + + pud =pud_offset (p4d, address); +if (pud_leaf (*pud)) +__split_huge_pud (vma, pud, address); +if (!split_pmd || !pud_present (*pud)) +return ; + + pmd =pmd_offset (pud, address); +__split_huge_pmd (vma, pmd, address, false); } void vma_adjust_trans_huge(struct vm_area_struct *vma,
@@ -3522,14 +3572,14 @@ void vma_adjust_trans_huge(struct vm_area_struct *vma, struct vm_area_struct *next) {/* Check if we need to split start first. */ -split_huge_pmd_if_needed (vma, start); +split_huge_leaf (vma, start);/* Check if we need to split end next. */ -split_huge_pmd_if_needed (vma, end); +split_huge_leaf (vma, end);/* If we're incrementing next->vm_start, we might need to split it. */ if (next) -split_huge_pmd_if_needed (next, end); +split_huge_leaf (next, end); }/*
Signature · 1 line
2.43.0
Re: [PATCH v3 43/47] mm/huge_memory: split huge PUDs when adjusting VMA boundaries
Quoted 13 lines · David Hildenbrand (Arm)
PFN mappings such as VFIO PCI BAR mappings can use PUD leaves while allowing VMA splits at page granularity. However,
vma_adjust_trans_huge()only handles PMDs.After faulting a writable PUD-sized VFIO BAR mapping, changing its first page to read-only with
mprotect()makes the PMD walk treat the PUD leaf as a PMD table. ApplyingMADV_DONTFORKto the first PMD-sized chunk leaves the PUD spanning two VMAs: the PMD-aligned boundary skips splitting.Walk the page tables once at each adjusted boundary and split a huge PUD or PMD leaf as needed. Handle PUD leaves even when the boundary is PMD-aligned.
Quoted 6 lines
Fixes f9e54c3a2f5b7 ("vfio/pci: implement huge_fault support")Tested-by Muhammad Usama Anjum <[email protected]>Reviewed-by Kiryl Shutsemau (Meta) <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
Nice work!
David Hildenbrand (Arm)
maintainerRe: [PATCH v3 43/47] mm/huge_memory: split huge PUDs when adjusting VMA boundaries
Quoted 22 lines · Lance Yang
Quoted 13 lines · David Hildenbrand (Arm)
PFN mappings such as VFIO PCI BAR mappings can use PUD leaves while allowing VMA splits at page granularity. However,
vma_adjust_trans_huge()only handles PMDs.After faulting a writable PUD-sized VFIO BAR mapping, changing its first page to read-only with
mprotect()makes the PMD walk treat the PUD leaf as a PMD table. ApplyingMADV_DONTFORKto the first PMD-sized chunk leaves the PUD spanning two VMAs: the PMD-aligned boundary skips splitting.Walk the page tables once at each adjusted boundary and split a huge PUD or PMD leaf as needed. Handle PUD leaves even when the boundary is PMD-aligned.
Quoted 6 lines
Fixes f9e54c3a2f5b7 ("vfio/pci: implement huge_fault support")Tested-by Muhammad Usama Anjum <[email protected]>Reviewed-by Kiryl Shutsemau (Meta) <[email protected]>Signed-off-by David Hildenbrand (Arm) <[email protected]>---
Quoted 3 lines
Nice work!
Reviewed-by Lance Yang <[email protected]>
Thanks for the review! Please do keep reviewing, but note that more trailers (including this one) will likely not make it upstream as I am about to prepare the PR to get most of these things here upstream.
Signature · 3 lines
Cheers, David
Lance Yang
reviewerRe: [PATCH v3 43/47] mm/huge_memory: split huge PUDs when adjusting VMA boundaries
Quoted 4 lines · David Hildenbrand (Arm)
Thanks for the review! Please do keep reviewing, but note that more trailers (including this one) will likely not make it upstream as I am about to prepare the PR to get most of these things here upstream.
All good, no big deal :D
Cheers, Lance
David Hildenbrand (Arm)
maintainerPATCH v3 44/47 mm/vma: don't leak the charge for stack growth
In reply to David Hildenbrand (Arm)acct_stack_growth() charges stack expansion even without VM_ACCOUNT. This can happen for read-only or MAP_NORESERVE mappings created with MAP_GROWSDOWN. Unmapping these VMAs does not release the charge, leaking Committed_AS.
Only check and charge growth for VM_ACCOUNT VMAs, matching mremap(). Unaccounted growth skips the overcommit check while remaining subject to the other stack-growth limits.
--- mm/vma.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
mm/vma.c +2 -1
@@ -3391,7 +3391,8 @@ static int acct_stack_growth(struct vm_area_struct *vma,* Overcommit.. This must be the final test, as it will * update security statistics. */ -if (security_vm_enough_memory_mm (mm, grow)) +if (vma_test (vma,VMA_ACCOUNT_BIT ) && +security_vm_enough_memory_mm (mm, grow))return -ENOMEM ;return 0 ;
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 45/47 mm/cma: don't release CMA pages still in use
In reply to David Hildenbrand (Arm)From: Rik van Riel <[email protected]>
When a driver calls dma_free_contiguous() before quiescing DMA, the page might still have a reference from the device. put_page_testzero() there returns false, WARN fires, but the code proceeds to free_contig_frozen_range() putting a live page onto buddy and clearing the bitmap. A subsequent allocation can hand the same PFN to a new owner while the original holder still references it.
A concurrent put_page() that drops the last reference between the testzero loop and free_contig_frozen_range() can double-queue the page via page->lru, corrupting buddy lists.
Avoid the corruption by not freeing a CMA region if any of the pages inside are still in use.
Make it explicit in the warning that the driver allowed a leak.
While at it, use page_ref_freeze() to make the semantics clearer and switch to pr_warn_ratelimited().
--- mm/cma.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
mm/cma.c +7 -3
@@ -1020,7 +1020,7 @@ bool cma_release(struct cma *cma, const struct page *pages, unsigned long count) {struct cma_memrange *cmr; -unsigned long ret =0 ; +unsigned long leaked =0 ;unsigned long i, pfn; cmr =find_cma_memrange (cma, pages, count);
@@ -1029,9 +1029,13 @@ bool cma_release(struct cma *cma, const struct page *pages, pfn =page_to_pfn (pages);for (i =0 ; i < count; i++, pfn++) - ret += !put_page_testzero (pfn_to_page (pfn)); + leaked += !page_ref_freeze (pfn_to_page (pfn),1 ); -WARN (ret,"%lu pages are still in use!\n" , ret); +if (leaked) { +pr_warn_ratelimited ("%lu pages are still in use, not freeing CMA region!\n" , + leaked); +return true; + }__cma_release_frozen (cma, cmr, pages, count);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 46/47 mm/memory-tiers: unregister empty tiers outside memory_tier_lock
In reply to David Hildenbrand (Arm)When offlining a node's last memory empties its tier, clear_node_memory_tier() calls device_unregister() while holding memory_tier_lock. A concurrent sysfs read can hold a kernfs active reference and block in nodelist_show() on that mutex. device_unregister() then waits in kernfs_drain() for the read to finish and release its active reference, while the reader waits for the offlining task to release the mutex.
Return the node's former tier from clear_node_memory_tier(), making the helper more similar to set_node_memory_tier(), which also returns a memory_tier pointer. Let the caller unlink an empty tier under memory_tier_lock before rebuilding demotion targets, then unregister its device after dropping the lock.
--- mm/memory-tiers.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-)
mm/memory-tiers.c +13 -16
@@ -572,21 +572,14 @@ static struct memory_tier *set_node_memory_tier(int node)return memtier; } -static void destroy_memory_tier (struct memory_tier *memtier) +static struct memory_tier *clear_node_memory_tier (int node) { -list_del (&memtier->list); -device_unregister (&memtier->dev); -} - -static bool clear_node_memory_tier (int node) -{ -bool cleared = false;pg_data_t *pgdat;struct memory_tier *memtier; pgdat =NODE_DATA (node);if (!pgdat) -return false; +return NULL ;/* * Make sure that anybody looking at NODE_DATA who finds
@@ -604,14 +597,10 @@ static bool clear_node_memory_tier(int node)synchronize_rcu (); memtype = node_memory_types[node].memtype;node_clear (node, memtype->nodes); -if (nodes_empty (memtype->nodes)) { +if (nodes_empty (memtype->nodes))list_del_init (&memtype->tier_sibling); -if (list_empty (&memtier->memory_types)) -destroy_memory_tier (memtier); - } - cleared = true; } -return cleared; +return memtier; }static void release_memtype (struct kref *kref)
@@ -884,13 +873,21 @@ static int __meminit memtier_hotplug_callback(struct notifier_block *self, {struct memory_tier *memtier;struct node_notify *nn = _arg; +bool empty = false;switch (action) {case NODE_REMOVED_LAST_MEMORY :mutex_lock (&memory_tier_lock); -if (clear_node_memory_tier (nn->nid)) + memtier =clear_node_memory_tier (nn->nid); +if (memtier) { + empty =list_empty (&memtier->memory_types); +if (empty) +list_del (&memtier->list);establish_demotion_targets (); + }mutex_unlock (&memory_tier_lock); +if (empty) +device_unregister (&memtier->dev);break ;case NODE_ADDED_FIRST_MEMORY :mutex_lock (&memory_tier_lock);
Signature · 1 line
2.43.0
David Hildenbrand (Arm)
maintainerPATCH v3 47/47 fs/proc/task_mmu: refuse addresses beyond 32 bits on 32bit
In reply to David Hildenbrand (Arm)From: Josef Bacik <[email protected]>
The addresses in struct pm_scan_arg are 64 bits wide. The first thing pagemap_scan_get_args() does with them is to cast them to unsigned long. On a 32-bit kernel the upper half is dropped without a word:PAGEMAP_SCAN of 0x100001000-0x100002000 scans, and with PM_SCAN_WP_MATCHING write-protects, 0x1000-0x2000, stores the result at the lower half of vec, and reports success.
Return -EFAULT, as for other addresses outside of the address space.
This is probably not worth CCing stable.
Fix generated by an LLM and sent by Josef Bacik in private.
--- fs/proc/task_mmu.c | 6 ++++++ 1 file changed, 6 insertions(+)
fs/proc/task_mmu.c +6 -0
@@ -3032,6 +3032,12 @@ static int pagemap_scan_get_args(struct pm_scan_arg *arg, arg->category_anyof_mask | arg->return_mask) & ~PM_SCAN_CATEGORIES)return -EINVAL ; +/* Don't cut 64-bit numbers down to addresses that happen to work. */ +if (arg->start != (unsigned long )arg->start || + arg->end != (unsigned long )arg->end || + arg->vec != (unsigned long )arg->vec) +return -EFAULT ; + arg->start =untagged_addr ((unsigned long )arg->start); arg->end =untagged_addr ((unsigned long )arg->end); arg->vec =untagged_addr ((unsigned long )arg->vec);
Signature · 1 line
2.43.0
Quoted 4 lines · David Hildenbrand (Arm)
Thanks to everyone for the review! Hoping I can get some more? :)
A quick resend, I'll queue these immediately after sending to get them into -next asap. Don't let that discourage you from reviewing ;)
Applied, thanks!
tree: mm/core.git
branch: for-next-fixes
Applied patches will appear soon in mm-next, followed by linux-next.
Please report any outstanding bugs that were missed during review, so we can either drop the patch series or decide how to fix it up.
It is encouraged to provide Acked-bys, Reviewed-bys, and Tested-bys even though the patches have already been applied. If possible, patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. Further, this is not a guarantee that the patches will go upstream. If in doubt, please check the listed branch.
[01/47] mm/mglru: don't use stack pointers when walking PUD and PMD tables
https://git.kernel.org/mm/core/c/3922fe76 [02/47] fork: take mmap_lock before copying mm_struct https://git.kernel.org/mm/core/c/6cf33ce8 [03/47] mm/shmem: make shmem_file() check the file's inode https://git.kernel.org/mm/core/c/d6003145 [04/47] mm/memfd: make f_pos accesses atomic https://git.kernel.org/mm/core/c/b6638250 [05/47] mm/truncate: look again if a folio left the mapping https://git.kernel.org/mm/core/c/e7086d79 [06/47] mm/filemap: read mapping->a_ops->free_folio under i_lock https://git.kernel.org/mm/core/c/c9073948 [07/47] mm/filemap: don't lose a folio lock wakeup to a signal https://git.kernel.org/mm/core/c/c39c22f2 [08/47] mm/filemap: finish fault cleanup before dropping the fault lock https://git.kernel.org/mm/core/c/335429f6 [09/47] mm/rmap: handle device-exclusive entries in folio_referenced_one() https://git.kernel.org/mm/core/c/6324d435 [10/47] mm/huge_memory: fix list corruption when deferred_split_scan() races with deferred_split_folio() https://git.kernel.org/mm/core/c/be9ec969 [11/47] mm/huge_memory: properly handle refcounted PUD mappings on fork https://git.kernel.org/mm/core/c/1ef69272 [12/47] mm/huge_memory: fix livelock on device private folio https://git.kernel.org/mm/core/c/88ee4890 [13/47] mm/huge_memory: fix truncate racing with PMD zapping https://git.kernel.org/mm/core/c/a63ff7ba [14/47] mm/huge_memory: fix zapping migration entries for device-private THPs https://git.kernel.org/mm/core/c/2814cce8 [15/47] mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/17382ca5 [16/47] mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/bf852461 [17/47] mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs https://git.kernel.org/mm/core/c/299b8524 [18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/22b08cd1 [19/47] mm/gup: recheck the PUD under PTL in follow_pud_mask() https://git.kernel.org/mm/core/c/e04a9b5e [20/47] mm/gup: fix clearing folios in memfd_pin_folios() https://git.kernel.org/mm/core/c/c6791388 [21/47] mm/gup: fix out-of-bounds write for nr_pages == 0 in GUP-fast https://git.kernel.org/mm/core/c/5b1b815c [22/47] mm/gup: check for overflow when converting nr_pages to bytes in GUP-fast https://git.kernel.org/mm/core/c/8791ac0a [23/47] mm/userfaultfd: write-lock the VMA before waking waiters when unregistering https://git.kernel.org/mm/core/c/35f48d65 [24/47] mm/userfaultfd: wake all waiters on unregister https://git.kernel.org/mm/core/c/9e4d60fe [25/47] mm/userfaultfd: wake waiters when registration modes are removed https://git.kernel.org/mm/core/c/a9cba229 [26/47] mm/userfaultfd: compare exact registration modes before skipping https://git.kernel.org/mm/core/c/86de3945 [27/47] mm/mmu_gather: fix flushing ranges with 4 GiB or larger https://git.kernel.org/mm/core/c/61437375 [28/47] mm/mmu_notifier: finish non-blocking interval invalidation on error https://git.kernel.org/mm/core/c/ec3feff7 [29/47] mm/mmu_notifier: set ->mm only after mmgrab() https://git.kernel.org/mm/core/c/10ad9b99 [30/47] mm/mremap: use pudp_huge_get_and_clear() when moving huge PUDs https://git.kernel.org/mm/core/c/f16f7e7c [31/47] mm/mremap: recheck PUDs under the page table lock https://git.kernel.org/mm/core/c/ae414aed [32/47] mm/mremap: fix moving PUD leaves https://git.kernel.org/mm/core/c/262a6a25 [33/47] mm/memory_hotplug: avoid races in pfn_to_online_page() with dying pgmap https://git.kernel.org/mm/core/c/67782b68 [34/47] mm/madvise: fix data loss from MADV_FREE on a KSM page https://git.kernel.org/mm/core/c/4fc80184 [35/47] mm/memory: reject remote COW faults on PFN mappings https://git.kernel.org/mm/core/c/99e96e4d [36/47] mm: fix unmap_mapping_range() beyond 4 GiB on 32-bit https://git.kernel.org/mm/core/c/1ca84b58 [37/47] mm: fix unkillable fault loop on MAP_DROPPABLE memory https://git.kernel.org/mm/core/c/a4c8fb4f [38/47] mm: fix deadlock of mremap() against UFFDIO_MOVE https://git.kernel.org/mm/core/c/55f62b5f [39/47] mm/ksm: reset the unstable trees of all nodes https://git.kernel.org/mm/core/c/9c1d2a06 [40/47] mm/ksm: fix use of a freed anon_vma at swap-in https://git.kernel.org/mm/core/c/9c20c73c [41/47] mm/ksm: check large swap-cache folios before mapping them https://git.kernel.org/mm/core/c/7fa9d8cb [42/47] mm/vma: call ->may_split() before a partial merge https://git.kernel.org/mm/core/c/e0b967df [43/47] mm/huge_memory: split huge PUDs when adjusting VMA boundaries https://git.kernel.org/mm/core/c/0653a27f [44/47] mm/vma: don't leak the charge for stack growth https://git.kernel.org/mm/core/c/a8ebd75d [45/47] mm/cma: don't release CMA pages still in use https://git.kernel.org/mm/core/c/d7950ad4 [46/47] mm/memory-tiers: unregister empty tiers outside memory_tier_lock https://git.kernel.org/mm/core/c/b6d7ad49 [47/47] fs/proc/task_mmu: refuse addresses beyond 32 bits on 32bit https://git.kernel.org/mm/core/c/021ea6d2
Signature · 3 lines
Cheers, David
David Hildenbrand (Arm)
maintainerQuoted 16 lines · David Hildenbrand
Quoted 4 lines · David Hildenbrand (Arm)
Thanks to everyone for the review! Hoping I can get some more? :)
A quick resend, I'll queue these immediately after sending to get them into -next asap. Don't let that discourage you from reviewing ;)
Applied, thanks!
tree:
mm/core.git
branch: for-next-fixesApplied patches will appear soon in mm-next, followed by linux-next.
Please report any outstanding bugs that were missed during review, so we can either drop the patch series or decide how to fix it up.
Quoted 8 lines
It is encouraged to provide Acked-bys, Reviewed-bys, and Tested-bys even though the patches have already been applied. If possible, patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. Further, this is not a guarantee that the patches will go upstream. If in doubt, please check the listed branch.
Small correction, I'll drop all that don't have CC:stable from the for-next-fixes branch and queue them ordinarily for the next merge window. Which is relevant for ...
Quoted 16 lines
[01/47] mm/mglru: don't use stack pointers when walking PUD and PMD tables
https://git.kernel.org/mm/core/c/3922fe76 [02/47] fork: take mmap_lock before copying mm_struct https://git.kernel.org/mm/core/c/6cf33ce8 [03/47] mm/shmem: make shmem_file() check the file's inode https://git.kernel.org/mm/core/c/d6003145 [04/47] mm/memfd: make f_pos accesses atomic https://git.kernel.org/mm/core/c/b6638250 [05/47] mm/truncate: look again if a folio left the mapping https://git.kernel.org/mm/core/c/e7086d79 [06/47] mm/filemap: read mapping->a_ops->free_folio under i_lock https://git.kernel.org/mm/core/c/c9073948 [07/47] mm/filemap: don't lose a folio lock wakeup to a signal https://git.kernel.org/mm/core/c/c39c22f2 [08/47] mm/filemap: finish fault cleanup before dropping the fault lock https://git.kernel.org/mm/core/c/335429f6
Quoted 8 lines
[09/47] mm/rmap: handle device-exclusive entries in
folio_referenced_one()https://git.kernel.org/mm/core/c/6324d435 [10/47] mm/huge_memory: fix list corruption when deferred_split_scan() races with deferred_split_folio() https://git.kernel.org/mm/core/c/be9ec969 [11/47] mm/huge_memory: properly handle refcounted PUD mappings on fork https://git.kernel.org/mm/core/c/1ef69272 [12/47] mm/huge_memory: fix livelock on device private folio https://git.kernel.org/mm/core/c/88ee4890
This one
Quoted 12 lines
[13/47]
mm/huge_memory: fix truncate racing with PMD zappinghttps://git.kernel.org/mm/core/c/a63ff7ba [14/47] mm/huge_memory: fix zapping migration entries for device-private THPs https://git.kernel.org/mm/core/c/2814cce8 [15/47] mm/khugepaged: always recheck the pmd in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/17382ca5 [16/47] mm/khugepaged: fix PMD table lock leak in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/bf852461 [17/47] mm/khugepaged: preserve dirty bits when collapsing PTE-mapped THPs https://git.kernel.org/mm/core/c/299b8524 [18/47] mm/khugepaged: check if PTE table is empty in try_collapse_pte_mapped_thp() https://git.kernel.org/mm/core/c/22b08cd1
This one
Quoted 22 lines
[19/47] mm/gup: recheck the PUD under PTL in
follow_pud_mask()https://git.kernel.org/mm/core/c/e04a9b5e [20/47] mm/gup: fix clearing folios in memfd_pin_folios() https://git.kernel.org/mm/core/c/c6791388 [21/47] mm/gup: fix out-of-bounds write for nr_pages == 0 in GUP-fast https://git.kernel.org/mm/core/c/5b1b815c [22/47] mm/gup: check for overflow when converting nr_pages to bytes in GUP-fast https://git.kernel.org/mm/core/c/8791ac0a [23/47] mm/userfaultfd: write-lock the VMA before waking waiters when unregistering https://git.kernel.org/mm/core/c/35f48d65 [24/47] mm/userfaultfd: wake all waiters on unregister https://git.kernel.org/mm/core/c/9e4d60fe [25/47] mm/userfaultfd: wake waiters when registration modes are removed https://git.kernel.org/mm/core/c/a9cba229 [26/47] mm/userfaultfd: compare exact registration modes before skipping https://git.kernel.org/mm/core/c/86de3945 [27/47] mm/mmu_gather: fix flushing ranges with 4 GiB or larger https://git.kernel.org/mm/core/c/61437375 [28/47] mm/mmu_notifier: finish non-blocking interval invalidation on error https://git.kernel.org/mm/core/c/ec3feff7 [29/47] mm/mmu_notifier: set ->mm only after mmgrab() https://git.kernel.org/mm/core/c/10ad9b99
Quoted 8 lines
[30/47] mm/mremap: use
pudp_huge_get_and_clear()when moving huge PUDshttps://git.kernel.org/mm/core/c/f16f7e7c [31/47] mm/mremap: recheck PUDs under the page table lock https://git.kernel.org/mm/core/c/ae414aed [32/47] mm/mremap: fix moving PUD leaves https://git.kernel.org/mm/core/c/262a6a25 [33/47] mm/memory_hotplug: avoid races in pfn_to_online_page() with dying pgmap https://git.kernel.org/mm/core/c/67782b68
This one
Quoted 20 lines
[34/47] mm/madvise: fix data loss from
MADV_FREEon a KSM pagehttps://git.kernel.org/mm/core/c/4fc80184 [35/47] mm/memory: reject remote COW faults on PFN mappings https://git.kernel.org/mm/core/c/99e96e4d [36/47] mm: fix unmap_mapping_range() beyond 4 GiB on 32-bit https://git.kernel.org/mm/core/c/1ca84b58 [37/47] mm: fix unkillable fault loop on MAP_DROPPABLE memory https://git.kernel.org/mm/core/c/a4c8fb4f [38/47] mm: fix deadlock of mremap() against UFFDIO_MOVE https://git.kernel.org/mm/core/c/55f62b5f [39/47] mm/ksm: reset the unstable trees of all nodes https://git.kernel.org/mm/core/c/9c1d2a06 [40/47] mm/ksm: fix use of a freed anon_vma at swap-in https://git.kernel.org/mm/core/c/9c20c73c [41/47] mm/ksm: check large swap-cache folios before mapping them https://git.kernel.org/mm/core/c/7fa9d8cb [42/47] mm/vma: call ->may_split() before a partial merge https://git.kernel.org/mm/core/c/e0b967df [43/47] mm/huge_memory: split huge PUDs when adjusting VMA boundaries https://git.kernel.org/mm/core/c/0653a27f
Quoted 8 lines
[44/47] mm/vma: don't leak the charge for stack growth
https://git.kernel.org/mm/core/c/a8ebd75d [45/47] mm/cma: don't release CMA pages still in use https://git.kernel.org/mm/core/c/d7950ad4 [46/47] mm/memory-tiers: unregister empty tiers outside memory_tier_lock https://git.kernel.org/mm/core/c/b6d7ad49 [47/47] fs/proc/task_mmu: refuse addresses beyond 32 bits on 32bit https://git.kernel.org/mm/core/c/021ea6d2
This one
Signature · 3 lines
Cheers, David