From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yw1-f169.google.com (mail-yw1-f169.google.com [209.85.128.169]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7AC16346AF5 for ; Sun, 11 Jan 2026 14:24:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768141470; cv=none; b=R4MOjKjw642JBdg6vxCmNa3uuwzTWc6jJKH0OG96QDt6fdiwgTBZbxp7Wtwf6kblkyGfvS2UnVyWoc3xeEvLKfZz7KmKyNk8935LrHIUF1CFUHyo5zWfu95kdUpFJ3NEDA6MKXLUIURAvoXZjVkrvaAoIf2QzI4QTqDg0g+ck6M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768141470; c=relaxed/simple; bh=sBhywfVPei2QMvqIWLEYydd0Rmkaq7hyhq+nE3hFuRk=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=mAWkGfiSH/HLmBD9PY27nnNslhUhd80GR9/BxpFNtppEb1kRtU9i+6IlX0psYMBRBFFyHTCpyxXbN0mhLjoIe3WOnPVGVbp/BCc9LJ5DwAg2YWil2Yy8FhaRKdn7aOr2ZOT9oGAT9J+0VJe8+ESujfazvS5gFEouUJLhk30PMOA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=WBJlGw0C; arc=none smtp.client-ip=209.85.128.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="WBJlGw0C" Received: by mail-yw1-f169.google.com with SMTP id 00721157ae682-7927541abfaso7946697b3.3 for ; Sun, 11 Jan 2026 06:24:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1768141467; x=1768746267; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=sybVQVvIUZ2xnQZHDweZkMjQ138E45xUi2gly0/jMW4=; b=WBJlGw0CrWuuQOW0b1J6CWmui40Nb+r1tV7glQz/0tOBkmIRIHCXsRRKsGLZyVDa8l KQfmfhM2sE0AuOYEfT1F4khTf5CpwzX/2i2edkcoajJfhSduO8lq2tl93Owid/VFdc4S jNIci7zYCDqcrDZU87cu2T8to2myTl3HXUp9xeNSV/ZQ1NVgVzxjq431gK7Ss/Y+q5Ad IAtpUJ/jqDUAE7znXW3Wwu1mQgEE0ZI5dYKymNAaklB1f+hGv+0OqGVMOxXWU1RN6OkR 7GnKhCfoDz96aBufW0ecKSVNd+VDGVzGd6/kmkP5J8stlIzr6b0j3Rd0KEMcF3i7ksai H91g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768141467; x=1768746267; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=sybVQVvIUZ2xnQZHDweZkMjQ138E45xUi2gly0/jMW4=; b=xC5Q/d5KM2TjfgRRMguQxqjuy/NVD40+DGNVkA8HoWw/Ma8fRnt0V9ZRtnHpFUBWkP JfHXXpvPmU64cM6rdXcLjUpSfjtLR2vsu+fRBSWGkGZpWWlDCCztXMENGhK11V299Cvn A/HkNzfTpkJzxc35ESIsRm+Cms+zouJnfgS3sy7QYfl0WB1aqlO2MoqOOhZ0kfvJANBs c+dmCwR/de+Jqu7EsEuH3gShoIRaAQlkmb7yAUExjx+6wWdbcu2W/t1n35N2yTPETgmf HxkvynJLjYW3/g5g0dEN+UuIqbwg6ncxpnTCltXMSRLliWc13wT7U5/WojCnpx4DAhuY G76Q== X-Forwarded-Encrypted: i=1; AJvYcCW4bJlInZGomLMWxrjJLAPP5U5nokKzFAGZcRggFCT7+EqC3792CzKVL99/zdAwSpBIyKlUHJuiDRC4DRk=@vger.kernel.org X-Gm-Message-State: AOJu0YyJ4Olk5K91CxcMt3ThfYi6H7ZpWjAv9v0DEeQ2fWS6Vx1/ZJjA uI95xw1nRFblODfMP8l+0gd3b00ZS6jCoyDfM64BHiLHEUSGkdx4xN2E X-Gm-Gg: AY/fxX6XEv/Mz3zn+PwH1k5HUnbnnW6o3DtPVhLiyrKPit0ff43iMw9vGKJb5RimKW8 Qt4OEFL57x9wnRstpXrpZQcajcyHiGE2w711fKHoUD6scFs+EcELoTs6eVgavWU0WoXyzRj1GD/ V/2QirTtDw37m1s1Ej64c3qbbiwlCSd8/4dZCzAID01wZc5N1Fy0zKJwpW+AF2r58Nx8Ys5oqPP 1sgTCOjfdQaBhMCCKSATofJLSYi1dWLuIpUqFAI6dghXXT9/iCw0RXMzttmC86QOcPeuToe1g37 JPrJ4G04jE4k3/QXRGISZgX5W5puLCVCg6ya4AfGj890DWyutpM78a+fccXMKoGdw47sbEiJN8o NdV5TAJT3/8vHObl7gl/3P2dw+Vjm11GhNUoY2HK3QMQVocQfDOVmgctFAIgHieaxFbI7EPpwtQ Om4SK2myG12Q== X-Google-Smtp-Source: AGHT+IGuOhyZ7k2UnW/gdq3Y10926SeAcysmT1Uz7hzyT0YXQCinQlOxp5RCgC6oy22dMjXvaxJNWA== X-Received: by 2002:a05:690e:1209:b0:646:7ad7:c85e with SMTP id 956f58d0204a3-64716ce4a2dmr11797959d50.85.1768141467392; Sun, 11 Jan 2026 06:24:27 -0800 (PST) Received: from localhost ([2a03:2880:25ff:50::]) by smtp.gmail.com with ESMTPSA id 956f58d0204a3-6470d81260asm6932271d50.10.2026.01.11.06.24.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 11 Jan 2026 06:24:26 -0800 (PST) From: Joshua Hahn To: Yajun Deng Cc: akpm@linux-foundation.org, vbabka@suse.cz, surenb@google.com, mhocko@suse.com, jackmanb@google.com, hannes@cmpxchg.org, ziy@nvidia.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm/page_alloc: Avoid duplicate NR_FREE_PAGES updates in move_to_free_list() Date: Sun, 11 Jan 2026 06:24:21 -0800 Message-ID: <20260111142425.2783953-1-joshua.hahnjy@gmail.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Sun, 11 Jan 2026 21:47:42 +0800 Yajun Deng wrote: > > > > 2026年1月10日 00:31，Joshua Hahn 写道： > > > > On Fri, 9 Jan 2026 18:51:21 +0800 Yajun Deng wrote: > > > >> In move_to_free_list(), when a page block changes its migration type, > >> we need to update free page counts for both the old and new types. > >> Originally, this was done by two calls to account_freepages(), which > >> updates NR_FREE_PAGES and also type-specific counters. However, this > >> causes NR_FREE_PAGES to be updated twice, while the net change is zero > >> in most cases. > >> > >> This patch introduces a new function account_freepages_both() that > >> updates the statistics for both old and new migration types in one go. > >> It avoids the double update of NR_FREE_PAGES by computing the net change > >> only when the isolation status changes. > >> > >> The optimization avoid duplicate NR_FREE_PAGES updates in > >> move_to_free_list(). > > > > Hi Yajun, > > > > I hope you are doing well, thank you for the patch! I was hoping to better > > understand the motivation behind this patch. > > > > From my perspective, I believe that the current state of the code is > > not optimal, but it is also not problematic. account_freepages seems like > > a relatively cheap function (at the core, it's just some atomic operations). > > Personally I also think that semantically, the code currently makes sense; > > we are doing the accounting for the old mounttype, then for the new mounttype, > > in a way that cancels out. And given that there is still some cases where > > the work doesn't end up canceling out due to one of the mounttypes being > > MIGRATE_ISOLATE, I think that there is enough purpose in making the two > > calls to do the accounting twice. > > > > On the other hand I think there is only one place in the codebase that > > will use account_freepages_both, so it might make the burden to understand > > the code a bit higher. > > > > What do you think? I don't have a strong stance on whether the performance > > effects are big here (if this change indeed has a big performance implication, > > then we should definitely go forth with this!) but I do believe the current > > code is quite semantically sound and more readable. > > > Hey Joshua, > > Thank you for sharing your thoughts. > > I currently don’t have any performance data, I just noticed from looking at the code > that there may be room for optimization. > You’re right. The original code is indeed more straightforward. I think we can add some > comments in the account_freepages_both to make it easier to understand. Hi Yajun, I hope you are doing well! On second thought, I did notice that at the end of move_to_free_list, we have some additional conditionals that depend on the migratetype of the mounttypes. What if we open-code the account_freepages_both, and skip doing the isolation checks twice? Your idea to use the ternary operator gave me this idea! @@ -869,14 +877,17 @@ static inline void move_to_free_list(struct page *page, struct zone *zone, list_move_tail(&page->buddy_list, &area->free_list[new_mt]); - account_freepages(zone, -nr_pages, old_mt); - account_freepages(zone, nr_pages, new_mt); - - if (order >= pageblock_order && - is_migrate_isolate(old_mt) != is_migrate_isolate(new_mt)) { - if (!is_migrate_isolate(old_mt)) - nr_pages = -nr_pages; - __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, nr_pages); + if (!old_isolated) + account_specific_freepages(zone, -nr_pages, old_mt); + if !(new_isolated) + account_specific_freepages(zone, nr_pages, new_mt); + + if (old_isolated != new_isolated) { + nr_pages = old_isolated ? nr_pages : -nr_pages; + __mod_zone_page_state(zone, NR_FREE_PAGES, nr_pages); + if (order >= pageblock_order) + __mod_zone_page_state(zone, NR_FREE_PAGES_BLOCKS, + nr_pages); } } I don't think it matters that we reorder the __mod_zone_page_state to be after the account_specific_freepages here, so hopefully it is OK here. So we can achieve the best of both worlds by preventing the duplicate adjustment and also keep the control flow simple! (We can also just include that additional check inside your account_freepages_both as well). This is just my small idea : -) Of course, please feel free to ignore it if you feel that it makes the code more confusing. I think that what is "simple" is mostly subjective, so this was just my thought. Thank you for your thoughts, I hope you have a great day! Joshua