Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Liam Howlett <liam.howlett@oracle.com>
To:     "maple-tree@lists.infradead.org" <maple-tree@lists.infradead.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>
CC:     Song Liu <songliubraving@fb.com>,
        Davidlohr Bueso <dave@stgolabs.net>,
        "Paul E . McKenney" <paulmck@kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Laurent Dufour <ldufour@linux.ibm.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Rik van Riel <riel@surriel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Michel Lespinasse <walken.cr@gmail.com>,
        Liam Howlett <liam.howlett@oracle.com>
Subject: [PATCH 94/94] mm: Move mas locking outside of munmap() path.
Thread-Topic: [PATCH 94/94] mm: Move mas locking outside of munmap() path.
Thread-Index: AQHXPERDRfg+Yjl5x0SvTuH2bZDirQ==
Date:   Wed, 28 Apr 2021 15:36:32 +0000
Message-ID: <20210428153542.2814175-95-Liam.Howlett@Oracle.com>
References: <20210428153542.2814175-1-Liam.Howlett@Oracle.com>
In-Reply-To: <20210428153542.2814175-1-Liam.Howlett@Oracle.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 83abecd5-c75e-4d01-dc38-08d90a5b9b84
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Apr 2021 15:36:32.6966
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: wC3UKuMYCVZpf5UFecvYd9QaJT+rv/Klwg9yvSOe7L4zDFdRwAD2lpQlK1Is7yxu4jkPxxgeFht7PbxSMqnHGA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4548
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9968 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 spamscore=0 adultscore=0
 suspectscore=0 mlxscore=0 mlxlogscore=999 malwarescore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104060000
 definitions=main-2104280101
X-Proofpoint-ORIG-GUID: QFOQEaci5sAFYsrYku4YPS8ZIO-xikNr
X-Proofpoint-GUID: QFOQEaci5sAFYsrYku4YPS8ZIO-xikNr
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Now that there is a split variant that allows splitting to use a maple stat=
e,
move the locks to a more logical position.

Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
---
 mm/internal.h |  4 ---
 mm/mmap.c     | 81 +++++++++++++++++++++++++++++++++------------------
 mm/nommu.c    |  4 +++
 3 files changed, 56 insertions(+), 33 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 0fb161ee7f73..68888d4d9cb3 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -367,9 +367,7 @@ static inline int vma_mas_store(struct vm_area_struct *=
vma, struct ma_state *mas
=20
 	mas->index =3D vma->vm_start;
 	mas->last =3D vma->vm_end - 1;
-	mas_lock(mas);
 	ret =3D mas_store_gfp(mas, vma, GFP_KERNEL);
-	mas_unlock(mas);
 	return ret;
 }
=20
@@ -388,9 +386,7 @@ static inline int vma_mas_remove(struct vm_area_struct =
*vma, struct ma_state *ma
=20
 	mas->index =3D vma->vm_start;
 	mas->last =3D vma->vm_end - 1;
-	mas_lock(mas);
 	ret =3D mas_store_gfp(mas, NULL, GFP_KERNEL);
-	mas_unlock(mas);
 	return ret;
 }
=20
diff --git a/mm/mmap.c b/mm/mmap.c
index 5335bd72bda3..a0a4d1c4ca15 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -239,6 +239,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 		goto success;
 	}
=20
+	mas_lock(&mas);
 	mas_set(&mas, newbrk);
 	brkvma =3D mas_walk(&mas);
 	if (brkvma) { // munmap necessary, there is something at newbrk.
@@ -289,19 +290,21 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 		goto out;
=20
 	mm->brk =3D brk;
-
 success:
 	populate =3D newbrk > oldbrk && (mm->def_flags & VM_LOCKED) !=3D 0;
 	if (downgraded)
 		mmap_read_unlock(mm);
-	else
+	else {
+		mas_unlock(&mas);
 		mmap_write_unlock(mm);
+	}
 	userfaultfd_unmap_complete(mm, &uf);
 	if (populate)
 		mm_populate_vma(brkvma, oldbrk, newbrk);
 	return brk;
=20
 out:
+	mas_unlock(&mas);
 	mmap_write_unlock(mm);
 	return origbrk;
 }
