From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B4518C001DB for ; Thu, 3 Aug 2023 19:15:15 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 48C2F280299; Thu, 3 Aug 2023 15:15:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 43C2E28022C; Thu, 3 Aug 2023 15:15:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2DCD5280299; Thu, 3 Aug 2023 15:15:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 1E54A28022C for ; Thu, 3 Aug 2023 15:15:15 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id D47F3B2DA9 for ; Thu, 3 Aug 2023 19:15:14 +0000 (UTC) X-FDA: 81083746548.10.0EFEC40 Received: from mail-yb1-f169.google.com (mail-yb1-f169.google.com [209.85.219.169]) by imf26.hostedemail.com (Postfix) with ESMTP id CF249140019 for ; Thu, 3 Aug 2023 19:15:12 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=vbHQuteP; spf=pass (imf26.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691090113; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=djFJX1OhEMWIt9+JtDxnjKllG6+RaJwtr30/ZUm6fls=; b=dYguD3YPjnDAt1KqJ5BxwoZtZ3DxizVrzSABYLrCPhV80s2LJndbdvcIXhdvWdK8RQg827 DcBFpeblxDoafuDSGY7MT67nyyHh/TbQ4GaU6KhBfHBvQPDypz0hCPBoPxY6cvJAk0yaC9 rmAWnb752tCtNshb/LO/UlKG4W4aS0c= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691090113; a=rsa-sha256; cv=none; b=KPygPeBoR+vKR69JDmmXscUJf+EhvS7VdcQ6/e5+tmovlketDVCPF18E6oacLwijHMPmBN W0uIxfBlUHZy0a0fAjwyNN76kW0oRTCh+y4bouicT8NuAjznMDOBFvykNNE/Jsyos+inuC 65f+fNo+ZyDJQaM0q41ljgq9+/v/oUc= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=vbHQuteP; spf=pass (imf26.hostedemail.com: domain of surenb@google.com designates 209.85.219.169 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-yb1-f169.google.com with SMTP id 3f1490d57ef6-d066d72eb12so1960863276.1 for ; Thu, 03 Aug 2023 12:15:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691090112; x=1691694912; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=djFJX1OhEMWIt9+JtDxnjKllG6+RaJwtr30/ZUm6fls=; b=vbHQutePvfDyc688KRgvZbfnElOsr7PcYYJp1WDfyRRLzMDnIBjPQLQjI8YY1zdZZI RKmRVL4I9yQmuwm4KK8uj6/A1nC5/W1+ocpcRKI9/mrbO6BVHZ5jy91Ox2uzYuLUVeT9 XxN53hBKG/9s5KEhQM8n0dtOHhBsFaXlbCfLZONxIjRj59EMWysgWa2nBla2AHronVJL 0TCHmbsE5yxZfcQcZLEb04BF4/voHVVBvD2SpwUFjDsmlFPwZsd/eb1eP0ANpIXUv/Dp 48JziEwYKcX6wNGn8dahnJ73PhywUDwA68H7k7Ajlh9H0NTbvJYOgxCm4vFLiHqvCz51 E5OQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691090112; x=1691694912; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=djFJX1OhEMWIt9+JtDxnjKllG6+RaJwtr30/ZUm6fls=; b=f9kH74XvnjmMesROU7obbg930VLJ2yMGGpX2SiHlHq6s/atEaGCo0joSqLG5lOETbT eGzWwfbHX1iwUsTygu8GEgPa1DZUbPacEDB88QSJcsarJlk1fRpCQchEkbLQVVpsYI74 fZYWf49ELCHCPVJeBGhrF+mv4C0HPKsRvV3eXay6DeWiIfjBZ473iWi25M/dJviyC2S0 FjDtAlzV2ZV4uZbPxtQtzOAVITtUQdQ3OIo8eI6u2StwMQXPrlKdw9uCos8RiNPZx4TW C4mj+tTZNSlwA3og770NoRwTzvtS7PxI+NXZhj1vWJ+InISJfZO9elkwhY3zLRjW2uRX vUMw== X-Gm-Message-State: ABy/qLbqZh/aHxfTEAOFU+G7tR3foQumsiVvSge75429zXU35UwzXEc2 5E/0MQrMJFTyYPxZ2/vIfZxK82dlbk0RZEVEiwwWfQ== X-Google-Smtp-Source: APBJJlGlDnONnDU/o3MkHLp0kgBm0ml4OcPWLSX7F+2EQESh8CWjBvM5PsMGQHsHyiPP4vnpnBnGIkNFlPuPQjSLX30= X-Received: by 2002:a25:b28a:0:b0:d03:37fe:5ae6 with SMTP id k10-20020a25b28a000000b00d0337fe5ae6mr21693483ybj.22.1691090111689; Thu, 03 Aug 2023 12:15:11 -0700 (PDT) MIME-Version: 1.0 References: <20230803172652.2849981-1-surenb@google.com> <20230803172652.2849981-7-surenb@google.com> <20230803183228.zreczwv3g3qp4kux@revolver> In-Reply-To: <20230803183228.zreczwv3g3qp4kux@revolver> From: Suren Baghdasaryan Date: Thu, 3 Aug 2023 12:14:58 -0700 Message-ID: Subject: Re: [PATCH v3 6/6] mm: move vma locking out of vma_prepare and dup_anon_vma To: "Liam R. Howlett" , Suren Baghdasaryan , akpm@linux-foundation.org, torvalds@linux-foundation.org, jannh@google.com, willy@infradead.org, david@redhat.com, peterx@redhat.com, ldufour@linux.ibm.com, vbabka@suse.cz, michel@lespinasse.org, jglisse@google.com, mhocko@suse.com, hannes@cmpxchg.org, dave@stgolabs.net, hughd@google.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org, stable@vger.kernel.org, Linus Torvalds Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: CF249140019 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: 9xsx5xca8osfritwr8ec3nhjybspz6q8 X-HE-Tag: 1691090112-801217 X-HE-Meta: U2FsdGVkX1/99eGOxVH6WmbXu4QPQE1y7gXrzp0f/372aO0Fif0SHyA/041PE+n7h+LIpW9b5EdQ5rC8Ig+xma8N3zbIRkMOQ01BKZyW59ZWZrApuGCVnO7EP60XbtRutxQ2mE8NWyfcXxYEIH5C0DDeq+BINZBWUy+7BENTDVMmGhrG/08RXm1Wjq52hetfgwu+BrhekzMVPpoVnYI4FwgFv35JTjRpxdGRyh0PxFWYhOkEsq3Hun3zRbvCUbNqSxCbeDT34HXKf+QqNwhc8goYYMP1j9laAr5qhEslAuavyBRCLGuZ4RPtmQfXoBHrV0iP/r0pRzxtRlj5n03fnuu9W7CSZ0Wo6xqzL1wSVL6nmFSHX/OpUzFxC/K3FHlKGRdetbGfxgB/6pYgpsHvF6E5qA0Mhvp+erbK6UqVVohxjnrPB+ho2HYiGnR7zp6jsZGWdTlwz6ga1ho+4tmItJfuEJ2EY+crSgJbBxS6RpnOmBUVUyRFwMuHVr8gCfPbLzVbTHTQ6jPgYHwIIsOVgGTvEnDh21aj3NezqZY5yaYPPROQWj67p0CgZfT/1ALkBzp3sPQlDQj0IZSifTwh4R1S0NpD+YGYFM4Xcv5eDVovpXPK3FwOrMrrbp34cd+wMzWqnC8jxokgdtWMU0f97uD/beEsd+4c1Xafwtg5d/kslHMBAhD4y/zujXP1ek5jNwOaMBPvpxqvJIwmXZI/y5pD4zr53UP85bFPuXfTL9Itwa/g735nK5kXNeP5+RGUwNTp7JXpr/p7BUSEzqUfZ61USDcW7liAiXjSPkaJb6Vs3drVcYvN9JXNmV9tHWGhMZE59mrmNpk9wVJVRNQmZFtUQwZIH6NRjb5uF8L4nQR8U89DDJfg0snIG3gt9cEZMU8sOsZLNzRtBZzm84c6p6C3HVLPHRkbre0bA/HZlXI8v1QVETNY4FmbovJufkd0b+qPCCDGDIQiXi0GK1C JZDWB+SS d6t+5t/AU7gM+CYQLVObT/ixaIWi9li/4grtb3d4Bugjj7tjxo1QTm+KwuAdAvKz0cij+09fYpVv1AF/+lD493oOgOj8Lfd76XxH1UFbg5PJBvQkSg8rFdLVfy9Aws7+sbIu2OYM6N3RuLxloNEPoUW0HpBUeammTAdS3nFhu5I1HKmSaAo2EoKXt/joa33hJrXqekRgUGefylUJRZix8YQe5uQH8G9IzsDR7qyvedgdVMbi/x2Zo4e2iDupY8oTw0OAhDdQIvF+meqo4U1/LZqXKhIPoDKALqogF8DSjVdgrhz9VPoH9LZVu3GxPuWFF6PEvAL9GI6OhS2FxiWfXzW3jyzJIQli4ya1s7gksdJ6wl/IFeT1iq3CsOqfGB4V2yfX/ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Aug 3, 2023 at 11:32=E2=80=AFAM Liam R. Howlett wrote: > > * Suren Baghdasaryan [230803 13:27]: > > vma_prepare() is currently the central place where vmas are being locke= d > > before vma_complete() applies changes to them. While this is convenient= , > > it also obscures vma locking and makes it harder to follow the locking > > rules. Move vma locking out of vma_prepare() and take vma locks > > explicitly at the locations where vmas are being modified. Move vma > > locking and replace it with an assertion inside dup_anon_vma() to furth= er > > clarify the locking pattern inside vma_merge(). > > > > Suggested-by: Linus Torvalds > > Suggested-by: Liam R. Howlett > > Signed-off-by: Suren Baghdasaryan > > --- > > mm/mmap.c | 29 ++++++++++++++++++----------- > > 1 file changed, 18 insertions(+), 11 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 850a39dee075..ae28d6f94c34 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -476,16 +476,6 @@ static inline void init_vma_prep(struct vma_prepar= e *vp, > > */ > > static inline void vma_prepare(struct vma_prepare *vp) > > { > > - vma_start_write(vp->vma); > > - if (vp->adj_next) > > - vma_start_write(vp->adj_next); > > - if (vp->insert) > > - vma_start_write(vp->insert); > > - if (vp->remove) > > - vma_start_write(vp->remove); > > - if (vp->remove2) > > - vma_start_write(vp->remove2); > > - > > if (vp->file) { > > uprobe_munmap(vp->vma, vp->vma->vm_start, vp->vma->vm_end= ); > > > > @@ -618,7 +608,7 @@ static inline int dup_anon_vma(struct vm_area_struc= t *dst, > > * anon pages imported. > > */ > > if (src->anon_vma && !dst->anon_vma) { > > - vma_start_write(dst); > > + vma_assert_write_locked(dst); > > dst->anon_vma =3D src->anon_vma; > > return anon_vma_clone(dst, src); > > } > > @@ -650,10 +640,12 @@ int vma_expand(struct vma_iterator *vmi, struct v= m_area_struct *vma, > > bool remove_next =3D false; > > struct vma_prepare vp; > > > > + vma_start_write(vma); > > if (next && (vma !=3D next) && (end =3D=3D next->vm_end)) { > > int ret; > > > > remove_next =3D true; > > + vma_start_write(next); > > ret =3D dup_anon_vma(vma, next); > > if (ret) > > return ret; > > @@ -708,6 +700,8 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_= area_struct *vma, > > if (vma_iter_prealloc(vmi)) > > return -ENOMEM; > > > > + vma_start_write(vma); > > + > > init_vma_prep(&vp, vma); > > vma_prepare(&vp); > > vma_adjust_trans_huge(vma, start, end, 0); > > @@ -940,16 +934,21 @@ struct vm_area_struct *vma_merge(struct vma_itera= tor *vmi, struct mm_struct *mm, > > if (!merge_prev && !merge_next) > > return NULL; /* Not mergeable. */ > > > > + if (prev) > > Maybe if (merge_prev) instead of prev? We will write lock prev if it > exists and won't change with the current check (case 3 and 8, > specifically), with this change case 4 will need to lock prev as it > shifts prev->vm_end lower. Ah, I see. I was trying to make sure we don't miss any locks and over-locked it for case 3 and 8. Ok, I'll change the check to if (merge_prev) and will add a separate locking for case 4. I think that's what you meant? > > > + vma_start_write(prev); > > + > > res =3D vma =3D prev; > > remove =3D remove2 =3D adjust =3D NULL; > > > > /* Can we merge both the predecessor and the successor? */ > > if (merge_prev && merge_next && > > is_mergeable_anon_vma(prev->anon_vma, next->anon_vma, NULL)) = { > > + vma_start_write(next); > > remove =3D next; /* case 1 */ > > vma_end =3D next->vm_end; > > err =3D dup_anon_vma(prev, next); > > if (curr) { /* case 6 */ > > + vma_start_write(curr); > > remove =3D curr; > > remove2 =3D next; > > if (!next->anon_vma) > > @@ -957,6 +956,7 @@ struct vm_area_struct *vma_merge(struct vma_iterato= r *vmi, struct mm_struct *mm, > > } > > } else if (merge_prev) { /* case 2 */ > > if (curr) { > > + vma_start_write(curr); > > err =3D dup_anon_vma(prev, curr); > > if (end =3D=3D curr->vm_end) { /* case 7 */ > > remove =3D curr; > > @@ -966,6 +966,7 @@ struct vm_area_struct *vma_merge(struct vma_iterato= r *vmi, struct mm_struct *mm, > > } > > } > > } else { /* merge_next */ > > + vma_start_write(next); > > res =3D next; > > if (prev && addr < prev->vm_end) { /* case 4 */ > > vma_end =3D addr; > > @@ -983,6 +984,7 @@ struct vm_area_struct *vma_merge(struct vma_iterato= r *vmi, struct mm_struct *mm, > > vma_pgoff =3D next->vm_pgoff - pglen; > > if (curr) { /* case 8 */ > > vma_pgoff =3D curr->vm_pgoff; > > + vma_start_write(curr); > > remove =3D curr; > > err =3D dup_anon_vma(next, curr); > > } > > @@ -2373,6 +2375,9 @@ int __split_vma(struct vma_iterator *vmi, struct = vm_area_struct *vma, > > if (new->vm_ops && new->vm_ops->open) > > new->vm_ops->open(new); > > > > + vma_start_write(vma); > > + vma_start_write(new); > > + > > init_vma_prep(&vp, vma); > > vp.insert =3D new; > > vma_prepare(&vp); > > @@ -3078,6 +3083,8 @@ static int do_brk_flags(struct vma_iterator *vmi,= struct vm_area_struct *vma, > > if (vma_iter_prealloc(vmi)) > > goto unacct_fail; > > > > + vma_start_write(vma); > > + > > init_vma_prep(&vp, vma); > > vma_prepare(&vp); > > vma_adjust_trans_huge(vma, vma->vm_start, addr + len, 0); > > -- > > 2.41.0.585.gd2178a4bd4-goog > >