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 15BE8EB64DD for ; Mon, 14 Aug 2023 21:22:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7685A8E0003; Mon, 14 Aug 2023 17:22:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 718D08E0001; Mon, 14 Aug 2023 17:22:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 592878E0003; Mon, 14 Aug 2023 17:22:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 4760A8E0001 for ; Mon, 14 Aug 2023 17:22:44 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 1E9BB1C99AF for ; Mon, 14 Aug 2023 21:22:44 +0000 (UTC) X-FDA: 81123984648.08.6290ABE Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf01.hostedemail.com (Postfix) with ESMTP id 55A4140007 for ; Mon, 14 Aug 2023 21:22:42 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=of0DjEEw; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of jannh@google.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1692048162; 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=gL0AM1Je6e5fBP6gbPX83S7jRl54DBoHaRHEKtf7HH8=; b=3xKvDCmiCPcTg8FIAHBde3NAmhbZuX4b/iXFf7k5BReXQtF9FyVgYgkwi4ETZoCWUJKmC7 HfI0ni8R0rvnAjhh9xKD7iHMPlFQYEZq9v6SOTQrE5KdWdmRugN600bnTGjMIpZi+FCSL2 thd5nrQapckWTjWBAuz68b7UDAnJl+M= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=of0DjEEw; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf01.hostedemail.com: domain of jannh@google.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=jannh@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1692048162; a=rsa-sha256; cv=none; b=vw6+untAVh3zkQYBu6gzNKW8yOPb4wgaodlUrWO4UOYG5w11rCk/8ZRZWPYVIxyEWk4wrY VcXtPN6f6mEcFj6ybHbGliwBEED/C0g6BMijPOauMnMShpliA05ec7YzljtgLUJ0Q+gUK3 46p6i0v3TDvNcpxxyrW04xeQ5odYLfo= Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-3fe32ec7201so12145e9.1 for ; Mon, 14 Aug 2023 14:22:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1692048161; x=1692652961; 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=gL0AM1Je6e5fBP6gbPX83S7jRl54DBoHaRHEKtf7HH8=; b=of0DjEEwmYxilK4o4yzt2UbCzOJrcQEaB5PQ0IncGy/5eVK4gRp37L3F/SWoqzSlt6 vCtVzBQKEmw/NCZ9PnHfJ0bnTlMdyo09OKI9HA5UU8pN4+efwnXvO6GuoKWiqG3m8VcF xj1Cqc4BLOa1nuzf8B0r8v409e7pBkV+Tw6zxXDm/CMeVNBvCgRvFZqa4T9v0M8ARiGw +sXkLjZ2/qeQTLR62P1z/zBGy9tRnuRQ2yv1x8mad+U7bhP9o5Nb/RV0tCWR25Xndbf+ SnvcUkAR6dy/v7h62LyqcyDMagE/PULla0IT3tEpgDIZ052T+SxUlfvDUJGGTcw4juRr Hrqw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1692048161; x=1692652961; 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=gL0AM1Je6e5fBP6gbPX83S7jRl54DBoHaRHEKtf7HH8=; b=EbBYvx8pwfqN5zM6SNum9x/DtsN5yHL1wEWprHz6HEs3hRTWwPEtnqb7ZBjSQepXik eV9S3khyqCuYLuiyk0HuySU3WEku7gHF2lpBJ+b0DL7+Xxd2JasFPJ/ZPIDnSUxyHB1j j4bqMQj63LFWuVQN2ym+2Nfy1orAkWf4qVXS5jWkU2L3Vc7e59LtgipWFF6UmAb5oRxy oKFvxx9VcXMWH7otBAjf8n36Awn0Zb0+4VSYwFqoSNBCCxSrrDOcjZtxobga9JcP1x4i tO89UoFmCTtWECee0RcvkgGfpywAzsL6SLO6y1iKuqPItpWp+nlHrUzovtxfcuIdNbRH g1XA== X-Gm-Message-State: AOJu0YwJOvQ6WWQFvntVFdKjz0wBzlF57K6XUSD2Bzt5uUMpyDQ8+Cf8 W162887GVQHh+LIgdYHMtQwRc0rQGNn4XooXQrkULg== X-Google-Smtp-Source: AGHT+IHul3sJIyCib/1PCvISVtSSIkVP4Pk9tk4L5J1YA2h/KC+U0UX04u2MNGToWdfc1d1QtDDHtjZ8I3flmd4y/ps= X-Received: by 2002:a05:600c:2146:b0:3fd:e15:6d5 with SMTP id v6-20020a05600c214600b003fd0e1506d5mr341439wml.2.1692048160762; Mon, 14 Aug 2023 14:22:40 -0700 (PDT) MIME-Version: 1.0 References: <20230724183157.3939892-1-Liam.Howlett@oracle.com> <20230724183157.3939892-16-Liam.Howlett@oracle.com> <20230814191835.jzj7ryvhi6dqwruy@revolver> In-Reply-To: <20230814191835.jzj7ryvhi6dqwruy@revolver> From: Jann Horn Date: Mon, 14 Aug 2023 23:22:04 +0200 Message-ID: Subject: Re: [PATCH v3 15/15] mm/mmap: Change vma iteration order in do_vmi_align_munmap() To: "Liam R. Howlett" , Jann Horn , Andrew Morton , Suren Baghdasaryan , linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: yug64zoub4icdjpy6c4fwmci5yk8164f X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 55A4140007 X-HE-Tag: 1692048162-885309 X-HE-Meta: U2FsdGVkX1/3Mm8n5IPU5vLGbJ9u9IBdDXaoLtB8NsEzgSnILUHuWuc9bz3ZAhnVCSy48s9hpWUTqO+jiNWuZAUHuEhm3E5FfieIr5iVyNEJf8eBkDxifd9e59H1uetv1tTODvP+RQpH7BEWwZprWru3PcsNBa1RbxrSN97CaCoSBbeXq2iUP/122DguQBakQ14ReXTX/Pzb4mkwlFN8JAjs+vv7Mx7iFHOIc8SciyVlBjc5JWWAvvvZy9R5DQodPw7Ty7K3qgWLX83L32RBNJjtIY1t+VvD0uAR20zfHsfTVLirt08Zla78Fia0MtGRp3uFMJCXd9uKFHzkENNAp6r68Eh4p4osybXVpHwXqR70DGNH0nxZOhEm0cwSM8IbLvt0mYyjL6l2jWwKJWWU9LI1qZyp1bK2g1quSFyoxwZRIf+n1A+rI0ceyDhgbG3mkkh6U8rOwNyJinJKS5+BDQFZTjt0mMd5LklKC+T7P+gOVQtBTZmOROgqcEyVW3JWC80Ah0FO/j9Yxsib5eyxpvln3z4+RjrdjunuIDWI+KnJx/v8dRxk2vRmGXBvFvCT5OFoIQ70zaYEh8M4e/zZgmUFwFbS0xttJiAQQhg5sKHw9EOI8kaN0inHJtt/9FSPu3OJQhdfcRIyBDTdKPf4RSMjf3uHuyWoEyQDGRro1lUDtDgf8czlWXUPuBGn04tmECf72SnjFNVeXdwe2hjwNp0dUt/sAy/CGznOTNwcNAToPUsls4cMYsTOcMXwSVBYDuq/cnczwCRtPIdRwZkRGUFNg5xtLcV5Y/4asf8HPfaHIUcfWAv4JIf5dHO/0IsRkycdY/igeLeNOWrT+l/WFDOs+K+8o/KQFWGUNG5CD1OpBLaemcyXlP4hy/vZQ70FrEIxkuLMEsudFbkNop2Zk/3m1vgEMM12HnVW4FYsrCb8ZBaCzDc3d0QN/Nh//mqm+1VQseF+rJj/FC3iGB2 E/Qe4IN9 cyqIJAJVc9rS83FC0x15m5wn4TR4yrVK8+sgO4qvlf0msV5nkUgsa967kBN+BNHjXG1/nDfQOoOX5y7KkweWq2ZQYru9hvU7iwIMPKcq+IG72VuQm2EQ5YLodSMJu3AmfAusB5V/btXE6DVfHoeIWes+dtLfdG78KJqZZUUityID4fs9Beo3DVPYPb9dGN4Hbfj/Vlt5hLGTHpk8hCZKbXKCrN3Bfm/yctldxh2b5q8WnmdXLExY6pmxlZ5TeXLPNvXsJA7qjrnZH7WEbrFnp10ea2qRyDpd2zlzxlMW0TN/IKfIqmViRMHvRa5KFtVRzlYVJ X-Bogosity: Ham, tests=bogofilter, spamicity=0.000034, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Aug 14, 2023 at 10:32=E2=80=AFPM Liam R. Howlett wrote: > * Jann Horn [230814 11:44]: > > @akpm > > > > On Mon, Jul 24, 2023 at 8:31=E2=80=AFPM Liam R. Howlett wrote: > > > Since prev will be set later in the function, it is better to reverse > > > the splitting direction of the start VMA (modify the new_below argume= nt > > > to __split_vma). > > > > It might be a good idea to reorder "mm: always lock new vma before > > inserting into vma tree" before this patch. > > > > If you apply this patch without "mm: always lock new vma before > > inserting into vma tree", I think move_vma(), when called with a start > > address in the middle of a VMA, will behave like this: > > > > - vma_start_write() [lock the VMA to be moved] > > - move_page_tables() [moves page table entries] > > - do_vmi_munmap() > > - do_vmi_align_munmap() > > - __split_vma() > > - creates a new VMA **covering the moved range** that is **not l= ocked** > > - stores the new VMA in the VMA tree **without locking it** [1] > > - new VMA is locked and removed again [2] > > [...] > > > > So after the page tables in the region have already been moved, I > > believe there will be a brief window (between [1] and [2]) where page > > faults in the region can happen again, which could probably cause new > > page tables and PTEs to be created in the region again in that window. > > (This can't happen in Linus' current tree because the new VMA created > > by __split_vma() only covers the range that is not being moved.) > > Ah, so my reversing of which VMA to keep to the first split call opens a > window where the VMA being removed is not locked. Good catch. > > > > > Though I guess that's not going to lead to anything bad, since > > do_vmi_munmap() anyway cleans up PTEs and page tables in the region? > > So maybe it's not that important. > > do_vmi_munmap() will clean up PTEs from the end of the previous VMA to > the start of the next Alright, I guess no action is needed here then. > I don't have any objections in the ordering or see an issue resulting > from having it this way... Except for maybe lockdep, so maybe we should > change the ordering of the patch sets just to be safe? > > In fact, should we add another check somewhere to ensure we do generate > the warning? Perhaps to remove_mt() to avoid the exit path hitting it? I'm not sure which lockdep check you mean. do_vmi_align_munmap() is going to lock the VMAs again before it operates on them; I guess the only checks that would catch this would be the page table validation logic or the RSS counter checks on exit?