@@ -501,7 +504,9 @@ static void vma_link(struct mm_struct *mm, struct vm_ar=
ea_struct *vma)
 {
 	MA_STATE(mas, &mm->mm_mt, vma->vm_start, vma->vm_end - 1);
=20
+	mas_lock(&mas);
 	vma_mas_link(mm, vma, &mas);
+	mas_unlock(&mas);
 }
=20
 /*
@@ -2442,8 +2447,6 @@ static inline void detach_range(struct mm_struct *mm,=
 struct ma_state *mas,
 	do {
 		count++;
 		*vma =3D mas_prev(mas, start);
-		BUG_ON((*vma)->vm_start < start);
-		BUG_ON((*vma)->vm_end > end + 1);
 		vma_mas_store(*vma, dst);
 		if ((*vma)->vm_flags & VM_LOCKED) {
 			mm->locked_vm -=3D vma_pages(*vma);
@@ -2548,14 +2551,12 @@ static int do_mas_align_munmap(struct ma_state *mas=
, struct vm_area_struct *vma,
 	}
=20
 	/* Point of no return */
-	mas_lock(mas);
 	if (next)
 		max =3D next->vm_start;
=20
 	mtree_init(&mt_detach, MAPLE_ALLOC_RANGE);
 	dst.tree =3D &mt_detach;
 	detach_range(mm, mas, &dst, &vma);
-	mas_unlock(mas);
=20
 	/*
 	 * Do not downgrade mmap_lock if we are next to VM_GROWSDOWN or
@@ -2567,8 +2568,10 @@ static int do_mas_align_munmap(struct ma_state *mas,=
 struct vm_area_struct *vma,
 			downgrade =3D false;
 		else if (prev && (prev->vm_flags & VM_GROWSUP))
 			downgrade =3D false;
-		else
+		else {
+			mas_unlock(mas);
 			mmap_write_downgrade(mm);
+		}
 	}
=20
 	/* Unmap the region */
@@ -2634,7 +2637,9 @@ int do_munmap(struct mm_struct *mm, unsigned long sta=
rt, size_t len,
 	int ret;
 	MA_STATE(mas, &mm->mm_mt, start, start);
=20
+	mas_lock(&mas);
 	ret =3D do_mas_munmap(&mas, mm, start, len, uf, false);
+	mas_unlock(&mas);
 	return ret;
 }
