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 2775DCD3431 for ; Wed, 4 Sep 2024 07:58:26 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AF7248D0235; Wed, 4 Sep 2024 03:58:25 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AA6AD8D0228; Wed, 4 Sep 2024 03:58:25 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 96E748D0235; Wed, 4 Sep 2024 03:58:25 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 79B908D0228 for ; Wed, 4 Sep 2024 03:58:25 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0085340E20 for ; Wed, 4 Sep 2024 07:58:24 +0000 (UTC) X-FDA: 82526303370.23.1F2634F Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by imf13.hostedemail.com (Postfix) with ESMTP id E997D2000F for ; Wed, 4 Sep 2024 07:58:22 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=JpKzeLDI; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.47 as permitted sender) smtp.mailfrom=richard.weiyang@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725436679; a=rsa-sha256; cv=none; b=TjZ+zwPmsp2LmackJNqu86255mV7Zga3x1sltYtmyiSykpFWX/bnZOIVASDIzl1kkipq+M +HdRTzjqdmFJbjyKIWv2Jtqxc/Xs+8rweGHNwo3QDSv39mayq4kxTwWpjzEV4ISSwqODjq PTSlGR9eDyzWwm+sfTKxgV4cUoRgviY= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=JpKzeLDI; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf13.hostedemail.com: domain of richard.weiyang@gmail.com designates 209.85.167.47 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=1725436679; h=from:from:sender:reply-to: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=MpOMOSsibb2/jLZc/3QrQ/4hlE4YxiU0LFAgZk5YXkw=; b=GkbdUMXeJdhQdsIkro7kmnYv3J6QGeqGqrbMIG40FrMV+ae19M84GEqwrh0fisv9jbxU2N pWHARYPnyDU74Bd8O6+TTB2r8Vek1hvYQDSA7eNCMqXMZ31Y8GJWSFzs8NrSV7YnFzcjvR VcQN6lZmQVbEkOfSqwW/GY0layLU708= Received: by mail-lf1-f47.google.com with SMTP id 2adb3069b0e04-5343617fdddso10766820e87.0 for ; Wed, 04 Sep 2024 00:58:22 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1725436701; x=1726041501; darn=kvack.org; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=MpOMOSsibb2/jLZc/3QrQ/4hlE4YxiU0LFAgZk5YXkw=; b=JpKzeLDI51yH9wswy/MclETdEGtAb02XFoYmCGhY0/R6WqXOaMBNEDjbcpNCuNzx4K lyGihxfZd8eSy4w9Kgtv3L7760EZxxfh/75Azordu7NC9VaHTBUjfmxBiHjYi0g9GizG 7cNQXfFkPdt8IcYSoEQ0QIFFZz4Iu23b4Z5WojrmYXgiHh2zlfIkzBL2OzRt0uJnzttj bUyR+I2vrjVLCsnx3eXwtDrxjB/UyaSW4W9CCYiibtxX4Mh+eZX0F9v8l579TfBoFyJ1 YtxdoGNx5MGprvLA1qQxPgT3ArZ+VkwUrmJYrqr+dAyw3hhubzPEfX90lLKtSZo9fGKJ L0bQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725436701; x=1726041501; h=user-agent:in-reply-to:content-disposition:mime-version:references :reply-to:message-id:subject:to:from:date:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=MpOMOSsibb2/jLZc/3QrQ/4hlE4YxiU0LFAgZk5YXkw=; b=Xwh+o4d0o/oU4DqDDUnGMS+xNoh/SeuX7bt+xRjY4hKr3RH90dtcAxSooDYtflgvY7 FFCIndB4hExIKWMzyef2DVGbpqcpo6SCxvFF2DFv7pw9qXYdORQzhQPMcVfhDBl/uWwd a4K5pLnBBJxu7vDtThYFiQHFVEXPS6Mm6mWlQpPfBoMToh9rro2o6JgifhgcOBgN9rCr iYveBLrqtedMvOXAfoG8IT8VTff/9LmVSPzCIwo0tKQEZAFZ/ZRf6UTqn3cskSiWzw5A 7F4MBvkTI9IoE4ksd87cu6W3Tmn6t1uInIx1Ls9OPteZq2/WIgUzvK2iln3My0hCcnot KUMg== X-Forwarded-Encrypted: i=1; AJvYcCWv0HvsTbW66/RSB5HAeErJ6d+1gLVXcDBYwTg2cyGSsNDGLUSjghBqqn6iLXigVSEpUVoLpTl7dg==@kvack.org X-Gm-Message-State: AOJu0YzVDwMOlla9P32T1MbYXxlCl1GyM0nEAwhfYbi8OTE18qkvt3jw xJepEreD+N8QK6HbvEeeDdf1lj2vAgdASO5AaP2w/oE6Cz2QYwMu X-Google-Smtp-Source: AGHT+IHxQq45iNobsFZRuIg/sJCDEpg5IGuFcofR1keCY1cmPdT24TZmm+1aapwd5Ap6LkslAowGfQ== X-Received: by 2002:a05:6512:440a:b0:535:681d:34b0 with SMTP id 2adb3069b0e04-535681d37c9mr831478e87.47.1725436700680; Wed, 04 Sep 2024 00:58:20 -0700 (PDT) Received: from localhost ([185.92.221.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a8988feb2a3sm778579666b.3.2024.09.04.00.58.20 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Wed, 04 Sep 2024 00:58:20 -0700 (PDT) Date: Wed, 4 Sep 2024 07:58:19 +0000 From: Wei Yang To: "Liam R. Howlett" , Wei Yang , akpm@linux-foundation.org, maple-tree@lists.infradead.org, linux-mm@kvack.org Subject: Re: [PATCH 1/3] maple_tree: use ma_data_end() in mas_data_end() Message-ID: <20240904075819.5vgnelkxxj7myyn4@master> Reply-To: Wei Yang References: <20240831001053.4751-1-richard.weiyang@gmail.com> <20240904001525.5zshbivv7op4ana7@master> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) X-Rspam-User: X-Rspamd-Queue-Id: E997D2000F X-Rspamd-Server: rspam01 X-Stat-Signature: z4mc9d7xg94tfwb4xpdbwd15kg7pq4fm X-HE-Tag: 1725436702-502926 X-HE-Meta: U2FsdGVkX1+L6mcsxc/FtcR3QRC1XcfwqC5frh2bhFANaLNUIlKma2bRdcxYWnh8eE7QCtshdviybhQ1XgjvtntUfXpXByq6HoikUGP5bumCq8z448NvGg2Arw4nvJrjSYMxPFxqpSk+ehBI27y/B8DHTKq4wDYbFEGIO+Ims7Bzek5SnDZDjgOjzq05a5F7ZOfMfeD3I7pEv9IxHjdvOZfqKspn8JTJuqqYL2OzffqkS0Ss+fwviDflzortftuTlSUyCzML8ZrSP0XXAjbiMPSMAA9lk9OhOHFl54NRtFiZTG0kiNZctroaIsjfMMjVsKLkqWiTnGOnpr6h6Z19VUFMp5WewU5LJApYAwkTk6/5U0CGhJNjXaG5C7X/6agl1LmIBcsJ6gnD5elzlDveDZ9RLDKMgB2SpoiKh5mUy3Bpa10oc+4McC33c9g8Y1QKVUzUzHXEG/5PFVYCxmGdZWXhGVgxcJT+63eMo8A/cdTKsL8edNBJsnfhIeO/Oqba3SPmhawv9oeKpH4P8wofGMGrlgNq9dx1mahJvJIoke07Hr+ZDyO2OloC/WHf4Bk+e6Vl4cT/ZNmPwVhXBFCKq1Dhy+vyESc3nRitH6ZDgtp4hh66YdXWvs61pfCGe6tN6BqZSL9E8DHHz6HDHA1ISoQ46hzxx5DpkwriOWdYfXoJGYrLe0TpJAwmVg7ts2yFOrI2wVSwwS7YYhBf3uLcrSUT/VEFQbqyJ87vXIJ6Tc4b3HO4RyaiJGML27V8kwtKxtsPxxbTY42lf90WPX11cKp4sfALPgCIUgnguyrsGIpEOmJn6a4A7Uo4bgpBYtvdv5ZgGnQpJ3WMriPUSXTGeQHSW/h+59DXhhvT8se0p4plQhbBcvn+nDFEMi3o28Kzg2JRs6suGLFSEl5vfuE8j218JvHPMhaW/wb6jsrmikyr8ZBQnL5/xgFRgJQLxlvIBw08SExRtX792yciUYY r2ajRUOp DS1eQH0ajj6uMp5sQznf5qB/3mjrhzFi/ltGLOtQOnmA0WN0NTqlT4Ex2oyrXG0sD3DH+ybMcvaPwAo8EY/KNUJeQn1XHfhzAuD5Mq52c/jfqVAVIhf2kgDRV4MDhFQqYOCVc1UfzLJOS9s841seeHUFjgLQd8hnsbWBDmi6He9mjjChjFghZ+fnATejfaFWemKTbO+bnn3rAnIG5nOtBOzl5dcpZwQILSZm5ss+QvSh5yHoB8UWb5c3uRO49cNpfQ2UI3R4GztaVK0yCYGcMRhCyZ08IZMyMtNrx09vGf7s/JcQtb1IWW9hR/ggEgqtfVgCPbxifdAtXnVwpAvE8xKAOrrbynIUHiF6ebGFXn67n67Tp7BMo1m6+UtPmpigtbrx0nlxV76EbElpQ1gfjSIYubmWRdjHRKTdB6ilU50QCL91i3rV5VFoiFMlrdejfr2g6s91/P95vWBbH6+oFm7oTQNZ0jlCrlDxBIS+GxMQTWwHf+7I3sFL29uGA4ZO/wcT7gH+p15dG98j0hyjtd/QcyUyZJDmfSuubqIdBbiBLA+mYrOiqKf9V433G3zopjQ4gpwYE+mIueDFkugWAFPztFPQTrx9Kn/LYF4bO6pB0D1ErToDMYh0scB7cn78jM7Tm17jEZ/C9sdkgT0EZJQByUCx5QYzn909O 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: List-Subscribe: List-Unsubscribe: On Tue, Sep 03, 2024 at 10:25:10PM -0400, Liam R. Howlett wrote: >* Wei Yang [240903 20:15]: >> On Tue, Sep 03, 2024 at 12:12:39PM -0400, Liam R. Howlett wrote: >> >* Wei Yang [240830 20:11]: >> >> These two function share almost the same code and do the same thing. >> > >> >Please stop trying to optimise code like this. I don't have time to >> >verify you are not making things worse and you haven't done any of the >> >testing required. >> > >> >I went through with perf to optimise dereferences and static inline >> >functions, so unless you are prepared to look at the generated code and >> >actually benchmark, please stop sending patches that I need to verify >> >like this myself. >> > >> >> Would you mind letting me know how could I verify the generated code and >> benchmark? What you expect to do? > Liam, Thanks for your patient reply. >There are a number of benchmarks in the test code that will run specific >tasks in iterations to check that there is not a regression. Depending >on what you change, you have to run the right one. Usually I write one >if one doesn't exist. Not familiar with benchmarks. Would you mind naming someone? or the ones you have written? So that I can learn to test or write one later. > >You can also use perf record and see what happens on specific tests. > >This function is all over the place, so I really don't know which one to >run, and I don't have time to write a test for your changes - and I >don't think this is worth it. > >> >> For example this one, these two functions share the same code. > >Do they? > >You have removed the fast path for maple_arange_64 using the metadata >end before getting the pivots, which means on basically all vma walks we >will be adding instructions to the walking of the tree (get the pivots, >check if the node is dead, check if the pivots pointer is null). You >have also added a check for if (!pivots), which is essentially checking >if a dead node was hit - which is already checked in the mas_data_end(). >These two checks are the reasons I left both copies of this function in >place. I am looking to reduce the execution time, not decrease the line >count of the file. > You are right, I should pay more attention to the fast path opt. >So, the last 8 lines of both functions are the same. And please don't >submit a patch that adds an inline function that does the last 8 lines >to reduce duplication. > Ok, I will not try to call an inline function just to reduce some similar code. >> What's your >> concern for this? The inline function expansion? > >Your clean up is compressing setting variables to the same line, which >is a bad change. It is better to have verbose code where it won't make >a difference in what is compiled. And certainly not worth adding after >the fact. > >The inline of the mas_data_end() function depends on the expansion >happening by the compiler (which might change based on the version or if >it's gcc vs clang), sure that's a bit of a concern. > >The biggest annoying thing about this whole patch series (without a >cover letter) is that it isn't doing anything for fixing, helping, or >adding functionality. I'm a big fan of forward progress and this isn't >it. > >It is only changing code for the sake of changing code. And it looks >like it will be slower, or the same speed if we are lucky. I have to >take time to verify things aren't slower or add subtle issues (maybe an >RCU race) because the code looked similar. It's just not worth it. > I am trying to make the code more easy to read, but seems not helping. BTW, I found in mas_update_gap(), if (p_gap != max_gap), we would access the first parent, parent's type and its gap twice. Once in mas_update_gap() and once in mas_parent_gap(). Do you think it worth a change to reduce one? -- Wei Yang Help you, Help me