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 23942C6FD1C for ; Thu, 23 Mar 2023 17:08:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A143F6B0072; Thu, 23 Mar 2023 13:08:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 99CFB6B0074; Thu, 23 Mar 2023 13:08:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8168D6B0075; Thu, 23 Mar 2023 13:08:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 6CF1F6B0072 for ; Thu, 23 Mar 2023 13:08:25 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 2C7881A0774 for ; Thu, 23 Mar 2023 17:08:25 +0000 (UTC) X-FDA: 80600796570.02.17189FC Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by imf06.hostedemail.com (Postfix) with ESMTP id 42AF5180021 for ; Thu, 23 Mar 2023 17:08:22 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Zn2FUBj4; spf=pass (imf06.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679591303; 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=p47XQPQvFte3iGIB9opW7jQR1slmuLWQ99gE5p50XPQ=; b=UBrbWssi2tlEy7c+/cFM6q+AtVy49yskTIOHnCm5m8tY3FzHisrxlWqkDv8XwbPy9PlbBS JJqCLSn68PV+WAafGBi1HmBsFKvuQmQLT9Vt7HrUV/VFvGKs/nwqHXS8kwzn25I40ix8Oz eRHOP8xgxLQYUzBM7xGT0XB5EQkm/bg= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Zn2FUBj4; spf=pass (imf06.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.221.47 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679591303; a=rsa-sha256; cv=none; b=00qCzKIjSLAisii53/+4ZILX37uUaDxYJH4YvRXiUw160jD0NGqzab/ds1VGDk7Hkv1pd/ QvQC6ft909xB/lxuxb7QiTk56puV5PLIhhmMY44+CJhYUxFJfIqVLpgP4EUe2N2uOBwh9t /NbZ0wdxTOG38C9s83asChlQaph1SAc= Received: by mail-wr1-f47.google.com with SMTP id m2so21396656wrh.6 for ; Thu, 23 Mar 2023 10:08:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679591302; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=p47XQPQvFte3iGIB9opW7jQR1slmuLWQ99gE5p50XPQ=; b=Zn2FUBj4Op8194h4XZzllO7iXDJqFyZlu4C2wn3r/uOybRY9viI+FL9W3WKo+1JN19 N/ZO0DVjII/gbEfKfQ0LwCHh+qZuPCJarB+v+xwms37Q/rGq9AGgrOvUnn5hxapj/jx4 YZIyEnrv2GlemuPdB7Z2LU+tiSqq99uGAAPrZ3uFTfmhdXQ1Azd2jsqOUwL+nwR8WXFy vMWauFTJ4w8UmGHyrWxM6QQIihJuB3Z2r3DsztDz3LOl/Kz7SK3w1l0D3BWXsO3RIey2 DGfKm1yJIjIkg1rWxLblNYCldKToXEdOw87ozbAN/yT/KxY9Ek33b+hX3G094v3PREHm qR9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679591302; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=p47XQPQvFte3iGIB9opW7jQR1slmuLWQ99gE5p50XPQ=; b=igQ3HPv170hapxcsvgLAKThjK3RJ27VprU4RO5iRe9d9jaa0v/g29DflDQXYQOvAiG 1dp8oT4vFyFaXBzRXTQmonUyM9Ya3X1zSj6OXJ+hSClYsplv/kSYwBGMD9CIE4LeMpaT BpeWDGhW5tzSeyK5Dx8qDPRUp/xvHht/ADL++7FRNO/bhwqxKvpsZM7MoP76iS1ccAk2 z6ztMpT/B0bqiA1Bxgj8cix1lxYpoF6YsXIRIpo2X10KxiYTZdIQ1NxiDRq8VspAEIqW RmtvpFxs0VQB9T7xwun0YPjpUyHo6NdKfxCtHubhexJ5Ak5q61zeeShq4lGMh13WRWCJ WvAA== X-Gm-Message-State: AAQBX9dt0mYFLeiU2x07C6TkAKWp3TW2zfHZLBO3fd+ehgCwftowr52w xo7g+fKrVgiyGOlCuc6rxHg= X-Google-Smtp-Source: AKy350ZhyAO9ZpOeJod3keJfbIobAwhXpkNqYCgkiMAj8yuljl/0aw787pdSWkewT1aTj0GwaWkxnQ== X-Received: by 2002:adf:d091:0:b0:2d7:452f:79ec with SMTP id y17-20020adfd091000000b002d7452f79ecmr3254675wrh.7.1679591301616; Thu, 23 Mar 2023 10:08:21 -0700 (PDT) Received: from localhost (host86-156-84-164.range86-156.btcentralplus.com. [86.156.84.164]) by smtp.gmail.com with ESMTPSA id n12-20020adfe34c000000b002da75c5e143sm4182657wrj.29.2023.03.23.10.08.20 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Mar 2023 10:08:20 -0700 (PDT) Date: Thu, 23 Mar 2023 17:08:19 +0000 From: Lorenzo Stoakes To: Marek Szyprowski Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , David Hildenbrand , Matthew Wilcox , Vlastimil Babka , "Liam R . Howlett" , maple-tree@lists.infradead.org, Vernon Yang Subject: Re: [PATCH v2 3/4] mm/mmap/vma_merge: extend invariants, avoid invalid res, vma Message-ID: <5f88b7f9-d9f9-44ef-9a74-b338d0ba9895@lucifer.local> References: <17b6fc3edc46c4b33aa93b9ef17a63a3a76f4b5f.1679431180.git.lstoakes@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 42AF5180021 X-Stat-Signature: dkkq4ppx5z6wfc78b7ibchz9sr6ers8z X-HE-Tag: 1679591302-31150 X-HE-Meta: U2FsdGVkX1/0SpY9uOLkIJz1EBcMtGaGJZQMt2t2J7NwutpqAgdlOlO7P5b10llffzdBT+c/BPJCExoE+WVcQSoG8KKyLPzc8RIbeHJCt2VA2yJJoGlYwrAaBBmIQ2gWVmEi1+k7++BZ+uP3Hx4hr4fnYOstMiNqvSG2YOU1Jat86O7jB2ULxVFDBeWtKp9CCYK6l5d4TCtYiXerXO4nh4cSkWlsY0YCiR3CkCti3cBkDPYAkxtq1TdMH0qg6T9yPd5CPEt/Zvs1pcF5vPEHtUlA/EAeo5YDBp2qH+y+ndWlKA+zFr5w1QMQ2+JraYwewZizEGB37t1kr8Q8/wJuFDGL3IWwIoCsQvBQJqWA8NwEdkqyz2UmYnJvsRG6pFfWr+109J1auLLxd2aN2BZlWGBdFBc8vkaCND2iVUibw0xPQuq/6b/t200Nxtio2U5PvjX07N3ObFEtUQ0pYh7HawqO10NhyrE1mDApdDmhp3ji+4AQwAAQDFOQ8giu9xd5TlcpLJUXVHw6gMlpydMEBgOFesRSOMiXBCEGNEcNU1kP2lq5UvDX+m+IG3GsYEawnVt+kBUj6qihrO9IRWRoblFEJpudTA1vs4yN8j7JVr4lurF2atfAVZaMhGloF0ON27xkS0eKPmfdrl1f4m2Fjvh3NpBiJBfVtAjwdGSAZJQoPGvhDBalF2MWSoACQzE9ZD6/ZZL+qITTK4l+mz//Y8nFcpWWYTv5hAkPFTS7SDohkI0mCgDRSUgcRsoC2GaZ2SIenhmSC3/nFICvWk0vD3SmCMdFAIreQ8uHYvd+SjEaJPZoG0g2HETj0ucXtItepo/CQBnVH9KU80ItwJzEW1fzHMOQ8rkB6JJ//iLIVnKLzyiTIAmbTHkd/UWEWcgafG4FAcjDUSSSp8em4gE3x/E6s/0ixqwNvc8SLTfQ3GJbn6w4/HOgDTyQHk6NSWDWOVF+bEuQW8ggQ2mE50j zmQY7/Oe L8yV9tMWeM+riFsxof05vHQtl7/l9KLiJacmDl6Rg4o5WP+X30rEOQ+heu5orfpzhuxlOS6WlIlIjT2b1J1BOHVhXMSNnqsn+FkO0gJNFaHRhltEgkqvdqH4tqvuSsWfR6Lhwot6q2Fp/wmamPBbQFdxdCvgOpBzfX2Q/HfzF21pIZTL1Vy7L3TWcCvXkwLr9hMWyQC0iR4FND8pMyIzShZBZO4bNfZ1p7mFT+o7/NBZFoyHn7KbXDkAevAgqKb0voaPXyB+CFnN898Kzyu2DgDdnJVg9LdLvx+hZ5J72a4dJ+0Robq7jZZGSfolTfpAKP0RWdNX66nCcC3XlYNdO5Iwlihsm/a5axvo87kE7DevatqbWGSZ4NITpOpKJ5pc5M9eADm7vB0EuJru6RKLWTnhrmZpDWCgNr1/JRGE7a5+ye9A8gX14k6zjJSe9m61hNyy9NlFkx/ZDyfXhnTpgIfTbEgE+phYozl0SJncCfym27rSiW3NfYfzV9j0jRogFXAA7eHfuaom0XXJz/+3pMwJNi3LGY6K2qG1K 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 Thu, Mar 23, 2023 at 06:00:45PM +0100, Marek Szyprowski wrote: > On 21.03.2023 21:45, Lorenzo Stoakes wrote: > > Previously, vma was an uninitialised variable which was only definitely > > assigned as a result of the logic covering all possible input cases - for > > it to have remained uninitialised, prev would have to be NULL, and next > > would _have_ to be mergeable. > > > > We now reuse vma to assign curr and next, so to be absolutely explicit, > > ensure this variable is _always_ assigned, and while we're at it remove the > > redundant assignment of both res and vma (if prev is NULL then we simply > > assign to NULL). > > > > In addition, we absolutely do rely on addr == curr->vm_start should curr > > exist, so assert as much. > > > > Signed-off-by: Lorenzo Stoakes > > This patch has been merged into today's linux next-20230323 as commit > 6426bbcc76be ("mm/mmap/vma_merge: extend invariants, avoid invalid res, > vma"). > > Unfortunately it breaks booting of some ARM 32bit machines, like Samsung > Exynos5422-based Odroid XU3 board. This shortened log shows the issue: I'm very sorry about this. This was due to a bug with uninitiased state. This was fixed in v3 of this series, which is now at v5 [1]. This has already been taken to mm-unstable in Andrew's repo, hopefully it will be taken to -next soon! I have made sure to reply quickly whenever this is raised to ensure people are aware, and I will also buy anybody affected a pint if I meet them in person :) [1]:https://lore.kernel.org/all/over.1679516210.git.lstoakes@gmail.com/ > > Run /sbin/init as init process >   with arguments: >     /sbin/init >   with environment: >     HOME=/ >     TERM=linux > 8<--- cut here --- > Unhandled fault: page domain fault (0x01b) at 0xb6f03010 > [b6f03010] *pgd=b5e84835 > Internal error: : 1b [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 2 PID: 1 Comm: init Not tainted 6.3.0-rc1-00296-g6426bbcc76be #6445 > Hardware name: Samsung Exynos (Flattened Device Tree) > PC is at vma_merge+0xa0/0x728 > LR is at vma_merge+0x294/0x728 > pc : []    lr : []    psr: a0000013 > ... > Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none > Control: 10c5387d  Table: 4f11406a  DAC: 00000051 > ... > Process init (pid: 1, stack limit = 0x5219a5c7) > Stack: (0xf0835e30 to 0xf0836000) > ... >  vma_merge from mprotect_fixup+0xc8/0x290 >  mprotect_fixup from do_mprotect_pkey.constprop.0+0x210/0x338 >  do_mprotect_pkey.constprop.0 from ret_fast_syscall+0x0/0x1c > Exception stack(0xf0835fa8 to 0xf0835ff0) > ... > ---[ end trace 0000000000000000 ]--- > note: init[1] exited with irqs disabled > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > Reverting it on top of linux-next, together with 183b2bced4c9 > ("mm/mmap/vma_merge: init cleanup, be explicit about the non-mergeable > case"), which depends on this patch, fixes the boot issue. > > > > --- > > mm/mmap.c | 19 ++++++++++++++----- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 6361baf75601..7aec49c3bc74 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -911,7 +911,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > { > > pgoff_t pglen = (end - addr) >> PAGE_SHIFT; > > pgoff_t vma_pgoff; > > - struct vm_area_struct *curr, *next, *res = NULL; > > + struct vm_area_struct *curr, *next, *res; > > struct vm_area_struct *vma, *adjust, *remove, *remove2; > > int err = -1; > > bool merge_prev = false; > > @@ -940,14 +940,18 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > /* Is there a VMA next to a hole (case 1 - 3) or prev (4)? */ > > next = vma_lookup(mm, end); > > > > - /* verify some invariant that must be enforced by the caller */ > > + /* > > + * By default, we return prev. Cases 3, 4, 8 will instead return next > > + * and cases 3, 8 will also update vma to point at next. > > + */ > > + res = vma = prev; > > + > > + /* Verify some invariant that must be enforced by the caller. */ > > VM_WARN_ON(prev && addr <= prev->vm_start); > > - VM_WARN_ON(curr && end > curr->vm_end); > > + VM_WARN_ON(curr && (addr != curr->vm_start || end > curr->vm_end)); > > VM_WARN_ON(addr >= end); > > > > if (prev) { > > - res = prev; > > - vma = prev; > > vma_start = prev->vm_start; > > vma_pgoff = prev->vm_pgoff; > > /* Can we merge the predecessor? */ > > @@ -958,6 +962,7 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > vma_prev(vmi); > > } > > } > > + > > /* Can we merge the successor? */ > > if (next && mpol_equal(policy, vma_policy(next)) && > > can_vma_merge_before(next, vm_flags, > > @@ -998,6 +1003,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > adj_start = -(prev->vm_end - addr); > > err = dup_anon_vma(next, prev); > > } else { > > + /* > > + * Note that cases 3 and 8 are the ONLY ones where prev > > + * is permitted to be (but is not necessarily) NULL. > > + */ > > vma = next; /* case 3 */ > > vma_start = addr; > > vma_end = next->vm_end; > > Best regards > -- > Marek Szyprowski, PhD > Samsung R&D Institute Poland >