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 A2613CF042B for ; Wed, 9 Oct 2024 01:47:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1EB116B00D5; Tue, 8 Oct 2024 21:47:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 172E66B00D7; Tue, 8 Oct 2024 21:47:14 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 063296B00D8; Tue, 8 Oct 2024 21:47:13 -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 D4EEB6B00D5 for ; Tue, 8 Oct 2024 21:47:13 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 0D935A0E59 for ; Wed, 9 Oct 2024 01:47:10 +0000 (UTC) X-FDA: 82652375946.02.5CF020C Received: from mail-yw1-f175.google.com (mail-yw1-f175.google.com [209.85.128.175]) by imf25.hostedemail.com (Postfix) with ESMTP id 8FB65A000E for ; Wed, 9 Oct 2024 01:47:11 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=bZL1wqWB; spf=pass (imf25.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.128.175 as permitted sender) smtp.mailfrom=joel@joelfernandes.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1728438251; 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=VBm0hfq2DcbVBzfR35/h5cOzEtSAYZKed8pK56bnJXg=; b=v41m+a/kgRLXrIEX6e35V82V+B6ZQR0+ermEYnCq1Dqv7oBvbS9araDZJJP/Wctm/wqbg4 aykqaV3xush+gypTab+TcTvr3VMm7I/osz9WNRhqUwRtKj3VW6MUJ2VmoMF1x02jxlQIz1 t23LrZ7nmTnTV31UzY9ZUxyWpnu8+Zg= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=joelfernandes.org header.s=google header.b=bZL1wqWB; spf=pass (imf25.hostedemail.com: domain of joel@joelfernandes.org designates 209.85.128.175 as permitted sender) smtp.mailfrom=joel@joelfernandes.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1728438251; a=rsa-sha256; cv=none; b=PoNxQMb33JYjo0Cx33f1inzYJr9ZqCC6zdxyH05sQ6qjIKy3Az3RpTzbQvmBESOa5t9JK2 BzMJ4AtsrDt8gPTYoDX2M/My2DdnbmIbGKEGY1TFSg9Vijdaf45u1DzjhjTWogp1t1nBxh L4MMP8ICq8XvIJvzmAMzuTumDqWB2t8= Received: by mail-yw1-f175.google.com with SMTP id 00721157ae682-6e232e260c2so53343307b3.0 for ; Tue, 08 Oct 2024 18:47:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; t=1728438430; x=1729043230; 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=VBm0hfq2DcbVBzfR35/h5cOzEtSAYZKed8pK56bnJXg=; b=bZL1wqWBaPItDN2x4BmwgUwqZcK7WS8aJfORyDG/PFZKnVFQA6pw5fDH5qoPKnoXYS FyJhxuZa4QcG1UCjtE0AFm5RR81azYjmhHA3HDn75dhF0uyvGtW34hsueMx0uRXEdNzp pYJ5hz9JAfrmFYLviM0b+S/+putqSeNhv9uIQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1728438430; x=1729043230; 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=VBm0hfq2DcbVBzfR35/h5cOzEtSAYZKed8pK56bnJXg=; b=XhOwRMUXrhCdd/xeDXBDvypzcQvptlGNdKEW8upOSjXOoLl6JwtvC1sJcALTjYQ8Um 61sbDdQo70v/IKXjdMr+shuRsECs/Za51oOqqWNJ92cuvknI8xr2vFBr4RdqHcLhS5M5 q3t1wMewPU4Nawc3Cmd+tKHhnDkVzOuDISv96KXX6v38yo2lrfrIjk7b4KBLig1ygOpV ZLspGCGxPr7185oOHxObvPZcU8f+4AOk9MqB8vQ9WbQu/KiXtJNLG+hUJnPEoW9Mf+PY 0KSHAJpF0Xf3ruijZeeU7dhdSStNBg7hI6mHrjSGTWq6R/K0LWRXZT/gVbXuMlSE88SY gyCw== X-Forwarded-Encrypted: i=1; AJvYcCVpdfBpMANCY/GUH/nhJd9QJzRbbFQxpxFMXfEeLrpCdBKZwBz/Z2jGpTW26/fKJ3YODPcIPcM05A==@kvack.org X-Gm-Message-State: AOJu0YzDgPLmdkSjpC/kUV2yCvzp6rvQs6E2TPYYbe4lrs6yUAGgzmQ8 uyFmdqA491sZ9h7O7ia80aM27k1ToXSlykwHj6h9FmoLuxQsHnxmrV4yOf6GIF3q3/5jYxqCAvD Q8eYRgXaP4WLeg6gSCCc1e/TgWk/5wC4/hNBX0Q== X-Google-Smtp-Source: AGHT+IGpuyjOSuBdipJ6DMe3pkHIUBlBJLYPgayvZ5t0w/YPLCdD9uwweKmbCeluhi/jTwGh9EfTkRJrLji4HOdLVAo= X-Received: by 2002:a05:690c:e0d:b0:699:7b60:d349 with SMTP id 00721157ae682-6e32212b7e6mr10584667b3.11.1728438430556; Tue, 08 Oct 2024 18:47:10 -0700 (PDT) MIME-Version: 1.0 References: <20241007-move_normal_pmd-vs-collapse-fix-2-v1-1-5ead9631f2ea@google.com> In-Reply-To: From: Joel Fernandes Date: Tue, 8 Oct 2024 21:46:59 -0400 Message-ID: Subject: Re: [PATCH] mm/mremap: Fix move_normal_pmd/retract_page_tables race To: Jann Horn Cc: akpm@linux-foundation.org, david@redhat.com, linux-mm@kvack.org, willy@infradead.org, hughd@google.com, lorenzo.stoakes@oracle.com, linux-kernel@vger.kernel.org, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 8FB65A000E X-Stat-Signature: d89z9bqa7w3ik5zwfkxpd4a35si8zjfd X-Rspam-User: X-HE-Tag: 1728438431-208036 X-HE-Meta: U2FsdGVkX19Tb8m3NsJlW8m9Qaz+2XI2S2fwyh5NIDcPMahdHyT0IB+zdvn+/+Svk9heCVfvSZI3XT6a77CFaNvVGZDMwBimE2MD0BQlPFJXXAKyJ91QpYg01F8enL7yPp/x/7nDwgW6eSlKM2rcz2krr5ucs2i2L+ICU84zX6i6+EhvubmQCK83P1827BciahdswJVFWRybVs1+LRU29GTkBmz+4tjljHgvCInXNi7UIozhWqB23Qa/EjPeMtm+NqZDFWUOFLuRMQ2ZVr4uULZjXfXewWazY6r+qnWylPDFm15RX+glgGbvdQjdx7McYOZ0jj96eWwSlWJMO4zpqK5KbFb99u2y4WFDVp+jjX57tO5jLOXOXrkhIEhAkLHwTFHzPKO4WHi3xX8QR4ZUfTNW9vmiFrAMlBYoWFJLIagKjaoWEFaM543A5lmeYcxziGCdJ8XxsfT30mtBKc2jlFPRurTdUVPDoNFfz0tFuatXlxsIL3ibmqJb6POMSpqJDN7CiRWMYiUg/mR0WEVfIBhlb2sgyR5Ta1ink0scbBbKWvT/grPdINPJSmF9ffKS3EPb+Mtog8tZ7sMElctawkkYY4qb+tul+f827tU+h/FxTVqSofJqvHTa12TPtvkONOJbWaSzHY2CxtW90p2i0YWJ54hH/2MSdygqNRGyQFbTrSVJm4nfjoFzfRCR7dAe2cUxSiYijgtsV+xPwvuBy94ytV3PJ51oWR4CmcljxhHIzX2iivbQBww8aV7ja+EwlVaWP2EvHUkoPGog2jtn9NWIruIO6T96QDtif47NWZnAxS0+WBIxP07QS7ZRSdc0Roi/+eCjHMPe+b9/TYsEU+i6ner3S4+Wmj9mbd6VvTkFeaZywmux7QPXIeBK01mGbMuSqAVFmOxW6VyeHMWH7vzR/Elzkfwpn8X/n5RrtbRcMLhocp0LS1xbkzAZFWpgT7ztZDOZmnnK80TMQZg 7517ZJJo mInRoqOK8/VtzW+khl1ZxG8veBPrUs20XMSMxhN3DRaadTaBCwS4IS1rnh7hkEmQSigCspNk2MwgTUJB2LCQHCBYZfbPvriLMZgkzUYcuQLcKPfOtJmGQ/IcgV/3BQLFJvGOBHbeT2TQA2OEeHW053G7Sn+SehQRI4ZkhixhFFWxWxSGezb4K65Q5vX8JyhRcy/8tpY1FkTJquIJhmqM0q6HTl+4jcp+EjQCptfbiB+rrDzhok56EtKhXtSjAfKHwtVDlLDsCyTaU3bOunko3DFNUUG00ULSq0K6OKV6i8PGm0rhMtHgTl7KR5O+T7KNro8zM 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 Tue, Oct 8, 2024 at 8:50=E2=80=AFPM Jann Horn wrote: > > On Wed, Oct 9, 2024 at 1:58=E2=80=AFAM Joel Fernandes wrote: > > On Mon, Oct 7, 2024 at 5:42=E2=80=AFPM Jann Horn wro= te: > > Not to overthink it, but do you have any insight into why copy_vma() > > only requires the rmap lock under this condition? > > > > *need_rmap_locks =3D (new_vma->vm_pgoff <=3D vma->vm_pgoff); > > > > Could a collapse still occur when need_rmap_locks is false, > > potentially triggering the bug you described? My assumption is no, but > > I wanted to double-check. > > Ah, that code is a bit confusing. There are actually two circumstances > under which we take rmap locks, and that condition only captures (part > of) the first one: > > 1. when we might move PTEs against rmap traversal order (we need the > lock so that concurrent rmap traversal can't miss the PTEs). Ah ok, I see this mentioned in move_ptes(). Thanks for clarifying. > 2. when we move page tables (otherwise concurrent rmap traversal could > race with page table changes) Ah ok, and these are the 4 call sites you mention below. Makes sense. > If you look at the four callsites of move_pgt_entry(), you can see > that its parameter "need_rmap_locks" sometimes comes from the caller's > "need_rmap_locks" variable (in the HPAGE_PUD and HPAGE_PMD cases), but > other times it is just hardcoded to true (in the NORMAL_PUD and > NORMAL_PMD cases). > So move_normal_pmd() always holds rmap locks. > (This code would probably be a bit clearer if we moved the rmap > locking into the helpers move_{normal,huge}_{pmd,pud} and got rid of > the helper move_pgt_entry()...) Thanks for clarifying, this makes a lot of sense now. So the optimization is when we move ptes forward, we don't need the rmap lock because the rmap code is cool with that due to traversal order. Ok. And that definitely doesn't apply to move_normal_pmd() as you mentioned I guess :). > (Also, note that when undoing the PTE moves with the second > move_page_tables() call, the "need_rmap_locks" parameter to > move_page_tables() is hardcoded to true.) Sure. > > The patch looks good to me overall. I was also curious if > > move_normal_pud() would require a similar change, though I=E2=80=99m in= clined > > to think that path doesn't lead to a bug. > > Yeah, there is no path that would remove PUD entries pointing to page > tables through the rmap, that's a special PMD entry thing. (Well, at > least not in non-hugetlb code, I haven't looked at hugetlb in a long > time - but hugetlb has an entirely separate codepath for moving page > tables, move_hugetlb_page_tables().) Makes sense. TIL ;-) thanks! - Joel