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 01FD9C6FD1F for ; Wed, 22 Mar 2023 06:25:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 2436D6B0075; Wed, 22 Mar 2023 02:25:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1F3B66B0078; Wed, 22 Mar 2023 02:25:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0BB8E6B007B; Wed, 22 Mar 2023 02:25:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id F06726B0075 for ; Wed, 22 Mar 2023 02:25:58 -0400 (EDT) Received: from smtpin09.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 1A61C12011C for ; Wed, 22 Mar 2023 06:25:58 +0000 (UTC) X-FDA: 80595548796.09.A97361D Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by imf14.hostedemail.com (Postfix) with ESMTP id 47FCE100008 for ; Wed, 22 Mar 2023 06:25:55 +0000 (UTC) Authentication-Results: imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=pPrbd+fF; spf=pass (imf14.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.52 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=1679466356; a=rsa-sha256; cv=none; b=lKhrsr7U8osMOC7EXI4fPAq8uzCzcaEd6CP00cA6kGH8/aFoRld4gmhTMPgHexkT8b6XOI tX4F94xxxezhElnNh9EuUMs/lI2naw8wjRYz95Z2to6j5Mirz1r5TqlZk4kR5mo54+MzU3 60ca1yg85UKgYLogXLNdXiR+9bD3P0o= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=pPrbd+fF; spf=pass (imf14.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.52 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=1679466356; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Z9XKHKg28SX/0LVxj0Ew+2k2WgxMDKd2HShHur6TSwk=; b=zDI8trKkwTAzyGWCfrMYX5m0T02P1/PuHtgSoj7FZzPS/NVvBvkCq2cfEiZhCyvg7k+/kc mKQyrs4FMyYoOdTq1pTRzht8waaBVZRFd5XqVinitzQd2VHUCjtlKhdIJ/VztHKuD2gX0C m9bs6ykAYcrnQdoWrw+GG9WEcIo8lxA= Received: by mail-wm1-f52.google.com with SMTP id o40-20020a05600c512800b003eddedc47aeso5245163wms.3 for ; Tue, 21 Mar 2023 23:25:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679466355; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Z9XKHKg28SX/0LVxj0Ew+2k2WgxMDKd2HShHur6TSwk=; b=pPrbd+fFtq6a4QHo3W9HNyj5LySYxI8tUZLf+eGbwSL7lElwFyOnyN5GGONrbUx4Qy 9h03XlQWRTpC+p996NXqzVLtXT74Q/ul632DgXkrPyrsAhCiFg/CUKxM8Tk+8twtSLH+ qqF+IW4IPtuU0al+bC4/WJlJcKsXIgaVGq1if1EIOcilM2k+7VJWijoei7DiJjraTUGh HP2gnamk4+oWqbyL7iKHPeGpgkeU4KK8m1t1sAdUQfo4ha/zDiZsesRwSt1SMtNjckA3 XYOgPDobClf6yDnNCAXBDNF1CMD+D1lLCk7USCFEjsp7ZfnnVTzr835zk039qRoXEF2b mCSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679466355; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Z9XKHKg28SX/0LVxj0Ew+2k2WgxMDKd2HShHur6TSwk=; b=XlBwMPooubPTB4aqK6oQ9T61sJLPL38xsk71gpHl9H8Hq5DmP/aTbxKreYwxAncNWR XHcGHM7/Me2RFag98NPHrBycmX7wgcig7pwZy1WQXwT+Q8+QfJ/UVy9BEeKD1df1C0t+ sOdIDEOVpv7ns1J0OoEj9Wu548/gv8j4EyTxQknhXV+zLqn3pmtKd1p4qRa9UN0bOjnX z9aQNinOLK7GNhXs6shXj2cPynXT1FhvuyYgmi4ky7PbKTaN9yV/X1nGRQAPGU7///dC cq/t/3ga1CuD/eLs6ALsDD9/DdKqDv4hVwyW+y9ogV1RcmGLblLyNb93IVRjDSbrfI8Q 2Wgg== X-Gm-Message-State: AO0yUKW70T6FgojTqUAjlZsOI7n8S45e1ZyFHJ8H72/6oJwLXDVypJXF ya5ecV3wwDrk/mZRJ5NSDyE= X-Google-Smtp-Source: AK7set/GgXqF/frcQXZ1HiDYhZtDGcAsnFpYy16NV+2VdNP+1Cb9XKHo9VcdXRSfyCahR33ZNG++dw== X-Received: by 2002:a1c:7311:0:b0:3ea:d611:f8 with SMTP id d17-20020a1c7311000000b003ead61100f8mr4670082wmb.38.1679466354645; Tue, 21 Mar 2023 23:25:54 -0700 (PDT) Received: from localhost (host86-146-209-214.range86-146.btcentralplus.com. [86.146.209.214]) by smtp.gmail.com with ESMTPSA id m21-20020a7bca55000000b003ee697ecefdsm275772wml.45.2023.03.21.23.25.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Mar 2023 23:25:53 -0700 (PDT) Date: Wed, 22 Mar 2023 06:23:43 +0000 From: Lorenzo Stoakes To: "Liam R. Howlett" , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , David Hildenbrand , Matthew Wilcox , Vlastimil Babka , maple-tree@lists.infradead.org, Vernon Yang Subject: Re: [PATCH v2 2/4] mm/mmap/vma_merge: set next to NULL if not applicable Message-ID: <467b6158-5c91-4c95-b05b-ea9331f7f563@lucifer.local> References: <3506997a8146a287ec13ee7a53cf916707537bf8.1679431180.git.lstoakes@gmail.com> <20230322014232.4iixbwn5urfo6t2y@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230322014232.4iixbwn5urfo6t2y@revolver> X-Rspam-User: X-Rspamd-Queue-Id: 47FCE100008 X-Rspamd-Server: rspam01 X-Stat-Signature: fnr6nteyqxx97kpcodzxx8gez9536m4p X-HE-Tag: 1679466355-84948 X-HE-Meta: U2FsdGVkX1+tNnJu6OOqblbFVfyvrWNywF83AkrnlHB9UtdMmT0Wg94KXk7dpaHCnW9Ds4P/j1GIrQKcD+XHFfzZ5RHCwTynWVdM176Uu5Ya9ymYQfkHMb8h+PdHSj4rK9YLePSwqbyqpQQGn3XmIh+IS9CiYRwErZtIhAA8vhXPk66ewrCwRAe+zRY8zbldHXNUzG2a+ctEiaIOH5UqvO879BcxGzhj27JN2cD968AOBhOQWCBXFxvoxFpsEprcva1E8q+QNZxByPb392tXP8gWb77p7+Od2ynL5GAMPwsrnUtJ2Hgn35WPySLX5+w3OsTj8lJy0QakK9KU81LcjVAOIjSQBhjjSKflrHVFkB9aLxi43XQSPrm8cl0erYYXCyS83bWorDFgu2mhi6p+3cdSI5ii0rJPvxnVkUNuH1E5KPlk8c+Fgi3WkRuq6G9fBsgPDbEPtW0pBD/itqkMgOB51KPh4toULb6coFSKVDOKC8EIU13azYzNfkfR7nV5gfNsXJ4HzEdygA/YWoEXpIQVwi7ZWxixVcBf/FjBtwnQbuAYlI8Fz53IxKXjWeBJsbxEv9xPX09fvR+nXXGBtU17aXyT2tNkXKkOJVfTNKr1ggQK9i66Iq5aaJ5TRVKqi3jqrX7iTWNi0VgS+TKfbdJVYIagf7oKA3uFrhr8n4mjxG3LFOZHBVXgnKln87yoVigTKXA0tWCP6BvcUX8dZLmbEzSf9gYt3tXaEii/LP9nusDwpff7eIoDWc75eIXT97jNMn78GJmupJVdnGcUKUb9lVzgBYRzL2sRVUjPGzFEmaVJsjrYT1TL+d0v1lB44XT5R+luKHfozWcHLhqrqTFUoEgIOy67ysF0DPNOOmvnJVfVVfVubyU2gvDNU5JAGoy5a72cD691iXjH9Oye47Jwsy9nR8w0UfNtMcwhDmOJlqOCQlHwsnA/I9jRn/C+V4y4XiIzHxyDCV/0f+P ouVUTA/J /tP9vS0Llx+E8r23qDK8PTrkL6dF6Z7u9M3h5r5NQ5NoaiBEH5TOvFIEvxrxBNpWTTZM05atGZ83XpBcTsXq9ZfBZSPSiiaIm8TbLIFrcZwONKTGFEq4LHoq7T7AEBLb4JRB7mJzFbsCs0MAY25pSm1H/pLdmNDjfiAzUpCj7njvSsGZn1dTrUMXHkZfTSLVhAqybfdqMewJ/Tg+5LyCZZyRN+gcOyx1R6kQi4z+ZjBe41RzxC2kpwraNiWiHpuh7/NIqUUzE3LWge8sU8/5O8PM6htEDuy/J7T5LPx46icjKNoKzBgh+qZLwpSsA0sX4y+n+wPJ+1fXFWn50zT4pOM/FG3CHhdPQ2fNTnc9Ra6TFO88tx+NO1a4QvOq45Xm4jXW+iYd6+lr9xam3GPAkCEzFGdYp6ns+twGkfYspj+lkEU5JCr7jMnki8wCU7gXMSpwJ7/f3oxOFB8kJoDGefeA69S26orliQf0L 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 Tue, Mar 21, 2023 at 09:42:32PM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes [230321 16:51]: > > We are only interested in next if end == next->vm_start (in which case we > > check to see if we can set merge_next), so perform this check alongside > > checking whether curr should be set. > > > > This groups all of the simple range checks together and establishes the > > invariant that, if prev, curr or next are non-NULL then their positions are > > as expected. > > > > This has no functional impact. > > > > Signed-off-by: Lorenzo Stoakes > > --- > > mm/mmap.c | 25 ++++++++++++------------- > > 1 file changed, 12 insertions(+), 13 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index c9834364ac98..6361baf75601 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -930,15 +930,15 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > if (vm_flags & VM_SPECIAL) > > return NULL; > > > > - curr = find_vma(mm, prev ? prev->vm_end : 0); > > - if (curr && curr->vm_end == end) /* cases 6, 7, 8 */ > > - next = find_vma(mm, curr->vm_end); > > - else > > - next = curr; > > + /* Does the input range span an existing VMA? (cases 5 - 8) */ > > + curr = find_vma_intersection(mm, prev ? prev->vm_end : 0, end); > > > > - /* In cases 1 - 4 there's no CCCC vma */ > > - if (curr && end <= curr->vm_start) > > - curr = NULL; > > + if (curr && end == curr->vm_end) > > + /* Is there is a VMA immediately adjacent (cases 6 - 8)? */ > > + next = vma_lookup(mm, curr->vm_end); > > Since end == curr->vm_end, this lookup is the same as below so these two > statements can be combined. I can't believe I didn't see this :) wood for the trees + a great example as to the benefit of code review. > > I also think you may need to initialize next to null since it may not be > set for the 'cannot merge' case. You're right, will fix. > > Something like: > if ((!curr) || /* case 1-4 */ > (end == curr->vm_end)) /* Case 6-8, adjacent vma */ > next = vma_lookup(mm, end); > else > next = NULL > This is really turning out to be quite nicely succinct now I think! Will do. > > > + else if (!curr) > > + /* Is there a VMA next to a hole (case 1 - 3) or prev (4)? */ > > + next = vma_lookup(mm, end); > > Nit, can we have braces for comments that make the if/else look like > it's unguarded? > Ack will fix in respin. > > > > /* verify some invariant that must be enforced by the caller */ > > VM_WARN_ON(prev && addr <= prev->vm_start); > > @@ -959,11 +959,10 @@ struct vm_area_struct *vma_merge(struct vma_iterator *vmi, struct mm_struct *mm, > > } > > } > > /* Can we merge the successor? */ > > - if (next && end == next->vm_start && > > - mpol_equal(policy, vma_policy(next)) && > > - can_vma_merge_before(next, vm_flags, > > - anon_vma, file, pgoff+pglen, > > - vm_userfaultfd_ctx, anon_name)) { > > + if (next && mpol_equal(policy, vma_policy(next)) && > > + can_vma_merge_before(next, vm_flags, > > + anon_vma, file, pgoff+pglen, > > + vm_userfaultfd_ctx, anon_name)) { > > merge_next = true; > > } > > > > -- > > 2.39.2 > >