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 8698FC7618A for ; Mon, 20 Mar 2023 17:09:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B1DA26B0074; Mon, 20 Mar 2023 13:09:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ACE066B0078; Mon, 20 Mar 2023 13:09:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96E266B007B; Mon, 20 Mar 2023 13:09:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 8364D6B0074 for ; Mon, 20 Mar 2023 13:09:47 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 1C34740ED5 for ; Mon, 20 Mar 2023 17:09:47 +0000 (UTC) X-FDA: 80589913614.26.ED29EAB Received: from mail-wm1-f45.google.com (mail-wm1-f45.google.com [209.85.128.45]) by imf30.hostedemail.com (Postfix) with ESMTP id 4B1A38001B for ; Mon, 20 Mar 2023 17:09:45 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=E9AQrjlO; spf=pass (imf30.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.45 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=1679332185; 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=rGCOrM4FMzTeub5/ravYucAeRNAx4GgviWtRpjAsQwg=; b=zDKLyo2BCn2cNaNAYlYcm7va/ScCVTkpXj9tkrbiI8iMMfl5Xo3X6X9sS9IUQTuI71mzEZ TSl4b83bwJNFzVhjTbkUbP4qXBjlSTWcInDQVcAmNdo/lqH7C6vPXcKyow26v0OjUDvuWJ 2dgppBchTtphZ3C3Ig9U3ht+wvgsNqM= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=E9AQrjlO; spf=pass (imf30.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.45 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=1679332185; a=rsa-sha256; cv=none; b=6KicynjM36A7brguGdjYfmDAWkIx4QzSxLLUG0oECrFgGKpQ3E/GPpr9DfVwyF43rZuPX4 Q1RRVM0OnGJv1xu/qWbH4sFQbYw1cL7hDfcP2E2pNARqYtzCb9TuMXTWFzudL9G79u9iOj Dc5KbrBdHAx+qeDBqbV+4msJq370I7o= Received: by mail-wm1-f45.google.com with SMTP id r19-20020a05600c459300b003eb3e2a5e7bso7999044wmo.0 for ; Mon, 20 Mar 2023 10:09:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679332184; 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=rGCOrM4FMzTeub5/ravYucAeRNAx4GgviWtRpjAsQwg=; b=E9AQrjlOsm+l+iOQUD4pwk+AqM5R1bYmm58QOXDqblfoHm3MXvqAnhkeZN1+9EJwx7 Aao5iFJHH0B+mn/jwjGJmeso7WXJ5F2aZyjaUtvEkl5jVMNpvuSMGyPsyTziEhgtTFAa BuolEZoDnl7tyKvrhf0Rf85bDtLcuczss0wphoiDB+N0FVSaPh+AE29x/qfvOBpXr2mc WnbcTA7ZTvbvYc0lQ1M3AhAbtHk8EfDzhEe2fiWZFxJ1inn4a8kVx2BStf/uhiHHRC+w CNp6Sn25eOTd3ZUqxtUCAfXDfIWMutOmjzzMtKSkHmICuvZPEczGURMj78dS+DfQCBHo eSBg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679332184; 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=rGCOrM4FMzTeub5/ravYucAeRNAx4GgviWtRpjAsQwg=; b=qGFFa5QekZtWeqo4DctDAwK0cYmRvj8C4EaFvgJDUsMbpHgY/3yFIohGvXc6aMnSyN wRs8N9QNW5hd+/lhi64CaZEiRTwAT3HYcAqi+9/neTbTnh4PhL1NYKVkJrc+aCGN6Wc3 /zKt3HoZzWNVQbNgDpCTwppsxgokf69JszRca9N3JVeUPfwEey+sltVDfTPAWS7nijh8 wfeNBaXYN8NB4C6W4nfoef7XJjjHV2YLYbR4ehvLSIciBCoTKNItSzk6idSCise63XaD zxcTrnfwWCxybNnr+oYB8KJhFQaWJ/mQU4FFJPs9kwd7RHjIKpEQIQg0D/ucWFMpo3Rw 8Fdw== X-Gm-Message-State: AO0yUKU2wyE/I/46mWVcLLENDkkCdqcqJ+LJwFbeErDpC0VwZ6F6HfgW lgpiGWd05nm9oN/PqfJpS/U= X-Google-Smtp-Source: AK7set8Zn5kiOPJpzlqDnRjE14OR99eyKESo6ny4jIJtj32Vsx+m/y8wwatZPjlZILI0qLL/qeeWlw== X-Received: by 2002:a7b:c7cf:0:b0:3ee:12cd:dfa7 with SMTP id z15-20020a7bc7cf000000b003ee12cddfa7mr221076wmk.13.1679332183603; Mon, 20 Mar 2023 10:09:43 -0700 (PDT) Received: from localhost (host86-146-209-214.range86-146.btcentralplus.com. [86.146.209.214]) by smtp.gmail.com with ESMTPSA id ay38-20020a05600c1e2600b003df7b40f99fsm16986271wmb.11.2023.03.20.10.09.42 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Mar 2023 10:09:42 -0700 (PDT) Date: Mon, 20 Mar 2023 17:09:41 +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 Subject: Re: [PATCH 0/4] further cleanup of vma_merge() Message-ID: References: <20230320164707.zpjhcwplkrp4tvgf@revolver> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230320164707.zpjhcwplkrp4tvgf@revolver> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 4B1A38001B X-Stat-Signature: ep7hf8agf4dcgtw1woqees9arbq4joyt X-HE-Tag: 1679332185-771238 X-HE-Meta: U2FsdGVkX1/cvxnSgd6S7trh1hYMc/A20upWlZylmXDkCMhmy3p+GWlttdyO8Xi3Mv1ve8bUC3X6iGn3BhIigmsRtR34QvpeQKUBiMBhd7qRitqlxp0GRVTHzGydBK3VLhXsUQOjDVRQ0OQx3T3qlON3PZEU2q69Uev5m6DqE7tuR4TXXwhdRvx1wbynXhzDZ9H6ymGiTPN2Esm6FrTSp3c5132Gz07iN01S9wFjtML6XQaOouXD4etlteQG+Ictcsyr/ZF3sBysGwjigY2yvMYlavrgvGwL/WLu4yUZQzVy0FpqyRUPQJ/a56ALC/I7I9ZzT2onrBy3wUJpdBKjpX/HJIip9Oc2z0OF1ZO3ZYyLAIFy6Qz1eEgJyLzGyWJyBv9dqYy6/ji2dRqBdbj6gW0BiKhUdU650ZdrigmbmXE58jBgS0A6dS4BrfdsxEd6Xkcxceru/b0ohJZJxjltbtLb2wOZ4ySKFamZRzUZP1uFuwlyrK7Nh0XTtqVXbZC10HcwS5MWKRa35x05JWHkJ3GRg8ILQKKAgjMlbf7oRD9ip6EtEvGR6XMgEVyni1IgvnDy20pONEZWMy1yshHoMyNyRwY17E3glkXFPQcGjjeSqJCYnfrs7v3P7Blbcsi+i9cMfijwLz280ZN0RSJgEL+qiZ8RA2RjlUoQPRHmFRa1DnmSI4xJge0wEffgnF2cXsoV8LEk61+Jb24jBMtenRs1kNk3J38nvGwapyNI1xy+Fy/BgSxXny6HzyeQZTqh3lT4cFq85j7vNaRk+S2/vEaLrk41NLyrUYpSLSLa4DT+LaT3mkbRYeiBKdnXx1Sfm8vscByP2nUwSVa5AJirxHxW1wQsU8pxbk5LI+7WDx374cGROd/wmNY/Oc1YeXZ+gbPYT5xRfcoxgDUzdHTMaavx5eZjVTex6DDufWm33x9XMmTX8Z97JjuLGTedP6VoxpcZNQo5uCk8XJpm1+N JJAD5JmN F/0n/EbAlmdY3Vl557eIXsZCKeiD/6VCOdnErW95qU7U6QG/FqBFRYpY0gIEx57D+MOzMTIAjwKBhrSY5tI9WpzLbYyu9PetSIBHM4BJfEFb8YSkz9v+tqJcF1yttp9Jb25wpJFPY7fu5g2C9/OtFIhEwEx6DkpQEwllKWhIxwQeCbJeToi4SUWRW+Qj/v03PruKMgywYdD0/19weSjGrDyw9vR5Ped5iuy82BbYaCotzgmS5OMzPeUs8Z7SaO1iXUA9obYQuR8xxNjPEuhDfYAm+zkjtnxgw7lh57pWEf0Wr9v8YZg5phc1jnN3B8qQ//YLohV4cARkrRxp69MGapsnQCh9Mz4fjjtHwslCQMG57Agk4JIph6c19ikB1mXmIajDmlidxZEQkM9m46HUvWUdChVwdf5nGMfktfAwXWI++2lta27ru8lfQAFd/B1UP345jgDrLzQJc96QW8xP5+YrzdGsjYHfZvgDO 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 Mon, Mar 20, 2023 at 12:47:07PM -0400, Liam R. Howlett wrote: > * Lorenzo Stoakes [230318 07:15]: > > Following on from Vlastimil Babka's patch series "cleanup vma_merge() and > > improve mergeability tests" which was in turn based on Liam's prior > > cleanups, this patch series introduces changes discussed in review of > > Vlastimil's series and goes further in attempting to make the logic as > > clear as possible. > > > > Nearly all of this should have absolutely no functional impact, however it > > does add a singular VM_WARN_ON() case. > > Thanks for looking at this function and adding more clarity. I'm happy > to have comments within the code, especially tricky areas. But I find > that adding almost 50 lines to this function makes it rather hard to > follow. > > Can we remove the more obvious comments and possibly reduce the nesting > of others so there are less lines? > > For example in patch 2: > /* > * If there is a previous VMA specified, find the next, otherwise find > * the first. > */ > vma = find_vma(mm, prev ? prev->vm_end : 0); > > Is rather verbose for something that can be seen in the code itself. > > I think we are risking over-documenting what is going on here which is > making the code harder to read; the function is pushing 200 lines now. > > > > > Lorenzo Stoakes (4): > > mm/mmap/vma_merge: further improve prev/next VMA naming > > mm/mmap/vma_merge: set next to NULL if not applicable > > mm/mmap/vma_merge: extend invariants, avoid invalid res, vma > > mm/mmap/vma_merge: be explicit about the non-mergeable case > > > > mm/mmap.c | 165 +++++++++++++++++++++++++++++++++++------------------- > > 1 file changed, 107 insertions(+), 58 deletions(-) > > > > -- > > 2.39.2 > Sure, I did try not to overdo things (once you start simplifying you can go too far), but it seems like I _did_ go too far on the commenting (perhaps pushing too far the other way). I will simplify, remove things implied by the code and strip down + respin.