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 96911D11717 for ; Fri, 25 Oct 2024 07:43:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 39C876B0082; Fri, 25 Oct 2024 03:43:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 34BE66B0083; Fri, 25 Oct 2024 03:43:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 213786B0093; Fri, 25 Oct 2024 03:43:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 048C16B0082 for ; Fri, 25 Oct 2024 03:43:17 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id D4936C02F0 for ; Fri, 25 Oct 2024 07:42:56 +0000 (UTC) X-FDA: 82711333446.10.CC5E80C Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf08.hostedemail.com (Postfix) with ESMTP id 96BEA160017 for ; Fri, 25 Oct 2024 07:43:02 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jIAC9M7H; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729842143; a=rsa-sha256; cv=none; b=QZmC0usf3UlLgBy/igyjcYmv9biCwqQ71UsEgaWPq502/xz6jSClmq1G2mZmsfIExI+VEJ JgBlpmUzZ3+Icdf5RFXFKsZDBC0CU6KswFQkfL48/EieOPdHxkRGKAgu0PG2su+Jfgg+QI 730hf1edlvNA5hGgOidcv2LfewNpDm8= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=jIAC9M7H; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf08.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729842143; h=from:from:sender:reply-to: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=VSlKz6KX9wFEtipmKwFu4U4Qx4mXrW2e04YuVOrqnuI=; b=oze1MocHtYxV4Li1nQV2dmkJ4fX2lkn7Vy9E+4z5Az0ZxPg1tK9b+v0oN5HNakZjfzd5hv Fh+VLo/c/cRkzkScqDX88PqMgPTqDyIPrmFRq9Gml/je7+tSl0sYEal4iVegTOxwft23e2 mlK5wnx9h8ldO/SJ4D+D+DgcVX6426k= Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-5c9c28c1ecbso2269873a12.0 for ; Fri, 25 Oct 2024 00:43:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1729842194; x=1730446994; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=VSlKz6KX9wFEtipmKwFu4U4Qx4mXrW2e04YuVOrqnuI=; b=jIAC9M7Hob8/EdBXOYYqVjz1b0ge3NFGSc3Da6BgIN7TOb35oKLofqOn0VIdevuZjv 9q97lGiLF3IqAJBtaDC62eWBx7dldTsmiTDLcxdnP0B8RDkM2T2qM6IUaSajlC5wVBCX y4JEDGbeCsiUwAWRi6PPjYxQu5h/08DMfB8Kp9jyTuq6/K7AWpUBrqdKV6Dt7SW7itf2 ftzeexUICQb4RZaNbzae1KxrdFm9C0eD+PXZEeSxCJDZH3iogb+3ciV1czQEsrqN+zfk Z5GuQkGfS24mCCB60YONYvUZHGB8YHX40CdXtUl5bUF9yg9QmJz08iBZGOAPlq6LsrZ/ bIKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729842194; x=1730446994; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:cc:to:from:date:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=VSlKz6KX9wFEtipmKwFu4U4Qx4mXrW2e04YuVOrqnuI=; b=Pzp27AFd86b03s5dsk8xVE6kzIQvR7cOEYOgBT3Q3kNyxeeR+6oapykeETigkm6ffj yGtIkVkkPUR70nA5FYoFDjA/QW+ylKboJCAhlTJpTollhAbWT7KkDSBho+exY/LQeofw XyMCJH9R8VgVS8gITYnWzrm81kjEPZxrXLInpZkJOMyexMbJoF06/gb2QIgWU5cZ/CgY js/CnL1jWdadFXgwzaztqEtaOZCHJGqHFIYebgHSr8w/KKxAKB/D+Uf8HqXU4PMZer/j 0kMP/9JUFDKpW9mA1avOch9Hca68e/JL1V6HDgw/TMEcXKMsQdn7ytZg7IpMyxpjipdK MMEw== X-Forwarded-Encrypted: i=1; AJvYcCXcujFL7UVDPyWVKI45WYNy2TR56+tFRitHsIGrlFiByLZi43b7PqIF5cTp1tee4s3Ll6zlAn3ybA==@kvack.org X-Gm-Message-State: AOJu0YxYB6Q3ePT6dO+hWbFrWlNd42KMSqMPeZaaCJE6S0Afg9N1/gwd 9V+LCC5SmshfHlO40r93+F9vDbEhqhHph33jLvOSeCNWShCmqO0z X-Google-Smtp-Source: AGHT+IFqTbQizmJ4nK1FmeY7yica6evESZ9n1+z+wqWRZYOESNxMjYbV/UO0n/5aXyl3ibVeFdE96w== X-Received: by 2002:a05:6402:27ce:b0:5c4:14fe:971e with SMTP id 4fb4d7f45d1cf-5cba249331fmr3969349a12.23.1729842193658; Fri, 25 Oct 2024 00:43:13 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5cbb62c42c7sm335055a12.51.2024.10.25.00.43.11 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Fri, 25 Oct 2024 00:43:12 -0700 (PDT) Date: Fri, 25 Oct 2024 07:43:11 +0000 From: Wei Yang To: Lorenzo Stoakes Cc: Wei Yang , akpm@linux-foundation.org, Liam.Howlett@oracle.com, vbabka@suse.cz, jannh@google.com, linux-mm@kvack.org Subject: Re: [PATCH 1/3] mm/vma: miss to restore vmi.index on expansion failure Message-ID: <20241025074311.invqnot3xow3pzs7@master> Reply-To: Wei Yang References: <20241025031847.6274-1-richard.weiyang@gmail.com> <20241025031847.6274-2-richard.weiyang@gmail.com> <848b5701-9dec-4c69-bcca-f9186090978a@lucifer.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <848b5701-9dec-4c69-bcca-f9186090978a@lucifer.local> User-Agent: NeoMutt/20170113 (1.7.2) X-Rspam-User: X-Rspamd-Queue-Id: 96BEA160017 X-Rspamd-Server: rspam01 X-Stat-Signature: 4rhdxt56hju4puh4u9ksuo8wt76fhiag X-HE-Tag: 1729842182-337017 X-HE-Meta: U2FsdGVkX18UU6OjW2WSs+FVXMsBiAcKoi7cDeLnWj8TEmPy6fo8Vqi1hx49HqkIwT/FVPMZt9dIWoMXnojCUQrOqiwGi8ujM3b+9mN6PPvXWuF3S/TveWqey/Zr3d4k+6WoBhrIWkEzN87e8+GPuV04theo5PalF5srmkeW2iSh9Y7pxhiAcFKHDffGCq7lJbNjb8PWDbcz/PmTfIt16fHBUOsaVVBMFzfR1faVyzQ00NKtTGXuFU7Z8Ovz5EhndFHDviIzoOTiSpr5hF1R6drEe/MbyO2nUhbic5PL6u6ZSUZ45m38iEReCaf6Op4TmCfJlNB1nXR8p/MDOp/HZ0LEXkt16ai/xf1YejAyO2q2rfMN0/tCxc4usMjQNKPRApwrWkGeiIWLAQFsuP6QvcveYHY6SXjT+BZIZXBpnyr2iMc9DPulSxfkJ9ZxM9LwQhHagBeOd/CFnatrNdv9etyna6m3U0pKyzAsBjL4gGCYu4bfvuJPf7H6MhGsZe6eiBXG9Hc+h+FVOL1YV09fLaRNFfmEno8GhcV8atlYYuILESivOZN0sEdQCkX7sFMwh3wEPejeyir5wi9TJD2m+s4mZmX5hfiFXCXljMbsu7kaIbThdMHgQIdYxJgEsxgmLqXNJI164k1L/zpXq3YlfO2PaGG8vxtf7xfMy7EMXdtdwrsURI0PwO7ZqpcJOBbi5Rt8m4+aM20gWjnJ3J4H6i1mUyr/nKHs/iKTt/ES4d6oTZ2X8ZKNGnqbYYxd9f90ZDs3kNlUzrQBd25nl7Zy2z5MjIzTtumshpwfRxdr2/NQOuBMPEa44nddJpcvTLh4DlXn3gFJX0E5t749oqvrdBx5xp/UDkk0huUYJZWyTDWaW6fzobf9qrTWJC91+GTVJml60rtEQ7E4dtvkXB/v7AFqoEYqGFzDbTWVJTeQ7m0jaB13EyHoqekq+wYEFQP32RxgGIQ3Vi6t2nRv5a1 PK5PD7TG FAME9deTjnYupQSGJ+HO/pFtfAxhLFs18GWUo/k+NgvL2KWg1X/CJt4RftplkcuNa1yRXlC4OddMMJbIcWAKDLEzmvr4qgb5BpyNTOCkCuvpoIMAfnZ+wFCOEGcxZhjKcUynJQnIJsRn8+7ihQ3fHMTeLry540LZu9FujoVIuDcoPD6Nvqyk+htF54toPqxuS5yfvKukgo1CQyOWvGNJmm/LLLLioXODWza9wpQUaXkO67A/x81rpDZ5XJNJponSy46f7EpsGQrcKMIuYim/tIdHp1U9zPguCq7dY1uw5aYS9IKNnd62LHlf9kZ9XLeWUSTugszr6pqvZM2SnM6SXM6G7vFHJ1bSBMIxAWAVk2WcAYIl4WZMRRhD4pU9vmROMdaotlQMAjVNLO48pcu/4eLiUtO8m/nuvyQ/BmC02Te9Uo2f5A6dd7Yr4Z91RXmPbYfdFa4AwTQIs6d5+j1xS/WpoO3avtwp85iAeCmp6fWaZo+5atRyeFtc8cMiv8crX9p0w7Ng7Y7CDO/95Mf6FEFLB8orMT8aiX6P9IfmS5NDIxOPimuCIGccbuz+mNlvIH6tjp4W92T0O+S0GPxOTWIPWIPuMvyJ1g2nrD6UUaEYzhTiuQf6VuqwrZpqBccUXKE/ts1Fb6F3MHfGDzros9G0/YCF8WC6xfZdYcSfPcEvCZ7o= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000163, 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 Fri, Oct 25, 2024 at 08:06:06AM +0100, Lorenzo Stoakes wrote: >On Fri, Oct 25, 2024 at 03:18:45AM +0000, Wei Yang wrote: >> On expansion failure, we try to restore vmg state, but we missed to >> restore vmi.index. The reason is we have reset vmg->vma before checking. >> So let's put the operation before reset vmg->vma. >> >> Also we don't need to do the restore if there is no mergeable adjacent >> VMA. Let's take it out to skip the unnecessary operations. >> >> Signed-off-by: Wei Yang >> CC: Lorenzo Stoakes >> --- >> mm/vma.c | 10 +++++++--- >> 1 file changed, 7 insertions(+), 3 deletions(-) >> >> diff --git a/mm/vma.c b/mm/vma.c >> index fb4f1863f88e..c94d953d453c 100644 >> --- a/mm/vma.c >> +++ b/mm/vma.c >> @@ -954,23 +954,27 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg) >> vma_prev(vmg->vmi); /* Equivalent to going to the previous range */ >> } >> >> + /* No mergeable adjacent VMA, return */ >> + if (!vmg->vma) >> + return NULL; >> + > >Kind of a pet peeve of mine is throwing in a random refactoring thats not >mentioned in the commit message. Please don't do that. > >I think it's fine as it is. > >> /* >> * Now try to expand adjacent VMA(s). This takes care of removing the >> * following VMA if we have VMAs on both sides. >> */ >> - if (vmg->vma && !vma_expand(vmg)) { >> + if (!vma_expand(vmg)) { >> khugepaged_enter_vma(vmg->vma, vmg->flags); >> vmg->state = VMA_MERGE_SUCCESS; >> return vmg->vma; >> } >> >> /* If expansion failed, reset state. Allows us to retry merge later. */ >> + if (vmg->vma == prev) >> + vma_iter_set(vmg->vmi, start); > >Good spot in that we've stupidly been setting the vma NULL each time before >comparing... (doh and mea culpa!), but this actually accidentally proves we >don't need to bother resetting this at all :) > >The only case where we care about a reset is mmap_region(), and there we reset >the iterator _anyway_. > >> vmg->vma = NULL; >> vmg->start = start; >> vmg->end = end; >> vmg->pgoff = pgoff; >> - if (vmg->vma == prev) >> - vma_iter_set(vmg->vmi, start); > >So please replace this whole series with a patch that just removes these >lines, thanks! You mean this? --- a/mm/vma.c +++ b/mm/vma.c @@ -964,14 +964,6 @@ struct vm_area_struct *vma_merge_new_range(struct vma_merge_struct *vmg) return vmg->vma; } - /* If expansion failed, reset state. Allows us to retry merge later. */ - vmg->vma = NULL; - vmg->start = start; - vmg->end = end; - vmg->pgoff = pgoff; - if (vmg->vma == prev) - vma_iter_set(vmg->vmi, start); - return NULL; } > >Also what tree are you making this change against? All mm changes should be >against akpm's tree in the mm-unstable branch. This change looks like it's >against another tree, as the code for this function has changed. > I use the upstream. Will rebase to mm-unstable next. >> >> return NULL; >> } >> -- >> 2.34.1 >> >> -- Wei Yang Help you, Help me