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 B836FC0218D for ; Mon, 27 Jan 2025 02:59:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E59042800D9; Sun, 26 Jan 2025 21:59:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E088C2800D4; Sun, 26 Jan 2025 21:59:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA90E2800D9; Sun, 26 Jan 2025 21:59:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id AA5122800D4 for ; Sun, 26 Jan 2025 21:59:10 -0500 (EST) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 2F1D41C9980 for ; Mon, 27 Jan 2025 02:59:10 +0000 (UTC) X-FDA: 83051725260.11.4E8E983 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by imf01.hostedemail.com (Postfix) with ESMTP id 58C7240006 for ; Mon, 27 Jan 2025 02:59:08 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=4PlygSnU; spf=pass (imf01.hostedemail.com: domain of hughd@google.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1737946748; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=gY7whz075gF2oDUVkEoQRi0JP9AZVP1TZQbx76u1kos=; b=RCb0Hqtvx3WpT6EiXXbLALzRLX0Z+vyxq17NL8QfJYsUjuDYlB0aO9yxWGNKabla0/LuIN aQsNVYpauLiOjww9LDnw5UJHmgg1+HSmrMJ17SPTTVWY+BaLKrBYT9wXCcedJjXPGyCc0y HLYytaKcfZkV7PEJuNaREEiVOmLldUk= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=4PlygSnU; spf=pass (imf01.hostedemail.com: domain of hughd@google.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=hughd@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1737946748; a=rsa-sha256; cv=none; b=BAG6daTt0c52jAealUeNyvqH+tQGqxLLKHDUNl3t2ly7CXs0kUUuljtbt6lPyc5UIIQVgC p9fzhHA7hNU8J/hAVmXlCfWbcv2s0NFIsJvPoarU/rO3Ky7BlpsWXMROFeWT8itVR5D/gt t10j+G0y4YLYXHpLHW1M0EtFsf+uwQE= Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-21628b3fe7dso66774945ad.3 for ; Sun, 26 Jan 2025 18:59:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1737946747; x=1738551547; darn=kvack.org; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to; bh=gY7whz075gF2oDUVkEoQRi0JP9AZVP1TZQbx76u1kos=; b=4PlygSnU3EUQIfZw7z7JjSjPttr4XQ2PD6oo6cvDeduzaU6nCrkO1hws6o5nks3Uaw dKdjMIB0lnmw46l0p+ix5pKXUTNm1VIcGmwvNBYHFNqVxOgwZd8BNCQSw8x2WhXiG1K1 5q3rZZsbUN15PULC58pQmdxvkwL4vW11K68oCD5z5Dn7B2Y+8VaAxfodGvEWQbmKcF4i pIJN28eJtUHdjGnndaGfplQ0u/Od+a+XygOLhPQ1riQHgJQmjf/ktw3vtPd+1os3+aAf aP774dla0kKxoCfoieB+ncyZB7mGfEOKwCJ+Dj9RZ/3FdehGLylE0CqHYiDcmwWv/dPS iAAQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737946747; x=1738551547; h=mime-version:references:message-id:in-reply-to:subject:cc:to:from :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=gY7whz075gF2oDUVkEoQRi0JP9AZVP1TZQbx76u1kos=; b=OBAxaIQHxu9gS8PLxtg+xL3fevninSI0aouuct3GWGowjOQ67eJ0358wns/K6Ljizl Lmos33xIiQzgtnKS+tuj/0Ko6B2h27GeDbP7xvTLEMBYMv7+MLSwhk0ROCp4c4+ek3ic kp7hifhpolizRxWspWDKHDmlXkiZAqQNTD3Ha+7DhUTLdDnQ/v4Ryef6p5Xe53UitgdC Fu6x9GqHz9g6DasjKT22ivYLH+c2kHXDvqB5Cdyi0bSVqCAcEIlTofBGSF+uYov4n17G LY8Z4IuVM6rxBhzZJywTbM48iZFlG13/90RyQwBcWReheQf97PDlczydMFiX+CagIhqD 8K+w== X-Forwarded-Encrypted: i=1; AJvYcCXlpFfR+eyUn6/P8x91qpTmfZxoXCeN+V6IXPQxA5v027PtXigOkaIMhohgx63bwseqYbEE9HZT7Q==@kvack.org X-Gm-Message-State: AOJu0YztI/3jaj82SDchYH3I/i+k0oHl+Kiujt8HbrtUZdVnCXsULsvk yB+MPElEXdSMybfIbE1/Ztx+kwEfUlJnMOq+jD589pYg5hJn0AGovHTbIx0e2A== X-Gm-Gg: ASbGncuOgmagKSJsbzqk+TPzvb2H+j55AH04wAWXG1qHNN4V+RXwtt1RnVmCr2F6bpE kFWQ1L8t7ZPiriLur/FPD0hMiUG0kiXmsXxnGEDa+Uf4zbAqPiX5a/fBIRUVZl3Gkn5SqNIFAK3 F5lC38Oy37wH9j6gXHMpZzwSXVuOHCFIb0FmlhUz+Hl33wfN0FHVdChEtpkxjqm4lY1PqB1DUHL d71OeXCYbLXkSvPDV76UQqiocziuu1vuHHkqcxPvflyyvCZDxhR6mRsrlZDeBreqH8GGh5Mxp/T chtUK9524CL4dGljNd/vgOT/3bOqsxtAqkatFieAyF3HAkNlVg+e74ENgAYrd4kd X-Google-Smtp-Source: AGHT+IEtis62Qgxn0OUJHhskPq+vJz2+MtDmdCQrrgYXMhkRJPuAp/K3UaUnvwTAxqlINGYQ2m8HrA== X-Received: by 2002:a17:902:e5c6:b0:216:5e6e:68b4 with SMTP id d9443c01a7336-21c355f0a3bmr611437575ad.46.1737946746849; Sun, 26 Jan 2025 18:59:06 -0800 (PST) Received: from darker.attlocal.net (172-10-233-147.lightspeed.sntcca.sbcglobal.net. [172.10.233.147]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-21da424eb96sm52650665ad.222.2025.01.26.18.59.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 26 Jan 2025 18:59:05 -0800 (PST) Date: Sun, 26 Jan 2025 18:59:04 -0800 (PST) From: Hugh Dickins To: Johannes Weiner cc: Hugh Dickins , Andrew Morton , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , linux-mm@kvack.org, cgroups@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] mm: memcontrol: move memsw charge callbacks to v1 In-Reply-To: <20250124155420.GA1222@cmpxchg.org> Message-ID: <9f162aae-00fb-dafd-848f-52214836789d@google.com> References: <20250124054132.45643-1-hannes@cmpxchg.org> <20250124155420.GA1222@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Rspam-User: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 58C7240006 X-Stat-Signature: 4i49m9tp1wzhyg6c1y7ccy8f6b7pezte X-HE-Tag: 1737946748-423098 X-HE-Meta: U2FsdGVkX19NtifmgRDS0R7CKnaxPQw2p1saqoWeHFjQwe/9i3tizOGnj4p/x1lHJQ+wgXc053teC0BhfDZAJl0K2xX+TV/0lxpQFWUJjooZ86tcpzU7BAW7BNSSNVu5UVc0YlHpWCncb9RsIwGy4aL6Rd4kkH0fx1yVn3jtYBkIfGW2ovSfXgAxLL5khSnxVCbvWZHi0IVfdJVl8qWwxWcnbbqkzDSIh+JtvxRuXMlC6L3RXsdY+KJTzTodawos0OhcA2k9XwQEH8in0RJ58ZJVDycS/8war5+C9wABOLMdpHwks4Y0AxcBJIlWQUVHZB1Fn+vrTxQkcfhAZBAdjKzIysXG255guFdvV5kgsGC5TX+oI6uGDj6SsKezlYCx1McIMrLGAnk42LemQ0RGi2qeuYAY/LO/nkOfL4G2IMFUvJAWgzEpN6CsLdQN162udaa2Jqv1vpGsPUlNGdh9bWLOGVSkRFmQTFNTvCyuEXDrJy6eV3GEJgxuBgXqVNVsd9TnH0NKf5hPNQHvWXhvKiGnDcMidXle72hTUPECjaVbaigHjZRZH7ayldfoOUDpVDXD5luUGyYWSi28Y7ik5zocllVWMpoCF0/ezf6WLU2YNNBltHD+YSIV4tx/fTDSNykO2NIccjo1cOYNLD5tGyLKgZG40CIW9J9ZM9qyfzjpfxKTMaHw2vykQw4xACblYBg1qbrvkN9XOsdq2hPDXQXmuQntLB1vZJQWq9P7KrMog5GMYCn0YUxBmqCN4pjpfjlfvplTM056ULNbdcIpEbfTGtq9w372fh/ryHFcH4iElhbvWOxO4D595Qb82MtfllFFdygEDFOxIWjTAOp08KEEz7dYtU62nNjsxtAmmtuA3t7oOMn6vsUrvcdOwzbIquldwpa0qHrMH5eeZuRbV7hCIcvO1N7mC51658D3cUonS5GPcd8aO65ZFqTNN9NcZ2QFouY2VKTA046nbm0 qzhZsn9c uWmTXUpyBWFQrQu9B410O6s4W7OH9icpBVFikc09G9/duKGH4BlDBRkJFsVHCpY/gqEu0eCWKcmjKntPp0IHRMdVDHWkfatPK70lbi6RYgZMzE9DTRv0ogGxznhlFIpmgT32hQLJOdXD2heJAvHtHrpdmW1PQNHiUQKD1NkUXb+gi1gsk7lDKhIQcOOb0orTY44XDRUMo5EFs62FedDG85LFPqGO7cRe2hijb7Xn8ABUIGMvT8nAO5M/Nkkc3ESPD5wM490is8nwxeoQ0/PmzHowk+IZGqAyTeaUhPxEV+grtaesulTv40W3GTzjvdYvfLVIENQ/2K/x/Hj7lyHvLUBQSMt2DpxILCXUP3YuR61wX5fkW9IRDBkqsxcqJWwT/62n7JnVndCTDNHpYwI78bfn6k8r0buHlZ5z5 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 Fri, 24 Jan 2025, Johannes Weiner wrote: > On Thu, Jan 23, 2025 at 10:53:04PM -0800, Hugh Dickins wrote: > > On Fri, 24 Jan 2025, Johannes Weiner wrote: > > > > > The interweaving of two entirely different swap accounting strategies > > > has been one of the more confusing parts of the memcg code. Split out > > > the v1 code to clarify the implementation and a handful of callsites, > > > and to avoid building the v1 bits when !CONFIG_MEMCG_V1. > > > > > > text data bss dec hex filename > > > 39253 6446 4160 49859 c2c3 mm/memcontrol.o.old > > > 38877 6382 4160 49419 c10b mm/memcontrol.o > > > > > > Signed-off-by: Johannes Weiner > > > > I'm not really looking at this, but want to chime in that I found the > > memcg1 swap stuff in mm/memcontrol.c, not in mm/memcontrol-v1.c, very > > misleading when I was doing the folio_unqueue_deferred_split() business: > > so, without looking into the details of it, strongly approve of the > > direction you're taking here - thank you. > > Thanks, I'm glad to hear that! > > > But thought you could go even further, given that > > static inline bool do_memsw_account(void) > > { > > return !cgroup_subsys_on_dfl(memory_cgrp_subsys); > > } > > > > I thought that amounted to do_memsw_account iff memcg_v1; > > but I never did grasp cgroup_subsys_on_dfl very well, > > so ignore me if I'm making no sense to you. > > Yes, technically we should be able to move all the code guarded by > this check to v1 proper in some form. > > [ It's a runtime check for whether the memory controller is attached > to a cgroup1 or a cgroup2 mount. You can still mount the v1 > controller when !CONFIG_MEMCG_V1, but in that case it won't have any > memory control files, so whether we update the memsw counter or not, > the results of it won't be visible. ] > > But memcg1_swapout()/swapin() are special in that they are completely > separate, v1-specific memcg entry points. The same is not true for the > other occurrences: Information overload! Thank you for going to the trouble of explaining those other cases, appreciated, but by "thought you could go even further", all I had meant was that the do_memsw_account() checks in memcg1_swap*() looked redundant to me; and possibly some other do_memsw_account() checks. I'll say no more, I don't want to expose my memcg2 ignorance further, and I don't deserve another reply. Hugh > > - mem_cgroup_margin(): > - mem_cgroup_get_max(): > > The v1 part is about half the function in both cases. We could > split that out into a v1 subfunction, but IMO at a relatively > high cost to the readability of the v1 control flow. > > - drain_stock: > - try_charge_memcg: > - uncharge_batch: > - mem_cgroup_replace_folio: > - __mem_cgroup_try_charge_swap: > - __mem_cgroup_uncharge_swap: > - mem_cgroup_get_nr_swap_pages: > - mem_cgroup_swap_full: > > The majority of the code applies to v2 or both versions, and > the v1 checks either cause an early return or guard the update > to the memsw page_counter. > > So not much to farm out code-wise. And the test uses a static > branch, so not much overhead to be cut either.