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 096C5C02180 for ; Mon, 13 Jan 2025 16:32:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8D28F6B0092; Mon, 13 Jan 2025 11:32:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 85B7E6B0095; Mon, 13 Jan 2025 11:32:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6AEE96B0098; Mon, 13 Jan 2025 11:32:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 489BA6B0092 for ; Mon, 13 Jan 2025 11:32:00 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id E88D7C0142 for ; Mon, 13 Jan 2025 16:31:59 +0000 (UTC) X-FDA: 83002970358.14.7D70E51 Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf28.hostedemail.com (Postfix) with ESMTP id 15EB7C0010 for ; Mon, 13 Jan 2025 16:31:57 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cVZcmXcG; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 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=1736785918; 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=RBPvIovHQiD9n8xzkcacIlBFecfDAv7IoE1iz0kn/Yo=; b=Mf/guhuZV4YGsnsnOwtKAi3V1RoDnX9SsFmmF/Q3cmZ/cAb7ayj9aKbMWzPZNE5HUN82S5 It66KQm8rinKbeqnxXHgad62vP/zAu3CU9/vS7eoLm5N/PjXrUlitUYHQbDgNJsQpiblxw 8WRt1pCVIPln8KomS7YRkXprRXUu68Q= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1736785918; a=rsa-sha256; cv=none; b=Jeh3y1XEiuT1dhLUs2TcL0OnTf+/DeYfbCwmPvKZXkfrt53O11p58sbSnR3Pr7DykNkXJv nbzBiJtxj2L42IOkcTYNO0cnjHtpZUnFVRmfKnr3O26Y0lBuQ8x/XnXRYNJrzgEG7sQ6Fq Uu8brWN2O4UVOWn1eqb6m9bRRPKaRss= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=cVZcmXcG; spf=pass (imf28.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-4678c9310afso380521cf.1 for ; Mon, 13 Jan 2025 08:31:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736785917; x=1737390717; 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=RBPvIovHQiD9n8xzkcacIlBFecfDAv7IoE1iz0kn/Yo=; b=cVZcmXcGuIrEW71/FVStDZHk1jhL5D29xMvzrhzvFcsHu12SdZPSQ7jhUcwQT+iKu/ 1KeGIvGJeJU3jBwoFzYNwX1TN3HjeMg7CIn/5qg5zi+0RhgmVpp4jaVB1HHmE/3k5h7h 8DkhzQQZDHXq6XrPfDzyZ0G+JWzJ2YBW92b8c/355eXlVywSpKEUu57OOAa6pM1W96sk 7CjuGT67jtVNoPkJthQR60dkNqskt+XhLufEYkh5rYLZ3fQe5VqdGivc1Rp1o3/JsKeE L9KnuNMK0ackYwKwbSVc/mxauF3THo7fB84xSqX+AkkV/3GIIsoxlxClfb7GVfJMlBvT YBWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736785917; x=1737390717; 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=RBPvIovHQiD9n8xzkcacIlBFecfDAv7IoE1iz0kn/Yo=; b=KI0ODRi7iiu+mQ5yop11Wu1ob0cViSV19kKUxaAhpDQ/eps+CMMZsb3apfmda3u8N9 Em+TPRATX/2mfMMJ6nB2IsUD3LwY+JNHWm7TrMjRX8Jwy3EgxbkUUZYU2lKglIM/QNeu z6d9bQ6BhDDwqg5ATxza1wnqpnG8yO9SqILaB8+Ta+UV5Q/jiK8Lx53iGuVSlItf2o+E sY/fsyzVH0/DQkphsQ2fb45SOWaYXdrK4mb9o6pNaksG1pqIqsHfHx0AQ7q5wzvhQmBP 2m7tFjxmA8AzysHGrn/LkAe9xxMfbm/NPokMzfiK/MPBM/ySOF5xk3c1f6sMjwO2vrpt XunA== X-Forwarded-Encrypted: i=1; AJvYcCUCSuLJtuQAt+ns8wjjCsakRFlJWUlLuIYHumruyYImJD9hHlHNouVRcfrHDo/Faa3f8KRi0KQbhQ==@kvack.org X-Gm-Message-State: AOJu0YyPThTv8p54eKPDXbSKnAQRlsDHZSDLfb7N9nuYGzPHV6/5yQCY lY81HMzxT4jw5SP7qv0NlEq9yI7SgimY1JsMdnuVyDuzKp53dyXXBCXx5C/cINyekztwlJg7uH3 H8PhzJRPHtrdxyi6SkHtPqaJLVIjjzUbUkNwi X-Gm-Gg: ASbGncuXbO3C8T3LiQ0H5IZeURHlXKKbiy0wuZ0EFbjx7NNu8xQBapsX3UPcVsyI9ng SDh8mcFyTdZ68qty/1nYnSL2h6gkxj4dHJiyKdg== X-Google-Smtp-Source: AGHT+IFdQ/9E90a49DORAdfOsNHvSpnAOsbcfqHZKomsT7w6CTF3kENfUIOD+Q/U2yNdDUgYVQkGACVRv9JrAFuL6to= X-Received: by 2002:ac8:7f54:0:b0:46c:70b1:c5e4 with SMTP id d75a77b69052e-46c89d3a757mr12363361cf.3.1736785916740; Mon, 13 Jan 2025 08:31:56 -0800 (PST) MIME-Version: 1.0 References: <20250111042604.3230628-1-surenb@google.com> <20250111042604.3230628-5-surenb@google.com> <6e9329ba-8dad-423f-9741-e5447f85659f@lucifer.local> In-Reply-To: <6e9329ba-8dad-423f-9741-e5447f85659f@lucifer.local> From: Suren Baghdasaryan Date: Mon, 13 Jan 2025 08:31:45 -0800 X-Gm-Features: AbW1kvZRZB3TkQoH7dhtoCM6Tu-jcq-DdPD4z7nWFcRSAcC0CvKRy7IYHlibKrk Message-ID: Subject: Re: [PATCH v9 04/17] mm: introduce vma_iter_store_attached() to use with attached vmas To: Lorenzo Stoakes Cc: akpm@linux-foundation.org, peterz@infradead.org, willy@infradead.org, liam.howlett@oracle.com, david.laight.linux@gmail.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, lokeshgidra@google.com, minchan@google.com, jannh@google.com, shakeel.butt@linux.dev, souravpanda@google.com, pasha.tatashin@soleen.com, klarasmodin@gmail.com, richard.weiyang@gmail.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-Rspamd-Queue-Id: 15EB7C0010 X-Rspam-User: X-Rspamd-Server: rspam07 X-Stat-Signature: tw1de8d4ubk5duozo4n93kr8xtohkoro X-HE-Tag: 1736785917-936255 X-HE-Meta: U2FsdGVkX1+N2sVsETHAW2aOkiRVo0DHTrfTsp1LTT+9jmnjKByaRXb8kRuS9R3tFh/ZGPY3mZXBUMMOQ3HY202HfxrAs+n7ctm1nZgtT2JO8pqL+WF8Io50KKdzgHzA3lV+xUVGSeYcWPXHWMK55QR9/6Px5kRy8WC0t9DnVPlIOmTNV+U3fceQ9WQVXn9alH/ooqEpzMXY4OutjFjkp1/K8UwX2S9yIKg/4iJSPdURaML/IR895YQ/G8FG4iyw6KBfQ7p/cxmY4zMNjn4GmEe+QXP3CEj/mQXg7FjhfWdUkMPoBDehj1hARJzf69Ik6/C3rqMYj9FkWkcl5B6QQqJ4U3iCKnG9MEyIj4Laxe5oukslYaakC9BJVcxp9tvToE4veYmCwKBkT6BBYjjdOtVwYGiFhAIiDPy+g3qkbm4kWIfnrlxBGSez9IuIzFo1FD5Y6HyTwSlPuKENVEbm+4LqML9XosAbkqZe/TXkZwnMbvhd6+8s3DLRuA0k64mcwUt5Su4XWzlQsI7edPUY4xtlzzlxd0jYKm/fj2/gO1Li2mGoIr7GvkUO4KEnfWoWv/h078FM6esQwf9xn+MKxGA7nnB+mSh885gS/Bm2M8sb+Kp8xxyhL2nUEOleujzGkpa3p+NT1bGDpF0nppploRA7yEpkfP7Pwuz80Ro4TVjYkpkVNR/QZXd6yNEwEiR5p5aXoxf7C6Mi10okdj4fGZppMAlLmEyqgXdC6F7Lktq1VbqlgRmdZt5hIX1uO/EHvtTbkzAXwtZmPgfFointdHjKT7yjQHW3YyAByPSRqnPSbKqF5bnxymUcY7V/EDHk4UXqzbSJkyTlGeUKzEbfwmyfDc2hRlTMNs8NM0QxbUgTvBaB0adLFM9lwGw9AfS1J1W2OKZdVWRR6hPUuEuN0TXuo+l0BvHi0Uc8X0P7QZ/xYjHNj+Iuvd0jlWtmHzgi392tbBCGfPYEiLtMS5w blWHh2AB 9ZspYu4Q4MqMc5d1BgeipLTIREP5KRUYhNmoRkeBB+aYC8nTroewu63NWYOBL7XGIGEQ9Z3SmdIermSX7PObJ6PiT8qMaAaLpLXNDSgy9Uy/G62/jBFbv9K/kRpi5bKApL9J47TVfTytj65Phs7uMu+kM/Gd4n+030AskJHCbXS9hMbvmjlAcXRIK37SYGCI7agqkfZ5fjyVaE0O39ytSbFtL7bBiXT33tETloKuo8Evhu6d8sRdc3Iiba3dJR4G7t7+4l77sMA1YgEWXX/agFFJ4lm76dYNusRCmF2gvnLu97QQkparHhQZCrOFsQXRAKeyNNtwXoU9vey3zxZ0/vRQWwOI3+6Q1OVHh8LIhhuDwQDQ= 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 Mon, Jan 13, 2025 at 3:58=E2=80=AFAM Lorenzo Stoakes wrote: > > On Fri, Jan 10, 2025 at 08:25:51PM -0800, Suren Baghdasaryan wrote: > > vma_iter_store() functions can be used both when adding a new vma and > > when updating an existing one. However for existing ones we do not need > > to mark them attached as they are already marked that way. Introduce > > vma_iter_store_attached() to be used with already attached vmas. > > OK I guess the intent of this is to reinstate the previously existing > asserts, only explicitly checking those places where we attach. No, the motivation is to prevern re-attaching an already attached vma or re-detaching an already detached vma for state consistency. I guess I should amend the description to make that clear. > > I'm a little concerned that by doing this, somebody might simply invoke > this function without realising the implications. Well, in that case somebody should get an assertion. If vma_iter_store() is called against already attached vma, we get this assertion: vma_iter_store() vma_mark_attached() vma_assert_detached() If vma_iter_store_attached() is called against a detached vma, we get this = one: vma_iter_store_attached() vma_assert_attached() Does that address your concern? > > Can we have something functional like > > vma_iter_store_new() and vma_iter_store_overwrite() Ok. A bit more churn but should not be too bad. > > ? > > I don't like us just leaving vma_iter_store() quietly making an assumptio= n > that a caller doesn't necessarily realise. > > Also it's more greppable this way. > > I had a look through callers and it does seem you've snagged them all > correctly. > > > > > Signed-off-by: Suren Baghdasaryan > > Reviewed-by: Vlastimil Babka > > --- > > include/linux/mm.h | 12 ++++++++++++ > > mm/vma.c | 8 ++++---- > > mm/vma.h | 11 +++++++++-- > > 3 files changed, 25 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 2b322871da87..2f805f1a0176 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -821,6 +821,16 @@ static inline void vma_assert_locked(struct vm_are= a_struct *vma) > > vma_assert_write_locked(vma); > > } > > > > +static inline void vma_assert_attached(struct vm_area_struct *vma) > > +{ > > + VM_BUG_ON_VMA(vma->detached, vma); > > +} > > + > > +static inline void vma_assert_detached(struct vm_area_struct *vma) > > +{ > > + VM_BUG_ON_VMA(!vma->detached, vma); > > +} > > + > > static inline void vma_mark_attached(struct vm_area_struct *vma) > > { > > vma->detached =3D false; > > @@ -866,6 +876,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_assert_attached(struct vm_area_struct *vma) {} > > +static inline void vma_assert_detached(struct vm_area_struct *vma) {} > > static inline void vma_mark_attached(struct vm_area_struct *vma) {} > > static inline void vma_mark_detached(struct vm_area_struct *vma) {} > > > > diff --git a/mm/vma.c b/mm/vma.c > > index d603494e69d7..b9cf552e120c 100644 > > --- a/mm/vma.c > > +++ b/mm/vma.c > > @@ -660,14 +660,14 @@ static int commit_merge(struct vma_merge_struct *= vmg, > > vma_set_range(vmg->vma, vmg->start, vmg->end, vmg->pgoff); > > > > if (expanded) > > - vma_iter_store(vmg->vmi, vmg->vma); > > + vma_iter_store_attached(vmg->vmi, vmg->vma); > > > > if (adj_start) { > > adjust->vm_start +=3D adj_start; > > adjust->vm_pgoff +=3D PHYS_PFN(adj_start); > > if (adj_start < 0) { > > WARN_ON(expanded); > > - vma_iter_store(vmg->vmi, adjust); > > + vma_iter_store_attached(vmg->vmi, adjust); > > } > > } > > I kind of feel this whole function (that yes, I added :>) though derived > from existing logic) needs rework, as it's necessarily rather confusing. > > But hey, that's on me :) > > But this does look right... OK see this as a note-to-self... > > > > > @@ -2845,7 +2845,7 @@ int expand_upwards(struct vm_area_struct *vma, un= signed long address) > > anon_vma_interval_tree_pre_update_vma(vma= ); > > vma->vm_end =3D address; > > /* Overwrite old entry in mtree. */ > > - vma_iter_store(&vmi, vma); > > + vma_iter_store_attached(&vmi, vma); > > anon_vma_interval_tree_post_update_vma(vm= a); > > > > perf_event_mmap(vma); > > @@ -2925,7 +2925,7 @@ int expand_downwards(struct vm_area_struct *vma, = unsigned long address) > > vma->vm_start =3D address; > > vma->vm_pgoff -=3D grow; > > /* Overwrite old entry in mtree. */ > > - vma_iter_store(&vmi, vma); > > + vma_iter_store_attached(&vmi, vma); > > anon_vma_interval_tree_post_update_vma(vm= a); > > > > perf_event_mmap(vma); > > diff --git a/mm/vma.h b/mm/vma.h > > index 2a2668de8d2c..63dd38d5230c 100644 > > --- a/mm/vma.h > > +++ b/mm/vma.h > > @@ -365,9 +365,10 @@ static inline struct vm_area_struct *vma_iter_load= (struct vma_iterator *vmi) > > } > > > > /* Store a VMA with preallocated memory */ > > -static inline void vma_iter_store(struct vma_iterator *vmi, > > - struct vm_area_struct *vma) > > +static inline void vma_iter_store_attached(struct vma_iterator *vmi, > > + struct vm_area_struct *vma) > > { > > + vma_assert_attached(vma); > > > > #if defined(CONFIG_DEBUG_VM_MAPLE_TREE) > > if (MAS_WARN_ON(&vmi->mas, vmi->mas.status !=3D ma_start && > > @@ -390,7 +391,13 @@ static inline void vma_iter_store(struct vma_itera= tor *vmi, > > > > __mas_set_range(&vmi->mas, vma->vm_start, vma->vm_end - 1); > > mas_store_prealloc(&vmi->mas, vma); > > +} > > + > > +static inline void vma_iter_store(struct vma_iterator *vmi, > > + struct vm_area_struct *vma) > > +{ > > vma_mark_attached(vma); > > + vma_iter_store_attached(vmi, vma); > > } > > > > See comment at top, and we need some comments here to explain why we're > going to pains to do this. Ack. I'll amend the patch description to make that clear. > > What about mm/nommu.c? I guess these cases are always new VMAs. CONFIG_PER_VMA_LOCK depends on !CONFIG_NOMMU, so for nommu case all these attach/detach functions become NOPs. > > We probably definitely need to check this series in a nommu setup, have y= ou > done this? As I can see this breaking things. Then again I suppose you'd > have expected bots to moan by now... > > > static inline unsigned long vma_iter_addr(struct vma_iterator *vmi) > > -- > > 2.47.1.613.gc27f4b7a9f-goog > >