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 88680D462C3 for ; Wed, 13 Nov 2024 15:37:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id F1AC96B0096; Wed, 13 Nov 2024 10:37:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id ED4D46B0098; Wed, 13 Nov 2024 10:37:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D6BA46B009D; Wed, 13 Nov 2024 10:37:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id B6BBF6B0096 for ; Wed, 13 Nov 2024 10:37:53 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 66BD0A0B6A for ; Wed, 13 Nov 2024 15:37:53 +0000 (UTC) X-FDA: 82781474202.17.66ADEB9 Received: from mail-qt1-f175.google.com (mail-qt1-f175.google.com [209.85.160.175]) by imf02.hostedemail.com (Postfix) with ESMTP id 188838000B for ; Wed, 13 Nov 2024 15:36:29 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=q7XACVv4; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf02.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=1731512208; a=rsa-sha256; cv=none; b=vlGI790eGhj6rlukctbN5rwpUuKjrcnVh8FHp1PFWwX89MQfti3iaU9wi8oPuZZ5JM9mHc MJp+uOgFlEqh1h1aYUsaaWvwnLUkVcr7gFhCTMkr0mIVtnvpdcDwujvnwgDJve1IPeqLCD fRMMLeiOshMwoytrhSM8alNdZ/6k0pc= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=q7XACVv4; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf02.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=1731512208; 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=YXS5EFs4Cl5CVnqNGjFqLrV5MNciF0+EB1K5+c2asu8=; b=CxPDmXWJiHqDVBMralXciWOahtvlsF3o9p6c7bN6Kqr60WsXenGP2e0B86vNFgkBS+l7Q4 /Pgcn2Z6xs3WKwPMxgC/DnP0ue2pTdpxgZoBoYdyzJO3JZOsrIL6iQEWXIzKXH9cPTgnYF vOh6xaD8nBex4jPCnS4kB39kANxoQdY= Received: by mail-qt1-f175.google.com with SMTP id d75a77b69052e-4608dddaa35so316541cf.0 for ; Wed, 13 Nov 2024 07:37:51 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1731512271; x=1732117071; 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=YXS5EFs4Cl5CVnqNGjFqLrV5MNciF0+EB1K5+c2asu8=; b=q7XACVv4AiT5fodHQ3mr+tjAqkRMluxzp94ad4SMbd3/oJAvWYU3BNGESMlnz61AFQ 4fWBIafBtqsdDePYPpU4grYBo3uW8I7nLq+/rqM8ylwcR1UbybAOYMOq+5xRkAo6X/cY eY7/qNzvO91sXI/Hmddg+oiRYP//6co3W/EOZo0Bj5YktMXZmY2Ye63HnxNfWySG8OUb k81HNmjZLG0cd2Pz3kmxx1KVhLHdIlEDOEoZQxhygx1WSBLSdAVG5iG/ZiGrHt6QotFN fgtHeLq00s2J666E8eeTBMP4vu5hm+gozTc2F9W9ZFtVEPLHwEuxMG7GzvkHxrT1WNnN gU9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1731512271; x=1732117071; 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=YXS5EFs4Cl5CVnqNGjFqLrV5MNciF0+EB1K5+c2asu8=; b=pyzWEOcM73zhwT0tgKmN5CHTx02sRrE7g3RNZaP9elAJkV4JkCm57xE/YBsT4eFUMy 8XcgzvyVymiFw//X37/McNXU7m4v+HJtVMsLyYl323K45K59f6HZ/CkxWtwwcPXSESC8 qK0HecWUEoSKurlpbUqSAfyBeX4Cm31mseZvfArjbB/7TMd7FKo54BjSA6nADI0ZwoYj wL2g1jo/+F9q7MBq1MetDpikT/iBUJgeg3rsWAKhkGGPMCdMcs7rvXQz45WuNo8W+Wmu Ix0taEojCidtmqMsffZ2GKfuoxP/Hu6MoLyr8lMN9NkGTS8QYk0gSoJiW8ooRwrBry/1 TVqg== X-Forwarded-Encrypted: i=1; AJvYcCV0vqOygq38Y9P6DPpFtWWL1TnGQLk+/O1vu9vzlI+XRl4i+Eao/FyUaeUNkJsDKORf21FPSIpX1A==@kvack.org X-Gm-Message-State: AOJu0YxlKI6pE6zY9t+2DlR3SNfiacfmZO5FbK/QpaLN4NUi/JwIqNa0 TUJQQ8e3Yvi4kahm24cq6aV90C44NkD+3l0N5VOMOy4By6IF5mBUWzyATBTm/iQnFZCSJHmEDv2 ryUfntxKvWU3NQi/iVN5B4vGoTHXec0NRIWJh X-Gm-Gg: ASbGncuEKuInEQjKP0ATQhq0avEp6jzRPqK8gqky2tS3g7KtQMeWJtQEr26sawU9pLP +MX94DKL2Ccpb4yOe/CZfOxp2UyroGlY= X-Google-Smtp-Source: AGHT+IHEU1U5dfPZ6mIgDaKJL898ldMOWvfg9vnhENKTzuQ+Pf15wwDYZWhS8Oa0MM4oqkQ4tiKJJOYu8UNHWap1Sis= X-Received: by 2002:a05:622a:47c6:b0:461:679f:f1ba with SMTP id d75a77b69052e-4634bca4e56mr3641501cf.20.1731512270176; Wed, 13 Nov 2024 07:37:50 -0800 (PST) MIME-Version: 1.0 References: <20241112194635.444146-1-surenb@google.com> <20241112194635.444146-4-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 13 Nov 2024 07:37:39 -0800 Message-ID: Subject: Re: [PATCH v2 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, 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: 188838000B X-Rspamd-Server: rspam01 X-Stat-Signature: 4b8fqtcss9zwhcyjhoe9qhbdqya8y3fb X-HE-Tag: 1731512189-73364 X-HE-Meta: U2FsdGVkX19J9qoy5DhVwMb3AGJqDr+akfQAk5bkwLTZvNdYzMWjxkamH+T6tsS0Icp39xeEI08jJwZ704fTAH/LeltHV4aQG/VEEmXKBx4HJrNMsFLN2GF4KASq+TgWUotB9HuRAa3xMblBQocq6xhnali4d7sNsUZlwk2x/d8QDES68+Yi8HQI4DRGXoUmTgkiTkzAZVdMEdzA+Se3mTCDiejoKVP6GQHfGdVvCE6Z5NqZ8Ep8vmjlxNrn74k8a/V78drNpyU7JGVOSQZhcXyNtfkxlUT7McveFAFv0UTgej5MalruNtfrcYVYcHX6ra0gxeMaIDd3lP9Lx2jbhn03lJ9EWCZgbSPyyJbGjaZK5owti0npjSLE4dwfYH46d3UAuiypj7/M2bixFfVOKDscuFTW0NXoBwRVr9NkLddeTOPCPk5bWKktVHzr5JP8jNto4SPxl9hrenlEn7Lx95tNrgIqImwqQD2tIAZjtI6FHJs/1iQA/w4llrvBy1keVIZP6M9byrbG0K2reaMqwWtB4WI4H+UXFimbLVZFB7mofxOZlHv5OJ69DpPwiqUQMwzPu3fhSZBofSF3qtZZez5FOWJl1xXBAyF9rnHuT5LmJztFTvpfh7LnUrDm7GhJjf843Tmg3HowkxL4sB91Ijm1B/tzk4qySl0FtJ21XYB5hYA7NXM2J5EqkZ0BaB/74XstKxOTtVJfnoyU6/G+NKw8EptAN14CBrm0/HarGFzdcM/W9GfPASFcdsgQ4KmTPpxdCcD0nFrxv8OSVu993zRceZuqgrcRGDM5VuZLdOS+k/ki0ScCEPbGcCvFTGwSBu7UDegYXcnCdjULEgZfAEwjPMHOKIy0L5+nhK1jIjbFHl9tQ2XAV3DxlbG6zLbzAAYH8UwrlXDK5cWKkEYQGQx4lSCBIrRGLkIhTjSV5lEals9lPrEQ9akpCPBfZD7PfnWNtk2xbTmRdh3Hokl sg+ZzRvK xGm808qf0H3znq8F8naxg+thEyTorpZUuew2I6G6AdeULuLYokxCPIYY5qD2RkajKho8v/Boqb4yM06k06uDbydmcMir7Oj1G9k97v/DcBodZFuOdQY6zEMM2YqxfQCnVpAoyuPCFw1jTuCzE5n5QE5o1YXjfakRfhNXdIaXazdMGEnMndOIvP0Y04W+sEUmJDph6atcTCBUZc4fAAPGHRDYwQFUrYVL7TYVw91fWK+sfw+FuYFLeSWdUYA== 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 Wed, Nov 13, 2024 at 6:43=E2=80=AFAM Lorenzo Stoakes wrote: > > On Tue, Nov 12, 2024 at 11:46:33AM -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. > > > > This seems very sensible. > > > Signed-off-by: Suren Baghdasaryan > > Aside from nits/refactoring suggestions below: > > Reviewed-by: Lorenzo Stoakes > > > --- > > include/linux/mm.h | 10 +++++++++- > > mm/memory.c | 2 +- > > mm/mmap.c | 2 ++ > > mm/nommu.c | 2 ++ > > mm/vma.c | 3 +++ > > tools/testing/vma/vma_internal.h | 3 ++- > > Just want to say THANK YOU for taking the time to update the testing stub= s :) > Much appreciated! > > > 6 files changed, 19 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index a5eb0be3e351..245a85caf4c3 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -812,6 +812,11 @@ static inline void vma_mark_detached(struct vm_are= a_struct *vma, bool detached) > > vma->detached =3D detached; > > } > > > > +static inline bool is_vma_detached(struct vm_area_struct *vma) > > +{ > > + return vma->detached; > > +} > > + > > static inline void release_fault_lock(struct vm_fault *vmf) > > { > > if (vmf->flags & FAULT_FLAG_VMA_LOCK) > > @@ -874,7 +879,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); > > How did this work before? Oh I guess we initialised the VMA lock earlier = right? Yes. > > > +#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); > > } > > 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/mmap.c b/mm/mmap.c > > index 386429f7db5a..1295c4cedaf4 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -1570,6 +1570,7 @@ static int do_brk_flags(struct vma_iterator *vmi,= struct vm_area_struct *vma, > > if (vma_iter_store_gfp(vmi, vma, GFP_KERNEL)) > > goto mas_store_fail; > > > > + vma_mark_detached(vma, false); > > mm->map_count++; > > validate_mm(mm); > > ksm_add_vma(vma); > > @@ -1890,6 +1891,7 @@ static struct vm_area_struct *__install_special_m= apping( > > if (ret) > > goto out; > > > > + vma_mark_detached(vma, false); > > similar to vma_iter_store() comment, maybe worht putting in insert_vm_str= uct()? Ah, this one I think is not needed because we already have insert_vm_struct() -> vma_link() -> vma_mark_detached(vma, false) > > > vm_stat_account(mm, vma->vm_flags, len >> PAGE_SHIFT); > > > > perf_event_mmap(vma); > > diff --git a/mm/nommu.c b/mm/nommu.c > > index 9cb6e99215e2..6afd5c2bd97d 100644 > > --- a/mm/nommu.c > > +++ b/mm/nommu.c > > @@ -1192,6 +1192,7 @@ unsigned long do_mmap(struct file *file, > > current->mm->map_count++; > > /* add the VMA to the tree */ > > vma_iter_store(&vmi, vma); > > + vma_mark_detached(vma, false); > > Since we to seem always to do this with vma_iter_store() do we want to pu= t this > there? Or maybe make a wrapper around the two if that seems not to separa= te > concerns enough? I think wrapper would be helpful. I'll try that and see if that looks bette= r. > > > > > /* we flush the region from the icache only when the first execut= able > > * mapping of it is made */ > > @@ -1357,6 +1358,7 @@ static int split_vma(struct vma_iterator *vmi, st= ruct vm_area_struct *vma, > > setup_vma_to_mm(vma, mm); > > setup_vma_to_mm(new, mm); > > vma_iter_store(vmi, new); > > + vma_mark_detached(new, false); > > mm->map_count++; > > return 0; > > > > diff --git a/mm/vma.c b/mm/vma.c > > index 8a454a7bbc80..1426871fa6e0 100644 > > --- a/mm/vma.c > > +++ b/mm/vma.c > > @@ -275,6 +275,7 @@ static void vma_complete(struct vma_prepare *vp, st= ruct vma_iterator *vmi, > > * (it may either follow vma or precede it). > > */ > > vma_iter_store(vmi, vp->insert); > > + vma_mark_detached(vp->insert, false); > > mm->map_count++; > > } > > > > @@ -1690,6 +1691,7 @@ int vma_link(struct mm_struct *mm, struct vm_area= _struct *vma) > > > > vma_start_write(vma); > > vma_iter_store(&vmi, vma); > > + vma_mark_detached(vma, false); > > vma_link_file(vma); > > mm->map_count++; > > validate_mm(mm); > > @@ -2369,6 +2371,7 @@ static int __mmap_new_vma(struct mmap_state *map,= struct vm_area_struct **vmap) > > /* Lock the VMA since it is modified after insertion into VMA tre= e */ > > vma_start_write(vma); > > vma_iter_store(vmi, vma); > > + vma_mark_detached(vma, false); > > map->mm->map_count++; > > vma_link_file(vma); > > > > diff --git a/tools/testing/vma/vma_internal.h b/tools/testing/vma/vma_i= nternal.h > > index 1d9fc97b8e80..fdb60978821f 100644 > > --- a/tools/testing/vma/vma_internal.h > > +++ b/tools/testing/vma/vma_internal.h > > @@ -438,7 +438,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; > > You're the best :) Thanks! > > > } > > > > static inline struct vm_area_struct *vm_area_alloc(struct mm_struct *m= m) > > -- > > 2.47.0.277.g8800431eea-goog > >