=20
@@ -2651,11 +2656,12 @@ unsigned long mmap_region(struct file *file, unsign=
ed long addr,
 	unsigned long merge_start =3D addr, merge_end =3D end;
 	unsigned long max =3D USER_PGTABLES_CEILING;
 	pgoff_t vm_pgoff;
-	int error;
+	int error =3D ENOMEM;
 	struct ma_state ma_prev, tmp;
 	MA_STATE(mas, &mm->mm_mt, addr, end - 1);
=20
=20
+	mas_lock(&mas);
 	/* Check against address space limit. */
 	if (!may_expand_vm(mm, vm_flags, len >> PAGE_SHIFT)) {
 		unsigned long nr_pages;
@@ -2668,23 +2674,21 @@ unsigned long mmap_region(struct file *file, unsign=
ed long addr,
=20
 		if (!may_expand_vm(mm, vm_flags,
 					(len >> PAGE_SHIFT) - nr_pages))
-			return -ENOMEM;
+			goto no_mem;
 	}
=20
 	validate_mm(mm);
 	/* Unmap any existing mapping in the area */
-	if (do_mas_munmap(&mas, mm, addr, len, uf, false)) {
-		return -ENOMEM;
-	}
+	if (do_mas_munmap(&mas, mm, addr, len, uf, false))
+		goto no_mem;
=20
 	/*
 	 * Private writable mapping: check memory availability
 	 */
 	if (accountable_mapping(file, vm_flags)) {
 		charged =3D len >> PAGE_SHIFT;
-		if (security_vm_enough_memory_mm(mm, charged)) {
-			return -ENOMEM;
-		}
+		if (security_vm_enough_memory_mm(mm, charged))
+			goto no_mem;
 		vm_flags |=3D VM_ACCOUNT;
 	}
=20
@@ -2735,10 +2739,8 @@ unsigned long mmap_region(struct file *file, unsigne=
d long addr,
 	 * not unmapped, but the maps are removed from the list.
 	 */
 	vma =3D vm_area_alloc(mm);
-	if (!vma) {
-		error =3D -ENOMEM;
+	if (!vma)
 		goto unacct_error;
-	}
=20
 	vma->vm_start =3D addr;
 	vma->vm_end =3D end;
@@ -2863,6 +2865,7 @@ unsigned long mmap_region(struct file *file, unsigned=
 long addr,
 	vma->vm_flags |=3D VM_SOFTDIRTY;
 	vma_set_page_prot(vma);
 	validate_mm(mm);
+	mas_unlock(&mas);
 	return addr;
=20
 unmap_and_free_vma:
@@ -2883,6 +2886,8 @@ unsigned long mmap_region(struct file *file, unsigned=
 long addr,
 unacct_error:
 	if (charged)
 		vm_unacct_memory(charged);
+no_mem:
+	mas_unlock(&mas);
 	return error;
 }
=20
@@ -2895,6 +2900,7 @@ static int __vm_munmap(unsigned long start, size_t le=
n, bool downgrade)
=20
 	if (mmap_write_lock_killable(mm))
 		return -EINTR;
+	mas_lock(&mas);
 	ret =3D do_mas_munmap(&mas, mm, start, len, &uf, downgrade);
 	/*
 	 * Returning 1 indicates mmap_lock is downgraded.
@@ -2904,8 +2910,10 @@ static int __vm_munmap(unsigned long start, size_t l=
en, bool downgrade)
 	if (ret =3D=3D 1) {
 		mmap_read_unlock(mm);
 		ret =3D 0;
-	} else
+	} else {
+		mas_unlock(&mas);
 		mmap_write_unlock(mm);
+	}
=20
 	userfaultfd_unmap_complete(mm, &uf);
 	return ret;
@@ -2957,6 +2965,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, star=
t, unsigned long, size,
 	if (mmap_write_lock_killable(mm))
 		return -EINTR;
=20
+	rcu_read_lock();
 	mas_set(&mas, start);
 	vma =3D mas_walk(&mas);
=20
@@ -3005,6 +3014,7 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, star=
t, unsigned long, size,
 			prot, flags, pgoff, &populate, NULL);
 	fput(file);
 out:
+	rcu_read_unlock();
 	mmap_write_unlock(mm);
 	if (populate)
 		mm_populate(ret, populate);
@@ -3021,7 +3031,8 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, star=
t, unsigned long, size,
  * @oldbrk: The end of the address to unmap
  * @uf: The userfaultfd list_head
  *
- * Returns: 0 on success.
+ * Returns: 0 on success, 1 on success and downgraded write lock, negative
+ * otherwise.
  * unmaps a partial VMA mapping.  Does not handle alignment, downgrades lo=
ck if
  * possible.
  */
@@ -3083,6 +3094,7 @@ static int do_brk_munmap(struct ma_state *mas, struct=
 vm_area_struct *vma,
 		munlock_vma_pages_range(&unmap, newbrk, oldbrk);
 	}
=20
+	mas_unlock(mas);
 	mmap_write_downgrade(mm);
 	unmap_region(mm, &unmap, mas, newbrk, oldbrk, vma,
 		     next ? next->vm_start : 0);
@@ -3165,13 +3177,10 @@ static int do_brk_flags(struct ma_state *mas, struc=
t ma_state *ma_prev,
 				anon_vma_lock_write(vma->anon_vma);
 				anon_vma_interval_tree_pre_update_vma(vma);
 			}
-			mas_lock(ma_prev);
 			vma->vm_end =3D addr + len;
 			vma->vm_flags |=3D VM_SOFTDIRTY;
-			if (mas_store_gfp(ma_prev, vma, GFP_KERNEL)) {
-				mas_unlock(ma_prev);
+			if (mas_store_gfp(ma_prev, vma, GFP_KERNEL))
 				goto mas_mod_fail;
-			}
=20
 			if (vma->anon_vma) {
 				anon_vma_interval_tree_post_update_vma(vma);
@@ -3242,10 +3251,12 @@ int vm_brk_flags(unsigned long addr, unsigned long =
request, unsigned long flags)
 	if (mmap_write_lock_killable(mm))
 		return -EINTR;
=20
+	mas_lock(&mas);
 	// This vma left intentionally blank.
 	mas_walk(&mas);
 	ret =3D do_brk_flags(&mas, &mas, &vma, addr, len, flags);
 	populate =3D ((mm->def_flags & VM_LOCKED) !=3D 0);
+	mas_unlock(&mas);
 	mmap_write_unlock(mm);
 	if (populate && !ret)
 		mm_populate_vma(vma, addr, addr + len);
@@ -3307,9 +3318,10 @@ void exit_mmap(struct mm_struct *mm)
=20
 	arch_exit_mmap(mm);
=20
+	mas_lock(&mas);
 	vma =3D mas_find(&mas, ULONG_MAX);
 	if (!vma) { /* Can happen if dup_mmap() received an OOM */
-		rcu_read_unlock();
+		mas_unlock(&mas);
 		return;
 	}
=20
@@ -3322,6 +3334,7 @@ void exit_mmap(struct mm_struct *mm)
 	unmap_vmas(&tlb, vma, &mas, 0, -1);
 	free_pgtables(&tlb, &mas2, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING=
);
 	tlb_finish_mmu(&tlb);
+	mas_unlock(&mas);
=20
 	/*
 	 * Walk the list again, actually closing and freeing it,
@@ -3346,12 +3359,16 @@ void exit_mmap(struct mm_struct *mm)
  */
 int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
 {
-	if (find_vma_intersection(mm, vma->vm_start, vma->vm_end))
-		return -ENOMEM;
+	MA_STATE(mas, &mm->mm_mt, vma->vm_start, vma->vm_end - 1);
+
+	mas_lock(&mas);
+	if (mas_find(&mas, vma->vm_end - 1))
+		goto no_mem;
=20
 	if ((vma->vm_flags & VM_ACCOUNT) &&
 	     security_vm_enough_memory_mm(mm, vma_pages(vma)))
-		return -ENOMEM;
+		goto no_mem;
+
=20
 	/*
 	 * The vm_pgoff of a purely anonymous vma should be irrelevant
@@ -3370,8 +3387,14 @@ int insert_vm_struct(struct mm_struct *mm, struct vm=
_area_struct *vma)
 		vma->vm_pgoff =3D vma->vm_start >> PAGE_SHIFT;
 	}
=20
-	vma_link(mm, vma);
+	mas_reset(&mas);
+	vma_mas_link(mm, vma, &mas);
+	mas_unlock(&mas);
 	return 0;
+
+no_mem:
+	mas_unlock(&mas);
+	return -ENOMEM;
 }
=20
 /*
diff --git a/mm/nommu.c b/mm/nommu.c
index a99e276445ce..65eee2770625 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -571,6 +571,7 @@ static void add_vma_to_mm(struct mm_struct *mm, struct =
vm_area_struct *vma)
=20
 	BUG_ON(!vma->vm_region);
=20
+	mas_lock(&mas);
 	mm->map_count++;
 	printk("mm at %u\n", mm->map_count);
 	vma->vm_mm =3D mm;
@@ -592,6 +593,7 @@ static void add_vma_to_mm(struct mm_struct *mm, struct =
vm_area_struct *vma)
 	mas_reset(&mas);
 	/* add the VMA to the tree */
 	vma_mas_store(vma, &mas);
+	mas_unlock(&mas);
 }
=20
 /*
@@ -601,6 +603,7 @@ static void delete_vma_from_mm(struct vm_area_struct *v=
ma)
 {
 	MA_STATE(mas, &vma->vm_mm->mm_mt, 0, 0);
=20
+	mas_lock(&mas);
 	vma->vm_mm->map_count--;
 	/* remove the VMA from the mapping */
 	if (vma->vm_file) {
@@ -616,6 +619,7 @@ static void delete_vma_from_mm(struct vm_area_struct *v=
ma)
=20
 	/* remove from the MM's tree and list */
 	vma_mas_remove(vma, &mas);
+	mas_unlock(&mas);
 }
=20
 /*
--=20
2.30.2