From 5f80d03c8ccefc18b587166cd95291621f5acfab Mon Sep 17 00:00:00 2001 From: Alexander Winkowski Date: Fri, 12 Jan 2024 05:02:10 +0000 Subject: [PATCH] Revert "ANDROID: mm: use raw seqcount variants in vm_write_*" This reverts commit f2c1ef71ed4dcdf40fa3220837634d753dcc5fb8. Change-Id: I5dc242f3d5aa05eb4630426d7444c36412ed6191 Signed-off-by: Alexander Winkowski --- include/linux/mm.h | 29 ++++++++++++++++++++++++----- mm/mmap.c | 38 +++++++++++++++++++++++++++++--------- mm/mremap.c | 8 ++++---- 3 files changed, 57 insertions(+), 18 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 0304a5241875..72d43be61a97 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -1613,13 +1613,22 @@ int generic_access_phys(struct vm_area_struct *vma, unsigned long addr, #ifdef CONFIG_SPECULATIVE_PAGE_FAULT static inline void vm_write_begin(struct vm_area_struct *vma) { - /* - * The reads never spins and preemption - * disablement is not required. - */ - raw_write_seqcount_begin(&vma->vm_sequence); + write_seqcount_begin(&vma->vm_sequence); +} +static inline void vm_write_begin_nested(struct vm_area_struct *vma, + int subclass) +{ + write_seqcount_begin_nested(&vma->vm_sequence, subclass); } static inline void vm_write_end(struct vm_area_struct *vma) +{ + write_seqcount_end(&vma->vm_sequence); +} +static inline void vm_raw_write_begin(struct vm_area_struct *vma) +{ + raw_write_seqcount_begin(&vma->vm_sequence); +} +static inline void vm_raw_write_end(struct vm_area_struct *vma) { raw_write_seqcount_end(&vma->vm_sequence); } @@ -1627,9 +1636,19 @@ static inline void vm_write_end(struct vm_area_struct *vma) static inline void vm_write_begin(struct vm_area_struct *vma) { } +static inline void vm_write_begin_nested(struct vm_area_struct *vma, + int subclass) +{ +} static inline void vm_write_end(struct vm_area_struct *vma) { } +static inline void vm_raw_write_begin(struct vm_area_struct *vma) +{ +} +static inline void vm_raw_write_end(struct vm_area_struct *vma) +{ +} #endif /* CONFIG_SPECULATIVE_PAGE_FAULT */ extern void truncate_pagecache(struct inode *inode, loff_t new); diff --git a/mm/mmap.c b/mm/mmap.c index eedd39382561..bc234247a09f 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -767,9 +767,29 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start, long adjust_next = 0; int remove_next = 0; - vm_write_begin(vma); + /* + * Why using vm_raw_write*() functions here to avoid lockdep's warning ? + * + * Locked is complaining about a theoretical lock dependency, involving + * 3 locks: + * mapping->i_mmap_rwsem --> vma->vm_sequence --> fs_reclaim + * + * Here are the major path leading to this dependency : + * 1. __vma_adjust() mmap_sem -> vm_sequence -> i_mmap_rwsem + * 2. move_vmap() mmap_sem -> vm_sequence -> fs_reclaim + * 3. __alloc_pages_nodemask() fs_reclaim -> i_mmap_rwsem + * 4. unmap_mapping_range() i_mmap_rwsem -> vm_sequence + * + * So there is no way to solve this easily, especially because in + * unmap_mapping_range() the i_mmap_rwsem is grab while the impacted + * VMAs are not yet known. + * However, the way the vm_seq is used is guarantying that we will + * never block on it since we just check for its value and never wait + * for it to move, see vma_has_changed() and handle_speculative_fault(). + */ + vm_raw_write_begin(vma); if (next) - vm_write_begin(next); + vm_raw_write_begin(next); if (next && !insert) { struct vm_area_struct *exporter = NULL, *importer = NULL; @@ -853,8 +873,8 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start, error = anon_vma_clone(importer, exporter); if (error) { if (next && next != vma) - vm_write_end(next); - vm_write_end(vma); + vm_raw_write_end(next); + vm_raw_write_end(vma); return error; } } @@ -983,7 +1003,7 @@ again: if (next->anon_vma) anon_vma_merge(vma, next); mm->map_count--; - vm_write_end(next); + vm_raw_write_end(next); put_vma(next); /* * In mprotect's case 6 (see comments on vma_merge), @@ -999,7 +1019,7 @@ again: */ next = vma->vm_next; if (next) - vm_write_begin(next); + vm_raw_write_begin(next); } else { /* * For the scope of the comment "next" and @@ -1047,9 +1067,9 @@ again: uprobe_mmap(insert); if (next && next != vma) - vm_write_end(next); + vm_raw_write_end(next); if (!keep_locked) - vm_write_end(vma); + vm_raw_write_end(vma); validate_mm(mm); @@ -3409,7 +3429,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, * that we protect it right now, and let the caller unprotect * it once the move is done. */ - vm_write_begin(new_vma); + vm_raw_write_begin(new_vma); vma_link(mm, new_vma, prev, rb_link, rb_parent); *need_rmap_locks = false; } diff --git a/mm/mremap.c b/mm/mremap.c index 56b2e64cd6e8..0485add3399f 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -533,7 +533,7 @@ static unsigned long move_vma(struct vm_area_struct *vma, * to be mapped in our back while we are copying the PTEs. */ if (vma != new_vma) - vm_write_begin(vma); + vm_raw_write_begin(vma); moved_len = move_page_tables(vma, old_addr, new_vma, new_addr, old_len, need_rmap_locks); @@ -552,7 +552,7 @@ static unsigned long move_vma(struct vm_area_struct *vma, move_page_tables(new_vma, new_addr, vma, old_addr, moved_len, true); if (vma != new_vma) - vm_write_end(vma); + vm_raw_write_end(vma); vma = new_vma; old_len = new_len; old_addr = new_addr; @@ -562,9 +562,9 @@ static unsigned long move_vma(struct vm_area_struct *vma, arch_remap(mm, old_addr, old_addr + old_len, new_addr, new_addr + new_len); if (vma != new_vma) - vm_write_end(vma); + vm_raw_write_end(vma); } - vm_write_end(new_vma); + vm_raw_write_end(new_vma); /* Conceal VM_ACCOUNT so old reservation is not undone */ if (vm_flags & VM_ACCOUNT && !(flags & MREMAP_DONTUNMAP)) {