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 B0AB8C433EF for ; Wed, 8 Dec 2021 19:22:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 079F66B0071; Wed, 8 Dec 2021 14:22:36 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 02A006B0073; Wed, 8 Dec 2021 14:22:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E35206B0074; Wed, 8 Dec 2021 14:22:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0169.hostedemail.com [216.40.44.169]) by kanga.kvack.org (Postfix) with ESMTP id D3D846B0071 for ; Wed, 8 Dec 2021 14:22:35 -0500 (EST) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 8F9858248076 for ; Wed, 8 Dec 2021 19:22:25 +0000 (UTC) X-FDA: 78895598250.17.9A587AF Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf01.hostedemail.com (Postfix) with ESMTP id D9F6840003 for ; Wed, 8 Dec 2021 19:22:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=BaY9YTruJBe+bh4x73v02WYez6jnp/vPQ8R+2at5oXY=; b=v0rhAGbRra0+8fclVnnMARYBr2 lf3Z/kkFdQZXsQxhBeNguUCB1HvYL23DZ4X7O8p76K8jIYu5Gfkt3Ep5WB/6nfrFImhc41xK/3j3H rpl3tfgVMT56nvgmFbHTM7Xs8Mkwi4caqra+ID2c2AvLA8EIEbUgUll432NXEZMxTlQfkQ+kEaRRJ Ws4ZIt/zkh5mw9uzhyOB9XsORMSM00XAToWgJPMvyLxWmc+x+kWRGiX+1QmMidRrKTRPyg0FrHHr3 SYxdAIialxlDm+FH+CHMv2134Tmqfpr8DSCiTRabA4Ath6MQuNIlx5UbTgUH9mKZZ3Aa21eYO/jZe gAyQwSxg==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1mv2Vy-008hsc-2A; Wed, 08 Dec 2021 19:22:06 +0000 Date: Wed, 8 Dec 2021 19:22:06 +0000 From: Matthew Wilcox To: Suren Baghdasaryan Cc: Michal Hocko , akpm@linux-foundation.org, rientjes@google.com, hannes@cmpxchg.org, guro@fb.com, riel@surriel.com, minchan@kernel.org, kirill@shutemov.name, aarcange@redhat.com, christian@brauner.io, hch@infradead.org, oleg@redhat.com, david@redhat.com, jannh@google.com, shakeelb@google.com, luto@kernel.org, christian.brauner@ubuntu.com, fweimer@redhat.com, jengelh@inai.de, timmurray@google.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Subject: Re: [PATCH v3 1/2] mm: protect free_pgtables with mmap_lock write lock in exit_mmap Message-ID: References: <20211207215031.2251719-1-surenb@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: D9F6840003 X-Stat-Signature: 8e64qzas9qxfscfhkz7ndjx6gpa3gguc Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=v0rhAGbR; spf=none (imf01.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none X-HE-Tag: 1638991344-437081 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: On Wed, Dec 08, 2021 at 11:13:42AM -0800, Suren Baghdasaryan wrote: > On Wed, Dec 8, 2021 at 8:50 AM Suren Baghdasaryan wrote: > > > > On Wed, Dec 8, 2021 at 8:05 AM Matthew Wilcox wrote: > > > > > > On Wed, Dec 08, 2021 at 04:51:58PM +0100, Michal Hocko wrote: > > > > On Wed 08-12-21 15:01:24, Matthew Wilcox wrote: > > > > > On Tue, Dec 07, 2021 at 03:08:19PM -0800, Suren Baghdasaryan wrote: > > > > > > > > /** > > > > > > > > * @close: Called when the VMA is being removed from the MM. > > > > > > > > * Context: Caller holds mmap_lock. > > > > > > > > > > > > BTW, is the caller always required to hold mmap_lock for write or it > > > > > > *might* hold it? > > > > > > > > > > __do_munmap() might hold it for read, thanks to: > > > > > > > > > > if (downgrade) > > > > > mmap_write_downgrade(mm); > > > > > > > > > > Should probably say: > > > > > > > > > > * Context: User context. May sleep. Caller holds mmap_lock. > > > > > > > > > > I don't think we should burden the implementor of the vm_ops with the > > > > > knowledge that the VM chooses to not hold the mmap_lock under certain > > > > > circumstances when it doesn't matter whether it's holding the mmap_lock > > > > > or not. > > > > > > > > If we document it like that some code might depend on that lock to be > > > > held. I think we only want to document that the holder itself is not > > > > allowed to take mmap sem or a depending lock. > > > > > > The only place where we're not currently holding the mmap_lock is at > > > task exit, where the mmap_lock is effectively held because nobody else > > > can modify the task's mm. Besides, Suren is changing that in this patch > > > series anyway, so it will be always true. > > > > Ok, I'll make it a separate patch after the patch that changes > > exit_mmap and this statement will become always true. Sounds > > reasonable? > > Actually, while today vma_ops->close is called with mmap_lock held, I > believe we want this comment to reflect the restrictions on the > callback itself, not on the user. IOW, we want to say that the > callback should not take mmap_lock while the caller might or might not > hold it. If so, I think *might* would make more sense here, like this: > > * Context: User context. May sleep. Caller might hold mmap_lock. > > WDYT? We're documenting the contract between the caller and the callee. That implies responsibilities on both sides. For example, we're placing requirements on the caller that they're not going to tear down the VMA in interrupt context. So I preferred what previous-Suren said to current-Suren, "this statement will become always true".