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 A2666EB64DD for ; Thu, 3 Aug 2023 18:27:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2BE4E28028F; Thu, 3 Aug 2023 14:27:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 26E6C28022C; Thu, 3 Aug 2023 14:27:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1363B28028F; Thu, 3 Aug 2023 14:27:00 -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 0429728022C for ; Thu, 3 Aug 2023 14:27:00 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id AE7B0C12F6 for ; Thu, 3 Aug 2023 18:26:59 +0000 (UTC) X-FDA: 81083624958.29.671FFA4 Received: from mail-yb1-f179.google.com (mail-yb1-f179.google.com [209.85.219.179]) by imf19.hostedemail.com (Postfix) with ESMTP id E0E131A000E for ; Thu, 3 Aug 2023 18:26:57 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=1dpeZTUn; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of surenb@google.com designates 209.85.219.179 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=1691087217; 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=LJY6KKzyPwphhQ98P/1zZEUfDdFiMUyJJPp/8Vkg4G4=; b=dMM5aYnaaAhyzjirwGVqlDuAwIYdz2Oa7AR+0J/ab1Ahaeb4Aa3910zIoVZvsgczftU1WW wUFFCuf9UjfEHInIBY/DsWpe6NfF0QFBTyYiUsZsLaI4GVUm8uSeOyo0fcx8ACYrdqVoeI DzhtacZVJPiLOgVkb8PKLO60hr3j4bA= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=1dpeZTUn; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf19.hostedemail.com: domain of surenb@google.com designates 209.85.219.179 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691087217; a=rsa-sha256; cv=none; b=7s/zF5dimtlzjuAIxUr8GkKY1H8cV5AXhS8Vt8AERcVYj+X0U4+JkVL3MZaIvlemq9mNT6 XheiYNTbzf3m8SbxPmb2Tn6GPxAGuvhY4jW+hjUNlS4/YgsnTAefGyhl3jApxGgDi5dtF/ 9JiwiEFNwZfyNsy7i5wWJ/0CvisEn+4= Received: by mail-yb1-f179.google.com with SMTP id 3f1490d57ef6-d2b8437d825so1342683276.3 for ; Thu, 03 Aug 2023 11:26:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691087217; x=1691692017; 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=LJY6KKzyPwphhQ98P/1zZEUfDdFiMUyJJPp/8Vkg4G4=; b=1dpeZTUnhOGV3DZJjOZDR3Hqj6ownpypLiqKU+MHOfvcc8XTZG71vJuCdY2CMpyfTI xnU0i8rCzOd9HFVgbgk0+zMKh5Z+mhSCZBMpmU6WOtFnahrIP9Wz2rUfI7jVfPomPrZi VO+J+aVB51uZa0vL3OJfxB908uxB5bdKteoHHi4qKj/MEwexITEwQPdjIy5czq9rJeyi htXb/vtJSJQ16PsQp5y2B+lxwmxdn3K1NMp5yYBy4VqRVCtyyUVEnCMBOQ0ZKRdU8KaD SqdKhVe3Q+O/5vF/Uf/UbwNOefjkDeEKVYXv5nSscqOW0DroIEXoHyGmWeor0VpkuDWD goEQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691087217; x=1691692017; 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=LJY6KKzyPwphhQ98P/1zZEUfDdFiMUyJJPp/8Vkg4G4=; b=DX39uqc2kRstzuv1patdVLjn9jXFEP6sE8d3larz8PFagHPPm2WtphLmiUKmK63qlY jLHLW2P0rv9IwK26NvoYkE3nkuwEypWi2kPXQzcIZrCtOWmOjyuqJ/13FZmdA7JcBeLO tM38k1XKgT23PuhmKNBbGkfo1CTUi2cd00TjA1k2ERGAtdj+Zhi/qn25TKD7+jLyqvmT eHGmIZcFkfuzGpDHNa4Y8RrsztuaAm+qXE7O9pS+VG6V1n4qrVKY7sDUOjj21TwS+v3W L4/Gvfzi8HbPRRf90oIA/XGguQyyuJdLMD8Qe7LHEueD7I9seGN1aWYxqjw3ZCQIrU7W C3DA== X-Gm-Message-State: ABy/qLb3+HJZDFbX0LHUYKMkr+3X7PkdkERWw+Mo3KcNcZ76EXNBPW0E BN6Rahkx0HWLTuumNPLUdelUu7g+5wNhBZu1SmowvQ== X-Google-Smtp-Source: APBJJlEJ/xQ6/X6lCi6Xal+GKcDNnYyzTSSA5nVnsb2xznOb20eo88GXdM78peIHKOQPxZrl7saNIhLRp6CaJzdXdww= X-Received: by 2002:a25:d1d8:0:b0:ce8:e6f:4408 with SMTP id i207-20020a25d1d8000000b00ce80e6f4408mr18331813ybg.7.1691087216765; Thu, 03 Aug 2023 11:26:56 -0700 (PDT) MIME-Version: 1.0 References: <20230803172652.2849981-1-surenb@google.com> <20230803172652.2849981-6-surenb@google.com> <20230803181520.yd5ao45rm3rxnsbs@revolver> In-Reply-To: <20230803181520.yd5ao45rm3rxnsbs@revolver> From: Suren Baghdasaryan Date: Thu, 3 Aug 2023 11:26:43 -0700 Message-ID: Subject: Re: [PATCH v3 5/6] mm: always lock new vma before inserting into vma tree To: "Liam R. Howlett" , Linus Torvalds , Suren Baghdasaryan , akpm@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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: E0E131A000E X-Stat-Signature: r5un84ajx6jrdecwwexrhuzkz3o4yaw8 X-HE-Tag: 1691087217-28146 X-HE-Meta: U2FsdGVkX1+lWCiKCkHLbA0lDbTE0JAtFli1/8Ws33tagUAABIMY98bbm1NTjm8E/FosFK93IHQ0tfm3T4w1vkjf6YoJowIhtgV6AlhKjlc840iXxQ/hnahx56YKzR7TlqHZxkRvp3M9QMLcpCgUgm4OoS3vKI6p2Rz6/9xozVlTAE3HWsCkDfk4OqGS/qHlgsOOrzZ+g/hP03AvBxXXHDYPjsYdgASDK4SksGMztc6iG9j2hn2ZnrWy+uz4SEXsQ7/R/JVbkEwX4xPNSyvIpoZxKrY9ozh683BolX5OCK+SNMVrrz9D10i6BHtgIYFk92eIaptfcamLdymYFr/jJzq1/2vPioHt3U3uhEPbhp0KKOpUjpHpXUplXMAAwsOBCPFRwk/8706GDuj/TRVaxV5rHFKa1ouXX2lw/Ndtjh+DGzG2ImZ50xdO0qob3zZVxfhPN/Go93Nvgi+SkJkVipTGhl5+LwT+g2/SvyP9sAGR4A5ADSXv3tsT7DDrossSov1/XUQIRvQsFXgBMFckPryZXpwxhGm+onZmnMiNuf38lnlgAxTsmnihhGgXlEntvw04JpMXY4Zb8ZBqnZbkto4iYOE6NamTtQSC7wttirWnsQHcqxizmre94hhV85rbKPHlhv38LHqw/jlVaJ+UHDoGeMzaw2QELF218AnH8g1HrYfKWoRqDtEjJOOGP5xXomJZOSZFu2TVLsimdkgtSw5I6/oqYMg14D9JgTpWD/0Nn1inqwt9mrUp94Cnua20YBSo2vhT49iU0gABJcFqGTqzIxBhoRtD/tMMLkpLavcFggtWrAktwRXb1yRCMM65kHXtcDmp6fEIbXMseriIBXdYBURRed2lH18UZxeeqHs6V0JvF8mxfeXUsrjXa9zkXzQPI/Txskwsvoh19ERqq+JjzKlGghcb23Bmgj2TloASxx1CyoeNu8bsFWpILSEFjEkFbcx9F2JGMRPMzlO gY6GzFUc Du+9NSSmD/A0a5kbIPdGmUwLof997xUSdhKTaEutqL6oj6Lp6+ElpBycwS4J2T4bs1eGNDeRpMfbkyAsCF+rBUz6RBZGC3OhHAdVCAZoba16IQCNWUgN6v8kym3KfH7BXPb+DkbcMgBJv7jfpmBFdlbiBnTa8WZhbf2FDGIqnNyln1T8TNbN/l6iF0tG7wRWZem4FMbGz4nRjurhT8moJ42PBWfH3M8XBLoT1sfY/DilABQ5O690bamLNkBx4Cx770FAH8krEXqUS+IasZ1dzNoeZ6IxAgP2iJGbSTktI/9bhakQpf4lZnCDLeS5MJ74rvzfNU0YuDm7YEws= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000021, 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:15=E2=80=AFAM Liam R. Howlett wrote: > > * Linus Torvalds [230803 14:02]: > > On Thu, 3 Aug 2023 at 10:27, Suren Baghdasaryan wro= te: > > > > > > While it's not strictly necessary to lock a newly created vma before > > > adding it into the vma tree (as long as no further changes are perfor= med > > > to it), it seems like a good policy to lock it and prevent accidental > > > changes after it becomes visible to the page faults. Lock the vma bef= ore > > > adding it into the vma tree. > > > > So my main reaction here is that I started to wonder about the vma allo= cation. > > > > Why doesn't vma_init() do something like > > > > mmap_assert_write_locked(mm); > > vma->vm_lock_seq =3D mm->mm_lock_seq; > > > > and instead we seem to expect vma_lock_alloc() to do this (and do it > > very badly indeed). > > > > Strange. > > > > Anyway, this observation was just a reaction to that "not strictly > > necessary to lock a newly created vma" part of the commentary. I feel > > like we could/should just make sure that all newly created vma's are > > always simply created write-locked. > > > > I thought the same thing initially, but Suren pointed out that it's not > necessary to hold the vma lock to allocate a vma object. And it seems > there is at least one user (arch/ia64/mm/init.c) which does allocate > outside the lock during ia64_init_addr_space(), which is fine but I'm > not sure it gains much to do it this way - the insert needs to take the > lock anyways and it is hardly going to be contended. Yeah, I remember discussing that. At the time of VMA creation the mmap_lock might not be write-locked, so mmap_assert_write_locked() would trigger and mm->mm_lock_seq is not stable. Maybe we can necessitate holding mmap_lock at the time of VMA creation but that sounds like an unnecessary restriction. IIRC some drivers also create vm_are_structs without holding mmap_lock... I'll double-check. > > Anywhere else besides an address space setup would probably introduce a > race. > > Thanks, > Liam >