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 9D72BC369C2 for ; Fri, 25 Apr 2025 15:34:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DDD226B000E; Fri, 25 Apr 2025 11:34:30 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D66926B0010; Fri, 25 Apr 2025 11:34:30 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BE1B16B0011; Fri, 25 Apr 2025 11:34:30 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 9CDA36B000E for ; Fri, 25 Apr 2025 11:34:30 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 590071D04B8 for ; Fri, 25 Apr 2025 15:34:30 +0000 (UTC) X-FDA: 83372963100.20.4D3F758 Received: from mail-qt1-f180.google.com (mail-qt1-f180.google.com [209.85.160.180]) by imf01.hostedemail.com (Postfix) with ESMTP id A72DB40014 for ; Fri, 25 Apr 2025 15:34:28 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ExuLwP3R; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of surenb@google.com designates 209.85.160.180 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1745595268; a=rsa-sha256; cv=none; b=bwjhSVUW9TnQYJ9/MnCPTVuhvx5GTrkJFN+hMzmZh93yLOQbwhqvdEkwqcpZuwyZhVQ4hB zjNzwT0i6l9z5L9HgMCUB4kgC1ibrdHBdqp2tJUm1XiOdSnivnTGX+SLAM3CLqwzlhm8u3 C+8buD94hfGZEvEDDHh2NSl/h5bNKn4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ExuLwP3R; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of surenb@google.com designates 209.85.160.180 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=1745595268; 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=g5/jbEJKCj61SS9UYhrPT3xCuMZt1d5yjdn5+6M7u7I=; b=WH5tjQI/tk/Jbwrck7PFCHQycaiOagmdMTImxNLT2crlNHoCoXN4gtIfXJppyzo+SP2DnR cE2Pr1JG3BzRrhDP8Vixu7h9pRUD5wcy/dg2r/mZdvC/RhciIyCXwA0NzYi4IpeBcz5gcQ ylrPwNcBRIjKLAMJekYoq3fwJSpPUOo= Received: by mail-qt1-f180.google.com with SMTP id d75a77b69052e-47681dba807so262351cf.1 for ; Fri, 25 Apr 2025 08:34:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1745595268; x=1746200068; darn=kvack.org; 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=g5/jbEJKCj61SS9UYhrPT3xCuMZt1d5yjdn5+6M7u7I=; b=ExuLwP3R06OvvFvUz3iAPHhdnXf3cCHRkvKecqq1j13yGiYJn1YajIQjwy/FDAJprp Ck1FneYt2NB1AAgq+TKaPgyHqiswltLQ3cE8KFS2b0c5Kx3X9XGZe7beWlwtZTrDflrD JJcGkBZ2EiKaTzMoiZnUMfYWXR1uwF2jGey5fq+ZR8+1+jLKQo+q9qvYQA9o1XvBKJCY CVqQd4gv5Jt58AktG6H8Pnirfx5hfiHYElkb+FW9ytpUQ/d4jOy4zg9Zb4b69ZEj/VDc O+7xvERPASb5LrK9rpfUDJC/QqhazP2ru/nn4uD+/TGIf3bsQFgddkW0eN1voLkI1vpr lFog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1745595268; x=1746200068; 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=g5/jbEJKCj61SS9UYhrPT3xCuMZt1d5yjdn5+6M7u7I=; b=RUgkocqE67hW1ZpeUabLiTXAPUA5rGbBmOGSM8CJ4SRrVjyaVy3zaK8/srr76BPKql jUVnDNx8yzCpD/33lwB73J1kFkhONhrRMwlbMzi8S8GZUPhlu329u7x2LbnMHgn1CbG2 7iPBfJVnIxuw542/Bblp6RUecbArW0YidCAI4Y+Ix1caghqwTzpUgha4JmcjwRF8CftL uESeUSNDyPUunbt501tam7+gEF7wRVdqQG3FJUeolhjiMue/cZ1l0YyUIvYs/D/dv+td WYSH+4Q7PAbrIGTXRNqfujxa3uruox10XzfYUmZCFYCAZlEmriWDPTH5nw0D6MgIAMM5 x7zw== X-Forwarded-Encrypted: i=1; AJvYcCUL2YqV8if3++BGJBRxsF0pNaI6PrQdifMTusBs0026ZZCPD3rQEq110qF36lcgrSjHU1JsT0HdEQ==@kvack.org X-Gm-Message-State: AOJu0YxrDc7oiDnSn4YAjIhUclZfkPPjScaZc6W4L2ypoO0cAP3+/qlL 6PLrVEvpuqsMT4mrJoR98NIty04Uc524TX5g19yM0RKEnzIZbzBuleRQkGEVcfIEYAw4O1+HdqC CICYp8tbW0atbI0xEs8+gCbvgaufAsOIrb0bY X-Gm-Gg: ASbGncvCjVb1mTdMBg3PaoqqyaeghCtN9fcEYCX8tc9DxbkzWUAg8D7h8V84mPrdvkH cBbH4wcRT12QmCaJQAR9ikQeA1EjWjnSMMkyZPSioinZSv0xYO5dQIRBwtXl/zEL5oANZBVtBkH sBrDBAF6kqBJqm/MR2QHL1 X-Google-Smtp-Source: AGHT+IFaLY3s0izE7nmgvYKPlu8fJRpk84aR32fIsczDTeevwRZvaPztL4MnBjYx9gEAYjD/NulcuZ/Xj53z+YfAbTw= X-Received: by 2002:a05:622a:230d:b0:475:1410:2ca3 with SMTP id d75a77b69052e-4800d34c448mr3287071cf.15.1745595267360; Fri, 25 Apr 2025 08:34:27 -0700 (PDT) MIME-Version: 1.0 References: <0f848d59f3eea3dd0c0cdc3920644222c40cffe6.1745528282.git.lorenzo.stoakes@oracle.com> <51903B43-2BFC-4BA6-9D74-63F79CF890B7@kernel.org> <7212f5f4-f12b-4b94-834f-b392601360a3@lucifer.local> In-Reply-To: From: Suren Baghdasaryan Date: Fri, 25 Apr 2025 08:34:16 -0700 X-Gm-Features: ATxdqUFzDiR6aSNEgXYvKyPF3vMF5nNp5WgPe7-q3Zd9yW0scIDZAX7G0iFsnAY Message-ID: Subject: Re: [PATCH 2/4] mm: perform VMA allocation, freeing, duplication in mm To: "Liam R. Howlett" , Lorenzo Stoakes , Kees Cook , Andrew Morton , Vlastimil Babka , Jann Horn , Pedro Falcato , David Hildenbrand , Alexander Viro , Christian Brauner , Jan Kara , Suren Baghdasaryan , linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: A72DB40014 X-Stat-Signature: irn85mfgbx4h91bmhsca9x9btjuhmygu X-Rspam-User: X-HE-Tag: 1745595268-884621 X-HE-Meta: U2FsdGVkX18IHgfVwvJZ69njOW8gwE3HRtwTQeufq0n8/o0gOsdmnYzLF+k2UXeFR0kXkZ+pYtoilcraw4TB2AJ/iZe8w/9Adgf8nZHFLgQ+AeRyUj96+0DVOFIjjgv7zZum05TMEMmb/Sadlv+KmnI0P/6Rp25I7LD2VOc2x5NN+J5tuusmQXsDngq7DiNGprN5FObnSspnO6337lTsREDtSp8K0v+pLFE0WT/1dtlLoKaZWNjLmsjC9y63aYtiinzGeF2/hT2+lHCD9EOsjDsyGYVTT9DxiNHCeVsjefG7XM5AW3UJYjhq4r+3dn0Ca+cAJo2IaNUWrr3HpfMR3sgeeg6wvJNBQEFqMPEZoQFDjc3726Xi3NfSgml3hjaAgvF+z/uPi1qM6BKgIATqy8UOt1FuOMs9XGLNGCfbcdRHhxWf+TZUhJYGcI1xPCCuHxlGAVGFWeS+6SOqXzl3CQq+jK1rkiJf6Ss1j4czXnjtiRLWnTlURlCEwsbdCDCQiS/mKNK2BSchZ4zsxqmIsl6HYb430VAUyW9NBjZW5So9h9x3q1HZT1FYbcLVcRsXX2EqFdtFuSinSBfJwuErJiCGGIwxjmu4Tnf4IdxVyBD6NAIz5MlRKLgyNkk9n8MbQSgoPEULZdR99Si9C26bQFPYNwpPk0vWINHbxtplnWRF9X39cVXljjoXF/ZflkaKA2p3z6yVkQxS2SUfHkKD0WVx7KRAxlY7DkLQBOiIkQmt2YAf6Yd/WPrQRPM2b8MkJW/58j6Kxlrq9rQ01YBjiXEbWTOkHv1yYiK+pFMWWSW9LSfVdBNlMUj4Tud+LmLmCv26UOj1YnXGpOkoPfgvbq74nQ7PgS+5qLnuAW3m8G4TNNB0ptwQAkKH8zZXEdkcmLJI9f9+ACxQqp2+wRM/6Lvh5wWKpPAR 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, Apr 25, 2025 at 8:32=E2=80=AFAM Suren Baghdasaryan wrote: > > On Fri, Apr 25, 2025 at 6:55=E2=80=AFAM Liam R. Howlett wrote: > > > > * Lorenzo Stoakes [250425 06:40]: > > > On Thu, Apr 24, 2025 at 08:15:26PM -0700, Kees Cook wrote: > > > > > > > > > > > > On April 24, 2025 2:15:27 PM PDT, Lorenzo Stoakes wrote: > > > > >+static void vm_area_init_from(const struct vm_area_struct *src, > > > > >+ struct vm_area_struct *dest) > > > > >+{ > > > > >+ dest->vm_mm =3D src->vm_mm; > > > > >+ dest->vm_ops =3D src->vm_ops; > > > > >+ dest->vm_start =3D src->vm_start; > > > > >+ dest->vm_end =3D src->vm_end; > > > > >+ dest->anon_vma =3D src->anon_vma; > > > > >+ dest->vm_pgoff =3D src->vm_pgoff; > > > > >+ dest->vm_file =3D src->vm_file; > > > > >+ dest->vm_private_data =3D src->vm_private_data; > > > > >+ vm_flags_init(dest, src->vm_flags); > > > > >+ memcpy(&dest->vm_page_prot, &src->vm_page_prot, > > > > >+ sizeof(dest->vm_page_prot)); > > > > >+ /* > > > > >+ * src->shared.rb may be modified concurrently when called from > > > > >+ * dup_mmap(), but the clone will reinitialize it. > > > > >+ */ > > > > >+ data_race(memcpy(&dest->shared, &src->shared, sizeof(dest->shar= ed))); > > > > >+ memcpy(&dest->vm_userfaultfd_ctx, &src->vm_userfaultfd_ctx, > > > > >+ sizeof(dest->vm_userfaultfd_ctx)); > > > > >+#ifdef CONFIG_ANON_VMA_NAME > > > > >+ dest->anon_name =3D src->anon_name; > > > > >+#endif > > > > >+#ifdef CONFIG_SWAP > > > > >+ memcpy(&dest->swap_readahead_info, &src->swap_readahead_info, > > > > >+ sizeof(dest->swap_readahead_info)); > > > > >+#endif > > > > >+#ifdef CONFIG_NUMA > > > > >+ dest->vm_policy =3D src->vm_policy; > > > > >+#endif > > > > >+} > > > > > > > > I know you're doing a big cut/paste here, but why in the world is t= his function written this way? Why not just: > > > > > > > > *dest =3D *src; > > > > > > > > And then do any one-off cleanups? > > > > > > Yup I find it odd, and error prone to be honest. We'll end up with un= initialised > > > state for some fields if we miss them here, seems unwise... > > > > > > Presumably for performance? > > > > > > This is, as you say, me simply propagating what exists, but I do wond= er. > > > > Two things come to mind: > > > > 1. How ctors are done. (v3 of Suren's RCU safe patch series, willy mad= e > > a comment.. I think) > > > > 2. Some race that Vlastimil came up with the copy and the RCU safeness. > > IIRC it had to do with the ordering of the setting of things? > > > > Also, looking at it again... > > > > How is it safe to do dest->anon_name =3D src->anon_name? Isn't that re= f > > counted? > > dest->anon_name =3D src->anon_name is fine here because right after > vm_area_init_from() we call dup_anon_vma_name() which will bump up the > refcount. I don't recall why this is done this way but now looking at > it I wonder if I could call dup_anon_vma_name() directly instead of > this assignment. Might be just an overlooked legacy from the time we > memcpy'd the entire structure. I'll need to double-check. > > > > > Pretty sure it's okay, but Suren would know for sure on all of this. > > > > Suren, maybe you could send a patch with comments on this stuff? > > Yeah, I think I need to add some comments in this code for > clarification. We do not copy the entire vm_area_struct because we > have to preserve vma->vm_refcnt field of the dest vma. Since these > structures are allocated from a cache with SLAB_TYPESAFE_BY_RCU, > another thread might be concurrently checking the state of the dest > object by reading dest->vm_refcnt. Therefore it's important here not > to override the vm_refcnt. Changelog in > https://lore.kernel.org/all/20250213224655.1680278-18-surenb@google.com/ > touches on it but a comment in the code would be indeed helpful. Will > add it but will wait for Lorenzo's refactoring to land into linux-mm s/linux-mm/mm-unstable. I need my morning coffee. > first to avoid adding merge conflicts. > > > > > Thanks, > > Liam