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 04CF9C001DF for ; Thu, 3 Aug 2023 18:34:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 70937280292; Thu, 3 Aug 2023 14:34:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6B99E28022C; Thu, 3 Aug 2023 14:34:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5815B280292; Thu, 3 Aug 2023 14:34:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 4553628022C for ; Thu, 3 Aug 2023 14:34:53 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 04D56A0640 for ; Thu, 3 Aug 2023 18:34:52 +0000 (UTC) X-FDA: 81083644866.20.589AB80 Received: from mail-yb1-f181.google.com (mail-yb1-f181.google.com [209.85.219.181]) by imf18.hostedemail.com (Postfix) with ESMTP id 246BA1C001C for ; Thu, 3 Aug 2023 18:34:50 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=j4ZZTUw3; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of surenb@google.com designates 209.85.219.181 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=1691087691; 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=Nyc2gitI2vs2BzBPE2gM4dSf776t2Bqkap6SAX9jGgw=; b=DqFcdqdur9ISwSX6oAPmX872R03DNkNo6UKWngOMqPMyhDBNRw2ZXUVFSeoddALjl5tnlL WJDxKfL9+1W/nZInU4bR1KAdtxvLXQETmrR83l/12CaqLNGk9Spn65bmyxGNZA+2utdPJe 0ohSBPs9L812EqK3OEXhSQal2hOGsog= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=j4ZZTUw3; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of surenb@google.com designates 209.85.219.181 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691087691; a=rsa-sha256; cv=none; b=Cmiz3yABIg9KQKJd8dgcjwxEB7H+SwfC6N1XP43/hPThQZGPfuuBFjLG0oqm7qfYdOALjE scu+Mbp4xIZWnNkRuI1kiuGruNOIpkjpbVtUyNUlZtIJj5Ug18EvGV3SzIj4V2W498ncNx 8idopyx5f1e7mz/6PYN97sal8//hAGY= Received: by mail-yb1-f181.google.com with SMTP id 3f1490d57ef6-d0e009433c4so1479000276.2 for ; Thu, 03 Aug 2023 11:34:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691087690; x=1691692490; 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=Nyc2gitI2vs2BzBPE2gM4dSf776t2Bqkap6SAX9jGgw=; b=j4ZZTUw3L8tzyfQ60uIIZQLmCPW7V3COQ5bMr151BNflpDbwZu3VUq++3RPLvFn0Vp cVdh6ZtsWZ5DvHoKm6S6btw6qLGyR5o/SsrLEColtzeo3VgcsyKK8EKqoA4MnRul4ruR MRrzpORy5/0mmVYDFe3O4DIfyYHSNc24+AKM1AH5J/jtnYcmHeu5KYeOTtLpJCddfAqr VvBVU7NFqgStEd38t6lIOJsIVydQzvdqsEA0wgQ96LZNS8pK/DKqipMT2ocQthJR/dqr 42hq+jAhgmkW/hMi7BukbXPppKS55KTMteXYUlRCdccI8m6cTu7cSMTsWe0hYL9nn+yv D5LA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691087690; x=1691692490; 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=Nyc2gitI2vs2BzBPE2gM4dSf776t2Bqkap6SAX9jGgw=; b=hfpHjpKQQSind6a13d/Nm2Lhjkcofy+Qhu1go15EHqP/XtRmiHE4hFTg/WstGYNtrr VlNJQcEGtkqa28OAuNlNL+/tQk0wtGR+a/VuwFRux8UMHrUFFaz+u2xWvLex5gOh7DHB mB6vGvSfM3H1g9Qvy5aSa7i1/jNdm043yTg7202M6PMBp8XjgGCL/fbEz27ESWTCXjtY b2tRQ08OETE5NRA42kFm/3D4CQVoqq/36yXHx99CX09CXvyjumkmhKHuxg9HQu7Ulh3Q 5XS69TouaTgcxRAFuH2Ju/DFpWFKdolr3GH69LdmMym1mHOdFBjBzCpSJVGIyYwvNDYG gkyg== X-Gm-Message-State: ABy/qLbHNJQcdOG60yEdV9+pgv5XQrfVSEaYaq8FAEe53iS2YPURgzGn QJbCUHgWDAXhmc7fmVddcDabE182tjgjuwaAg10eFg== X-Google-Smtp-Source: APBJJlEG7Mv/Fp2FsNqC1nmgVx/mt5hF/Jf12KZxbp1VyjIkLYpf4NX3BUvGR61e0lC3QkL9nAthftAM244WYxSlgc4= X-Received: by 2002:a25:24d:0:b0:c60:982f:680c with SMTP id 74-20020a25024d000000b00c60982f680cmr18200156ybc.63.1691087689991; Thu, 03 Aug 2023 11:34:49 -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: From: Suren Baghdasaryan Date: Thu, 3 Aug 2023 11:34:38 -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-Rspamd-Queue-Id: 246BA1C001C X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: ihxx53ah1h4xmqccwx3h1heeasbsxkt3 X-HE-Tag: 1691087690-180395 X-HE-Meta: U2FsdGVkX1+2egEgk3Khggy18/kyaRFBqWx1ZshQmSnB/79gdn/YNV6mIMKXN7CJuuSWT594kuOE+9ZkFgL+WMGCSB/Ujne3UrFadqIoM3d9QL6VbnSVvrTid2u7yc1cq8mBW068sjzZJIJQt6SzyPrEu4q8SRqwrAeUnVtLfPnJV1mgGs8DHJSf95rbfPBzwQIyJuLBnH1TZiddol9vuBXyi0wlObenE44vm5W59PN//IXxuOmBq22u8krhG7hew9jfdLXtmin7Dwc/oSp/c0ZtRLL4B9bt1w8+Rt/dz7Z0DHX4z/SnTTJOqFD1+bK2VB5Lwnj0fxSdiNYY0MDOoQf+LabZblx3nvyeR3settisvM4WMwXp+bJFA8qHSwwDalBpH441mYl2Kfnqlx+7kLyPvVGhpbeEQ//0ZJ6da58fYTnnSELQaPFYPhdDK4agJNCE5SXZwrtTBsQUGW9gGQnqbNUDsfu/su7g1qp+ya2XPyjqA7tGjIs9YIClF2MwPRVeOxG5+P7tDT66BIoMM9HeoeggKmWCwHRP/cyPN0ZKKVhLj/Rfibb+DNU2gDxOGQtqeavHQs3F3FEAibh5cfq5e1O7RJiITByQxw5a4/TwTrjFyFlmkYIh6HSN4DZ+peGWwiXumkdFhRJBy/paHwqePqjRS0BlvkcCWrte8qW0kFbze0ipRA/2XWxTkdxUOouoSYxH+gklEfTYBhHcZ9ktAlfz7wyAaPkn1H2xDWWwC4M4ytZCbQkzv4RjnNMVl0tO4yidGo4DzYlbLvQVGAlkB4H2Tty8Y0udkVoKoaSOp7Yv58jYGRe19YEuprkJF+7gUHHtt+Y0+FsAZjbFu8jbgjj35YBJViWws6J2wRC8NqZqq8c+mFSL3A8W6XM8czUB7J+MGsZVWt9VZOWhHzbvYu5WnzBvJxiaKL6iBMKs33FHbFKulrSRkWWXgwilyxRMw4q5bUVldlER+t1 j5kDsRFT dWDBLeuwzr4AHRlPtKdbEuGTn5XqV9uK0rFF+VmkTaF6uFLGLxu61uWs7D5B4G6vlJc88/ZDINzx5RybScuBcTL4M6J0Y2OvZtNAIN/hJWKwlIOnu5ApFKjRmXaOx9fBkRLVSM2zQk5+CMqS0/UWCtwbSEFHKHwuDGhGsqNX7kgv+4OfVgzNTC7iSzR0HZC4zX1wzS0U3awKoh1AQYuTmLimnbSLVqZJiG/l5OY3ixj8cQqpYu9r5JonRrM8Q/TkA4BDD7aAtrIlPzieUl6J1dHZQTMrOfcuVEQFWeSvwbkqnL0//S29+a+JrNxA+h6uBEn/p4NTTdDrqEHM= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000520, 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:26=E2=80=AFAM Suren Baghdasaryan wrote: > > 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 w= rote: > > > > > > > > While it's not strictly necessary to lock a newly created vma befor= e > > > > adding it into the vma tree (as long as no further changes are perf= ormed > > > > to it), it seems like a good policy to lock it and prevent accident= al > > > > changes after it becomes visible to the page faults. Lock the vma b= efore > > > > adding it into the vma tree. > > > > > > So my main reaction here is that I started to wonder about the vma al= location. > > > > > > 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. Yeah, there are places like an initcall gate_vma_init() which call vma_init(). I don't think these are called with a locked mmap_lock. > > > > > Anywhere else besides an address space setup would probably introduce a > > race. > > > > Thanks, > > Liam > >