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 6F2FEC433FE for ; Fri, 25 Feb 2022 14:28:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D30FE8D0003; Fri, 25 Feb 2022 09:28:03 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id CE0638D0001; Fri, 25 Feb 2022 09:28:03 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BF6258D0003; Fri, 25 Feb 2022 09:28:03 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.28]) by kanga.kvack.org (Postfix) with ESMTP id B41128D0001 for ; Fri, 25 Feb 2022 09:28:03 -0500 (EST) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 8BA8E12198E for ; Fri, 25 Feb 2022 14:28:03 +0000 (UTC) X-FDA: 79181531646.14.85C593D Received: from mail-pl1-f172.google.com (mail-pl1-f172.google.com [209.85.214.172]) by imf22.hostedemail.com (Postfix) with ESMTP id 13C9FC0027 for ; Fri, 25 Feb 2022 14:28:02 +0000 (UTC) Received: by mail-pl1-f172.google.com with SMTP id q11so4941106pln.11 for ; Fri, 25 Feb 2022 06:28:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=GFLYYQlWg1u4pZYXk6bRReVXN+TJXsDbN2Z5LBQlK4M=; b=p8yJD/xqCtSBUYd+XdI4r7XzAK9wzh0DDyKog2oClXns2LUfEgYqAuqDQNjbYXatnW zbEpuCCf4ph++RLETo4pfNvVRuNqTHEn0a37cTaYwf0/kSB3O3WxQeyiHfohtW/lj4cI 7uKj8hgBtB0syhS+r/YATSgGeN0CgAwsyoT+8Np+nUYN+MI/2aG9mTzc6hKfqkBpUKkq H7mOVbnJtYwK93izxnczrYD6D+7xw7T3V4/iSIItJERkt508KW3Tnmet+UjCnuCoQL02 /wQne9ZBl1VzkgYNH+PX4ntzKltw9/wq6Eyr05OYj4VTib22MfjWdz1bwu93JK+dhHUh Fk8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=GFLYYQlWg1u4pZYXk6bRReVXN+TJXsDbN2Z5LBQlK4M=; b=hJYIB7OZ9qZszV34n1rpiAdnF6actaYuVmR8ksA1RCZ8l+Xidyt9Qtb3NFY25IXUhj J1a6MRT4TCjp/dWU38BVoa/74SvFyXBHJX4adwCCh8wGftrIuqBG+6WaEExeEdeBi299 EUo44KHIfbTVLU/8KtPE5vOw7pZZXsfmMv5ChKSoskhzfBXiPebVdIFXX/YZzgOPbYYW ob3NU/vZNN+hGNr2KZYPKX5xBC/ApS1KfC0tXXRhNgV+9AUl+wLOgcwpNei1fvScYTat Q79ykEf1TEKUrTomOm+6BARRYQHXScMRy6uPS+UTUgmV+Kcaj7Q84UYrNJvhye8e7E/O jS6A== X-Gm-Message-State: AOAM533C75dIBjBRpB2mfsYrTzS09KE+7eXo2zz+F5V1Fxo0m1PVRGHl etL5BXx35Xw3jLeZdWJu3Y4RDngue71pcr4et8s= X-Google-Smtp-Source: ABdhPJzR/AXKvc90+oYK7YNeq+OXjjqBlkF5sdO7ZjaNE7bcasrImCEo+W+cECzj2EjIirRbg3xkF33PwLe2Zwf5EpA= X-Received: by 2002:a17:903:240a:b0:14e:dad4:5ce4 with SMTP id e10-20020a170903240a00b0014edad45ce4mr7833792plo.125.1645799281704; Fri, 25 Feb 2022 06:28:01 -0800 (PST) MIME-Version: 1.0 References: <20220218122019.130274-1-matenajakub@gmail.com> <20220218122019.130274-2-matenajakub@gmail.com> <20220218194338.5sdi7jwusfvh3b45@revolver> In-Reply-To: <20220218194338.5sdi7jwusfvh3b45@revolver> From: =?UTF-8?Q?Jakub_Mat=C4=9Bna?= Date: Fri, 25 Feb 2022 15:26:59 +0100 Message-ID: Subject: Re: [RFC PATCH 1/4] [PATCH 1/4] mm: refactor of vma_merge() To: Liam Howlett Cc: "linux-mm@kvack.org" , "patches@lists.linux.dev" , "linux-kernel@vger.kernel.org" , "vbabka@suse.cz" , "mhocko@kernel.org" , "mgorman@techsingularity.net" , "willy@infradead.org" , "hughd@google.com" , "kirill@shutemov.name" , "riel@surriel.com" , "rostedt@goodmis.org" , "peterz@infradead.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Stat-Signature: 4mbgssf896gqmdpgzdb9n1sebfenxe5i X-Rspam-User: Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b="p8yJD/xq"; spf=pass (imf22.hostedemail.com: domain of matenajakub@gmail.com designates 209.85.214.172 as permitted sender) smtp.mailfrom=matenajakub@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 13C9FC0027 X-HE-Tag: 1645799282-449499 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 Fri, Feb 18, 2022 at 8:43 PM Liam Howlett wrot= e: > > * Jakub Mat=C4=9Bna [220218 07:21]: > > Refactor vma_merge() to make it shorter, more understandable and > > suitable for tracing of successful merges made possible by following > > patches in the series. > > > > Signed-off-by: Jakub Mat=C4=9Bna > > --- > > mm/mmap.c | 81 +++++++++++++++++++++++++++---------------------------- > > 1 file changed, 39 insertions(+), 42 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 1e8fdb0b51ed..b55e11f20571 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -1172,6 +1172,9 @@ struct vm_area_struct *vma_merge(struct mm_struct= *mm, > > pgoff_t pglen =3D (end - addr) >> PAGE_SHIFT; > > struct vm_area_struct *area, *next; > > int err; > > + int merge_prev =3D 0; > > + int merge_both =3D 0; > > + int merge_next =3D 0; > > You set these as true, can you please use booleans? As you mentioned in another email. This corresponds with one of the following patches, but you are right, booleans should be used here. > > > > > /* > > * We later require that vma->vm_flags =3D=3D vm_flags, > > @@ -1191,65 +1194,59 @@ struct vm_area_struct *vma_merge(struct mm_stru= ct *mm, > > VM_WARN_ON(addr >=3D end); > > > > /* > > - * Can it merge with the predecessor? > > + * Can we merge predecessor? > > */ > > if (prev && prev->vm_end =3D=3D addr && > > mpol_equal(vma_policy(prev), policy) && > > can_vma_merge_after(prev, vm_flags, > > anon_vma, file, pgoff, > > vm_userfaultfd_ctx, anon_name= )) { > > - /* > > - * OK, it can. Can we now merge in the successor as well= ? > > - */ > > - if (next && end =3D=3D 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) && > > - is_mergeable_anon_vma(prev->anon_vma, > > - next->anon_vma, NUL= L)) { > > - /* cases 1, 6 */ > > - err =3D __vma_adjust(prev, prev->vm_start, > > - next->vm_end, prev->vm_pgoff, NU= LL, > > - prev); > > - } else /* cases 2, 5, 7 = */ > > - err =3D __vma_adjust(prev, prev->vm_start, > > - end, prev->vm_pgoff, NULL, prev)= ; > > - if (err) > > - return NULL; > > - khugepaged_enter_vma_merge(prev, vm_flags); > > - return prev; > > + merge_prev =3D true; > > You could set area =3D prev here and simplify the if statements below. Thanks I will. > > > } > > - > > /* > > - * Can this new request be merged in front of next? > > + * Can we merge successor? > > */ > > if (next && end =3D=3D 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_nam= e)) { > > + anon_vma, file, pgoff+pglen, > > + vm_userfaultfd_ctx, anon_name)) { > > + merge_next =3D true; > > + } > > + /* > > + * Can we merge both predecessor and successor? > > + */ > > + if (merge_prev && merge_next) > > + merge_both =3D is_mergeable_anon_vma(prev->anon_vma, next= ->anon_vma, NULL); > > + > > + if (merge_both) { /* cases 1, 6 */ > > + err =3D __vma_adjust(prev, prev->vm_start, > > + next->vm_end, prev->vm_pgoff, NUL= L, > > + prev); > > + area =3D prev; > > I don't think you need all three booleans since merge_both is only used > here. This again corresponds with one of the following patches, but in this patch merge_both is not needed. > > > + } else if (merge_prev) { /* cases 2, 5, 7 = */ > > + err =3D __vma_adjust(prev, prev->vm_start, > > + end, prev->vm_pgoff, NULL, prev); > > + area =3D prev; > > + } else if (merge_next) { > > if (prev && addr < prev->vm_end) /* case 4 */ > > err =3D __vma_adjust(prev, prev->vm_start, > > - addr, prev->vm_pgoff, NULL, next= ); > > - else { /* cases 3, 8 */ > > + addr, prev->vm_pgoff, NULL, next)= ; > > + else /* cases 3, 8 */ > > err =3D __vma_adjust(area, addr, next->vm_end, > > - next->vm_pgoff - pglen, NULL, ne= xt); > > - /* > > - * In case 3 area is already equal to next and > > - * this is a noop, but in case 8 "area" has > > - * been removed and next was expanded over it. > > - */ > > I think the above comment is still true? Yes, it is. It just slipped away. > > > - area =3D next; > > - } > > - if (err) > > - return NULL; > > - khugepaged_enter_vma_merge(area, vm_flags); > > - return area; > > + next->vm_pgoff - pglen, NULL, nex= t); > > + area =3D next; > > + } else { > > + err =3D -1; > > } > > If you initialize err to something, you can drop this else. Thank you, that sounds better. > > > > > - return NULL; > > + /* > > + * Cannot merge with predecessor or successor or error in __vma_a= djust? > > + */ > > + if (err) > > + return NULL; > > + khugepaged_enter_vma_merge(area, vm_flags); > > + return area; > > } > > > > /* > > -- > > 2.34.1 > >