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 0DA4EE69185 for ; Fri, 22 Nov 2024 17:47:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 984806B0089; Fri, 22 Nov 2024 12:47:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 90E346B008C; Fri, 22 Nov 2024 12:47:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7878B6B0093; Fri, 22 Nov 2024 12:47:43 -0500 (EST) 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 51AF16B0089 for ; Fri, 22 Nov 2024 12:47:43 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 072C0AF29D for ; Fri, 22 Nov 2024 17:47:43 +0000 (UTC) X-FDA: 82814460792.03.53B179E Received: from mail-qt1-f175.google.com (mail-qt1-f175.google.com [209.85.160.175]) by imf23.hostedemail.com (Postfix) with ESMTP id 88387140003 for ; Fri, 22 Nov 2024 17:47:05 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=A9ABiB4o; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.160.175 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1732297593; a=rsa-sha256; cv=none; b=XRXlXi1SxhPFEs4Lli0gK/Kkjh2Va9HtgrKOMn489iYNWrhjyDZLqJZVgDoUGMxAOn9KgE 5obJTOjL5/nfb+U6Zwlji2SOCGCUjnSSQLnS4DP5fjW87KYRIMx+2SS0vnJaC5TLamIFcn xhR/dOvPAjs9Pm+3/24BpGcJMg8pw2g= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=A9ABiB4o; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.160.175 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1732297593; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc: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=NXu6AT/ibx2XVcD9pO1ZbpAdsb4GGiSqwFIjR6ukUCw=; b=BUeSIsFQ/pIIdZuBYWFsAna01u0Lkye4oZoY/xJqLVMSE12lCCXfe9/dsrJbV0pwPKfQCo iZcZfhKL9XY4V3XZ5RoFz86lg5+eUybllGsevF5oqwzaKsXjFdaAk0R/+aIOLRMXXo930g 1I+Yk1j+wgSEfdMFVgsvZv9IcNUwP/U= Received: by mail-qt1-f175.google.com with SMTP id d75a77b69052e-4635b03a3cdso251cf.1 for ; Fri, 22 Nov 2024 09:47:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1732297660; x=1732902460; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=NXu6AT/ibx2XVcD9pO1ZbpAdsb4GGiSqwFIjR6ukUCw=; b=A9ABiB4o56DX+bvWdze+jZaEYxU64CxY3dBONy8ONsCsOG7urXsOkoBjRJ5EuMecAA AOm19x+hhPFgk7Xh7gFBJw86IdhzDbLz354GHDJh9Zh9+iXf9IAFEZ56NLPTIvvpjw4x ow58nIiEDv5CNpsUXnYX+erH2zBse0FaCVzrG4Ikagmh5iTanR4fxnlxuCsa7ICl1yfb TzTc3Cjeg9/Bt4Dcxf40Ed11GqwxeKpzliqcsbrdgGKsoA41N+EE12RxPd2ZRfEE5fco /RON8XWIBzG+mytS9HADiXAnyF+CBw5eivV36sTH7u215QVIQ5CeLv8iSvuK4PYB92pO 1A+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1732297660; x=1732902460; h=content-transfer-encoding:cc: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=NXu6AT/ibx2XVcD9pO1ZbpAdsb4GGiSqwFIjR6ukUCw=; b=l9ySK3qo7xA57xXr8XVB+U7K9J+m4COfyKj5qJhSeh32BevCdHapLr7SSlyH4K7+WH fINQ1ht+RF6Dhr2Br+cjcqYwUl5b6AVKzqD87+lQJLhQ9/VWoWms8rdFFgemKwaIbj1i 0rInSENVMOARXC6RohuA2dtON6lqTSQinnrFOkzB+JQR5/wbNo/kYGFwSlZO1GoJ7htf alovNAqmNTtYqX85DqHGimHG5ExUcG1S/VncAROT3iAqsoQHpnpI8HSx6s6zTkA/NKiE lcdTSAmDdzN9SkOhL6+qzBgobSJxAYtI0CPPUDXA/XcgmHiZf9ysg56jDmuT+K2m4/L9 h8cA== X-Forwarded-Encrypted: i=1; AJvYcCWF5SJ1PyG87DRVojjPEPM7TtSOcSyZGRLUpzG9iGWNjKXmcGI3NMeadqIAc8UFKwSHf9Mh6BSJgg==@kvack.org X-Gm-Message-State: AOJu0Yxh8yfpehXztbjdfxkbS2HdIe2vmY06OfuPNGTPk+2s0ZVNmSUD +nR176EvnYpirKSg+nee7wjgLuAd0EnkzNIRF5s0mbeFix7yyNV63nJgqIdwFJcZYvX1hRIu5vI ZLNHesizKMSNRcQP6Su/2kQEadgtwh/fKkLPF X-Gm-Gg: ASbGnctPsZXq0noBaXJoBfTqtlmNclWpmBdhA7+aZKYPJSyBx5o1kI6XpgthPo84kkZ vU7yiDZemadygsp+7lP4QrdSFBJdvZ0hRMMgcTWyVNnko3feEsvABu13yC8BohA== X-Google-Smtp-Source: AGHT+IFmgMkmysbndCTt9BLdV9WRe4B095x+9C/l5f22Xm7RKNPX/qreBplcxtPmMN6ho4wucxBBIS3mf6jg7JZqSiI= X-Received: by 2002:a05:622a:206:b0:460:463d:78dd with SMTP id d75a77b69052e-4653d472de8mr4011941cf.4.1732297659816; Fri, 22 Nov 2024 09:47:39 -0800 (PST) MIME-Version: 1.0 References: <20241120000826.335387-1-surenb@google.com> <20241120000826.335387-4-surenb@google.com> <6460975a-46b2-481a-9073-55a5f2ac2f46@lucifer.local> In-Reply-To: <6460975a-46b2-481a-9073-55a5f2ac2f46@lucifer.local> From: Suren Baghdasaryan Date: Fri, 22 Nov 2024 09:47:28 -0800 Message-ID: Subject: Re: [PATCH v4 3/5] mm: mark vma as detached until it's added into vma tree To: Lorenzo Stoakes Cc: akpm@linux-foundation.org, willy@infradead.org, liam.howlett@oracle.com, mhocko@suse.com, vbabka@suse.cz, hannes@cmpxchg.org, mjguzik@gmail.com, oliver.sang@intel.com, mgorman@techsingularity.net, david@redhat.com, peterx@redhat.com, oleg@redhat.com, dave@stgolabs.net, paulmck@kernel.org, brauner@kernel.org, dhowells@redhat.com, hdanton@sina.com, hughd@google.com, minchan@google.com, jannh@google.com, shakeel.butt@linux.dev, souravpanda@google.com, pasha.tatashin@soleen.com, corbet@lwn.net, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: 88387140003 X-Rspamd-Server: rspam01 X-Stat-Signature: w9zc75ssgn3gb1dgmg9gm67wyibwxdyc X-HE-Tag: 1732297625-753659 X-HE-Meta: U2FsdGVkX1/7lhF8VtEKozDXF91YFTiaK4ldEOJI+Swc3XIBQBybFrW6PwOrMwfvkw8nhWGC02y9BrVLsP0KiomXg4sqIeKkkAlyZWcxE7vmUchMRelV3qJQGO8JMM1A9btzLEcULiMLCvZYCPBvqGek2aR/GvO1u+gbkZOJHotIdePZz3/juHrTcxuNawO0L9byr/Y8i7vGlvX6aWmwsAxY5vEGEIT2QwhDDBcUILaQN+N71ARgymJ6UIfbX5+SHh1cWp9uSMF4bjsZ+RFT+yhcIgKh8P+vugY1kKD+cP1/UAZs5l5W68h0rVulu/q5ZdyoVj6RIQmfu/vMTCIzaRgNDQt98f1/nPjwyUQQHY3WjcJY4nZ1bonpHm0AsEIeW9OWsubqe6Cx1yqPylULbtE6dmfWa35gaJZyK+P9LIkN6Dnp+8oMqIZPtDH9V7oVCvHJYfOx7aFGo41GdETcFK5U+SvvOxs1xZcnkmdX/qu9KyOWOj4jRx+Zx6xNhRjD98B8wZ5YvPQGpn2BI7zcS90NbTEKxbgZ6DwzmLrNj9FoKEbs3cFb+M/7YPnsvRIwhbeFzKRLshwBq196hpM37rUIBwduITrjVFmFrIQjMsYk9aTpf8OCKUbqQtmskBaKqwolSLtF6HIjSic4QIP8bZLPIBEzT/b80njB9k0JJcN0nWjo2UEf/KMhpJonp7zmQDXu9gsOtrQ58w5rPyOmJWPJ31upOhLJ9Q0aaOEh7CY+bnPT4tQbZPbQWGK4SyKBOfa4SWZ4En2NHMvdgknKTDB+RZPzX1JeDngLnH+/usX5STFmqBMrYNYyDyWuBZl1ajclLFzN70WOF7lQ87li1lwA3TRN/DpobkEK1j7DumvTefcejntYxuUoxgOifeCDlUOBjWDRzUnFk7ry4ywBXByyy5Yh2YSOZEWGre+nvh1lk28XaRcWtKmYwz7MlOBq/ZvnRx3MWxbI2bHfr9c SqLOLSOI OBjuZ+Vvcnlp7c2AkCsxagZcn3m2TqXw/dXzuF6ZmJcuyEXki553Qf+0fQ8YAsxlACRVpY3BlzUkI820EdfVdVzHW+WUZ/a1R4hRABFR4k8et0SiPe8c/IFQWAjfP9KEGBe5uCG8mowI3TCPvBidhR9AyiVj02nAbAeL3N/gviKcNYclS9syggDZFyYIQhjBtokQyZNYKLCIHMjnvK8h8OJK0GVCJMHah0rJwW9dWtrELdXXmPZwElR6OYw== 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: List-Subscribe: List-Unsubscribe: On Fri, Nov 22, 2024 at 8:47=E2=80=AFAM Lorenzo Stoakes wrote: > > On Tue, Nov 19, 2024 at 04:08:24PM -0800, Suren Baghdasaryan wrote: > > Current implementation does not set detached flag when a VMA is first > > allocated. This does not represent the real state of the VMA, which is > > detached until it is added into mm's VMA tree. Fix this by marking new > > VMAs as detached and resetting detached flag only after VMA is added > > into a tree. > > Introduce vma_mark_attached() to make the API more readable and to > > simplify possible future cleanup when vma->vm_mm might be used to > > indicate detached vma and vma_mark_attached() will need an additional > > mm parameter. > > > > Signed-off-by: Suren Baghdasaryan > > I tested this (whole series) locally and on real hardware and did a kerne= l > compile on real hardware just to be sure :)) and all looks good. > > The code looks sensible, so: > > Reviewed-by: Lorenzo Stoakes Thanks Lorenzo! I'll post a new version today, addressing Matthew's and other's comments but overall functionality should stay the same. > > > --- > > include/linux/mm.h | 27 ++++++++++++++++++++------- > > kernel/fork.c | 4 ++++ > > mm/memory.c | 2 +- > > mm/vma.c | 6 +++--- > > mm/vma.h | 2 ++ > > tools/testing/vma/vma_internal.h | 17 ++++++++++++----- > > 6 files changed, 42 insertions(+), 16 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 737c003b0a1e..dd1b6190df28 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -808,12 +808,21 @@ static inline void vma_assert_locked(struct vm_ar= ea_struct *vma) > > vma_assert_write_locked(vma); > > } > > > > -static inline void vma_mark_detached(struct vm_area_struct *vma, bool = detached) > > +static inline void vma_mark_attached(struct vm_area_struct *vma) > > +{ > > + vma->detached =3D false; > > +} > > + > > +static inline void vma_mark_detached(struct vm_area_struct *vma) > > { > > /* When detaching vma should be write-locked */ > > - if (detached) > > - vma_assert_write_locked(vma); > > - vma->detached =3D detached; > > + vma_assert_write_locked(vma); > > + vma->detached =3D true; > > +} > > + > > +static inline bool is_vma_detached(struct vm_area_struct *vma) > > +{ > > + return vma->detached; > > } > > > > static inline void release_fault_lock(struct vm_fault *vmf) > > @@ -844,8 +853,8 @@ static inline void vma_end_read(struct vm_area_stru= ct *vma) {} > > static inline void vma_start_write(struct vm_area_struct *vma) {} > > static inline void vma_assert_write_locked(struct vm_area_struct *vma) > > { mmap_assert_write_locked(vma->vm_mm); } > > -static inline void vma_mark_detached(struct vm_area_struct *vma, > > - bool detached) {} > > +static inline void vma_mark_attached(struct vm_area_struct *vma) {} > > +static inline void vma_mark_detached(struct vm_area_struct *vma) {} > > > > static inline struct vm_area_struct *lock_vma_under_rcu(struct mm_stru= ct *mm, > > unsigned long address) > > @@ -878,7 +887,10 @@ static inline void vma_init(struct vm_area_struct = *vma, struct mm_struct *mm) > > vma->vm_mm =3D mm; > > vma->vm_ops =3D &vma_dummy_vm_ops; > > INIT_LIST_HEAD(&vma->anon_vma_chain); > > - vma_mark_detached(vma, false); > > +#ifdef CONFIG_PER_VMA_LOCK > > + /* vma is not locked, can't use vma_mark_detached() */ > > + vma->detached =3D true; > > +#endif > > vma_numab_state_init(vma); > > vma_lock_init(vma); > > } > > @@ -1073,6 +1085,7 @@ static inline int vma_iter_bulk_store(struct vma_= iterator *vmi, > > if (unlikely(mas_is_err(&vmi->mas))) > > return -ENOMEM; > > > > + vma_mark_attached(vma); > > return 0; > > } > > > > diff --git a/kernel/fork.c b/kernel/fork.c > > index 7823797e31d2..f0cec673583c 100644 > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -465,6 +465,10 @@ struct vm_area_struct *vm_area_dup(struct vm_area_= struct *orig) > > data_race(memcpy(new, orig, sizeof(*new))); > > vma_lock_init(new); > > INIT_LIST_HEAD(&new->anon_vma_chain); > > +#ifdef CONFIG_PER_VMA_LOCK > > + /* vma is not locked, can't use vma_mark_detached() */ > > + new->detached =3D true; > > +#endif > > vma_numab_state_init(new); > > dup_anon_vma_name(orig, new); > > > > diff --git a/mm/memory.c b/mm/memory.c > > index 209885a4134f..d0197a0c0996 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -6279,7 +6279,7 @@ struct vm_area_struct *lock_vma_under_rcu(struct = mm_struct *mm, > > goto inval; > > > > /* Check if the VMA got isolated after we found it */ > > - if (vma->detached) { > > + if (is_vma_detached(vma)) { > > vma_end_read(vma); > > count_vm_vma_lock_event(VMA_LOCK_MISS); > > /* The area was replaced with another one */ > > diff --git a/mm/vma.c b/mm/vma.c > > index 8a454a7bbc80..73104d434567 100644 > > --- a/mm/vma.c > > +++ b/mm/vma.c > > @@ -295,7 +295,7 @@ static void vma_complete(struct vma_prepare *vp, st= ruct vma_iterator *vmi, > > > > if (vp->remove) { > > again: > > - vma_mark_detached(vp->remove, true); > > + vma_mark_detached(vp->remove); > > if (vp->file) { > > uprobe_munmap(vp->remove, vp->remove->vm_start, > > vp->remove->vm_end); > > @@ -1220,7 +1220,7 @@ static void reattach_vmas(struct ma_state *mas_de= tach) > > > > mas_set(mas_detach, 0); > > mas_for_each(mas_detach, vma, ULONG_MAX) > > - vma_mark_detached(vma, false); > > + vma_mark_attached(vma); > > > > __mt_destroy(mas_detach->tree); > > } > > @@ -1295,7 +1295,7 @@ static int vms_gather_munmap_vmas(struct vma_munm= ap_struct *vms, > > if (error) > > goto munmap_gather_failed; > > > > - vma_mark_detached(next, true); > > + vma_mark_detached(next); > > nrpages =3D vma_pages(next); > > > > vms->nr_pages +=3D nrpages; > > diff --git a/mm/vma.h b/mm/vma.h > > index 388d34748674..2e680f357ace 100644 > > --- a/mm/vma.h > > +++ b/mm/vma.h > > @@ -162,6 +162,7 @@ static inline int vma_iter_store_gfp(struct vma_ite= rator *vmi, > > if (unlikely(mas_is_err(&vmi->mas))) > > return -ENOMEM; > > > > + vma_mark_attached(vma); > > return 0; > > } > > > > @@ -385,6 +386,7 @@ static inline void vma_iter_store(struct vma_iterat= or *vmi, > > > > __mas_set_range(&vmi->mas, vma->vm_start, vma->vm_end - 1); > > mas_store_prealloc(&vmi->mas, vma); > > + vma_mark_attached(vma); > > } > > > > static inline unsigned long vma_iter_addr(struct vma_iterator *vmi) > > diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_i= nternal.h > > index 11c2c38ca4e8..2fed366d20ef 100644 > > --- a/tools/testing/vma/vma_internal.h > > +++ b/tools/testing/vma/vma_internal.h > > @@ -414,13 +414,17 @@ static inline void vma_lock_init(struct vm_area_s= truct *vma) > > vma->vm_lock_seq =3D UINT_MAX; > > } > > > > +static inline void vma_mark_attached(struct vm_area_struct *vma) > > +{ > > + vma->detached =3D false; > > +} > > Yeah I think sensible to just accept that sometimes we are already attach= ed > when we mark attached. > > > + > > static inline void vma_assert_write_locked(struct vm_area_struct *); > > -static inline void vma_mark_detached(struct vm_area_struct *vma, bool = detached) > > +static inline void vma_mark_detached(struct vm_area_struct *vma) > > { > > /* When detaching vma should be write-locked */ > > - if (detached) > > - vma_assert_write_locked(vma); > > - vma->detached =3D detached; > > + vma_assert_write_locked(vma); > > + vma->detached =3D true; > > } > > > > extern const struct vm_operations_struct vma_dummy_vm_ops; > > @@ -431,7 +435,8 @@ static inline void vma_init(struct vm_area_struct *= vma, struct mm_struct *mm) > > vma->vm_mm =3D mm; > > vma->vm_ops =3D &vma_dummy_vm_ops; > > INIT_LIST_HEAD(&vma->anon_vma_chain); > > - vma_mark_detached(vma, false); > > + /* vma is not locked, can't use vma_mark_detached() */ > > + vma->detached =3D true; > > vma_lock_init(vma); > > } > > > > @@ -457,6 +462,8 @@ static inline struct vm_area_struct *vm_area_dup(st= ruct vm_area_struct *orig) > > memcpy(new, orig, sizeof(*new)); > > vma_lock_init(new); > > INIT_LIST_HEAD(&new->anon_vma_chain); > > + /* vma is not locked, can't use vma_mark_detached() */ > > + new->detached =3D true; > > > > return new; > > } > > -- > > 2.47.0.338.g60cca15819-goog > >