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 8A50CC4167B for ; Thu, 30 Nov 2023 20:35:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0B2F56B04AD; Thu, 30 Nov 2023 15:35:31 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 03D156B04AE; Thu, 30 Nov 2023 15:35:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DF7F46B04AF; Thu, 30 Nov 2023 15:35:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id C953B6B04AD for ; Thu, 30 Nov 2023 15:35:30 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 9B9F21C0336 for ; Thu, 30 Nov 2023 20:35:30 +0000 (UTC) X-FDA: 81515776020.08.F74684F Received: from mail-ot1-f53.google.com (mail-ot1-f53.google.com [209.85.210.53]) by imf04.hostedemail.com (Postfix) with ESMTP id 7DAC140011 for ; Thu, 30 Nov 2023 20:35:28 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=ckD1ebN6; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.210.53 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701376528; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=BvSdTJaNOnOGpTCIQ0TIiZEwNuG0mo2jRjX7yhoMQa8=; b=1MOJ/h8vEHUaF73xny0mYJ3hyiV1JnghOu+vqrHxu2dHHTehvW7ZW82FjSc/DtThaAMhAp PGlrG1HZC+UnTvueAUM0IoLf/conrSs6gq0EDwMbCzYWuh2GEJP0ILqXuLvoGReGuLGrow Di4vwNxEh0hQl9Oia8KSf66bnq77U0k= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=ckD1ebN6; dmarc=pass (policy=none) header.from=cmpxchg.org; spf=pass (imf04.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.210.53 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701376528; a=rsa-sha256; cv=none; b=K84X7Agbo4PotnDsYS0UIaiUUdlQlci0gmPyqiAeXadyG3mODIPm+w224vIdWcyK7tCvJb I53oZDQUcF2Mko0Od9sW6L9HpUHl3bAN2z5W1BC80/t1t12vHLddwY4hnVJvOfh/gc3DQI 8glOKoe7KYiYXBTheGEDvpqpRRUB2Rc= Received: by mail-ot1-f53.google.com with SMTP id 46e09a7af769-6d817ccaa6dso837227a34.2 for ; Thu, 30 Nov 2023 12:35:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1701376527; x=1701981327; darn=kvack.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=BvSdTJaNOnOGpTCIQ0TIiZEwNuG0mo2jRjX7yhoMQa8=; b=ckD1ebN6OlBqCI8nXgB3XyVVecu7sM8BVfmYy+KPjmmy0im1XSPl/5JUZY3aDuphD/ 84MIGkxTlsQrCu9WbWw2TiCZMODjNFqhzZGPdP2cIwkIZ678tYF3X4igoCS71iJCzUBe YbO6huzf3ggs0W07+inPJadjU3PXBr2h6o8nzIMUZAPrC2w4SzAKkaNwSWcv34tSRJym KL20uc0F51QgXcen2JKTHmwa23PXPRNyTUcUSBPyadaSYx3mA0L3S5CV7fnoZWtRYJkN 2lYAODdtT2hrt4CsZg4F2U/1xNzVXsWly2cdXlb2WK08bHrXmB9uZitvRnQ5kEP4fxfG RS/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701376527; x=1701981327; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=BvSdTJaNOnOGpTCIQ0TIiZEwNuG0mo2jRjX7yhoMQa8=; b=TmR8nQcrFGBFBlbvY91z8n9lglNuIfuqxQmtwUwcERDHVsSUzdxYXyjk3iDYcNjCet tKoPXGX7pBAwPVosjH+iE7WfHMPGtUQlE3a+xv2vZzdCdvQ84t/7c3G00R9HQdHZcn5f wFLPAeMcph0k7aFZOy4Z0lWROhRsVmeU3KkTXhnHU52/yGrTHvCpCTyoPQXODQJMQEm4 UjEXaiaBZcuh8iOgS2dG43SOKnE7gS9NG/CECBO3e3p0SmXR3fV2wMdGepwUBCILWqE+ 1cEp7cvthsaCQmnGsAiLQhUM2XyL8ygv+uGmx3rIeplh3ja5pDqmAJJVXNECgLTuuYI8 /l0g== X-Gm-Message-State: AOJu0YxSfWUK0prD3BEQLfOg3iiLlEiSag5KjHEgTMammwgaXZIw0jgf YxJszhvrsjO9x1j3aHLfkH2AJA== X-Google-Smtp-Source: AGHT+IHJ7OLYrr+3HHJd78vh493YJrStqv/97nm/tolthFXTq9aW9P83YmmgwtFds6L9t5rJr1+WbA== X-Received: by 2002:a05:6830:4423:b0:6d8:5027:4620 with SMTP id q35-20020a056830442300b006d850274620mr946111otv.24.1701376527391; Thu, 30 Nov 2023 12:35:27 -0800 (PST) Received: from localhost (2603-7000-0c01-2716-da5e-d3ff-fee7-26e7.res6.spectrum.com. [2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id eq8-20020a05622a5e0800b0042404e87331sm66369qtb.64.2023.11.30.12.35.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 30 Nov 2023 12:35:26 -0800 (PST) Date: Thu, 30 Nov 2023 15:35:22 -0500 From: Johannes Weiner To: Nhat Pham Cc: Matthew Wilcox , akpm@linux-foundation.org, cerasuolodomenico@gmail.com, yosryahmed@google.com, sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com, mhocko@kernel.org, roman.gushchin@linux.dev, shakeelb@google.com, muchun.song@linux.dev, chrisl@kernel.org, linux-mm@kvack.org, kernel-team@meta.com, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org, shuah@kernel.org Subject: Re: [PATCH v8 1/6] list_lru: allows explicit memcg and NUMA node selection Message-ID: <20231130203522.GC543908@cmpxchg.org> References: <20231130194023.4102148-1-nphamcs@gmail.com> <20231130194023.4102148-2-nphamcs@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 7DAC140011 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: gnhffrkqwutdpiramk375zrw9665uxff X-HE-Tag: 1701376528-246918 X-HE-Meta: U2FsdGVkX18M78BDuHqdy2jU4g25pNJgapb2QfcAD7xe4Ud6E7B9oFbEaXePYVGufKR9bhWMABzz1FcRZtFsbYftAvAiCc5T3hpNRODGVu5yvldNcHMd3SLjOSd4UFC1XCgIpYz+3o5x600jnbUjrgO/VdznQ93qrZqYwi3dDECIlwd4MMRE9hDOSb+3TUt4MdmHdnRCo4dCMS0PdSVNHtfRVtEE5p3qiI0ZqB3BVDJgJuvimzmqEeuYFPiom79dlwLWgFDNq91E5PmqWLs0ykVuVjfr6qFaIjgeuYkFQ1tFAngPeMEO9RS1c5flEL7Mz3b3xA6q9V/B1IyuEvqzeO4Xqiocn3qeSJ3I851hG8dbZqJm/W5jF0+hMyAyEZoTsXTpAdYrNZbDrHST/l5O8YNcz+3bmtF0ZooLMxoom76IWG8+Jap2wUFdLCM43G5m5yp73JIktyT8vDipAH0I05gPAQgg5pMVTLs5sEXnsVF4b2vzTlycZZdcxZQlVqnx9qhUgNARu8bwPiqFRKr24+cP+fBdtxZIQxBnU4lugUwfEhcJDPWQc/0Z9g5b5+b/i3WveuTmNtQwZ1OvMg3Hh7IC5an3CxsMKSdvgPz2m46A9BcNPJbQ+uJCD24muOrdmo/YDzX8E8TP4QF1gYqGWmURIEAMKQXU9g/Qytk1ji74O/zwYDf4tMW0XOThU5SG/OkmfwikC8Z2bMahOtfj5w5o4STZYgXrQ5ql/UMwsKE+Q3dFVv+/wTTOTFAR6HChxesKHYWmKRdz6d1+xvmDTg+jaVNXav/8EctU4z3TA1uuEZ2vkOLAdoVymvSfHqts+gO4BGe5obMORS2dBt7LJeUhEoPzk0+tdaA/ohjgqIs1d+9BH5c/qRcWQhcL5dPGSj2J0UHXjsF2/Rgq76a0Q2c06IdVpagvtmG7+wdEAnWZ1SMFHQQlX6rBGk9Um/Rm3ns0rEF1uc9cvDI7ZlQ g3W9bwBX zz8ws8WHfEtUoPy+HalO1iALhZzethnpb4YeuQwudOw6YxQR1Hux+ET1h+oVJYhpH53ONXylcnOMmTYlyq9CbBmQP9brgachYiTFs8XbyJpkN5w1F+rHMxgaXuNx14Y3bZpCZJhAHQtqBg/Yag8WqNmJD374DizD+2b0L48mTmQ7aZp/820KTcgeaOvX38lgXWwEHJ5BpwJdI0NZ5+GhTHQn4ou4sIeYCVfiUEN92mvQ1CiINuXFHjUz3vXp6ao6pteLXaX6ucAv77CWcPwZVy76D9+8wHrg8to7rljBh7FWuCJ5Q/T9SvSCijYR1szFbROBJovgYWccg9WbqOZZFbKgtCfKg2Qv2y3BhrUe5iSr2UcfyNkNP/rDZQ95By91lsjZ+vovmueu3Hi7Yd9em/3ZIyvnzc+NXQrqq1ThRtOPy6/AKkmCjomCvIzgZ9O+Xr6gOcZGuyXXBqHUm88Zz3oo+nrZ8MQZYpNNE0pWFLQiMHyRlsSiXUwleUkRHRbKelGFY7WUdWTGUJYlMohfU5ftrtcqmpbaQxShXK1X/E1VSscEsW6Gjola5wwGg2bbJpCJ4OSiNPpc/1xxxjgeL97peNYlBaF8k1Ha3 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000002, 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 Thu, Nov 30, 2023 at 12:07:41PM -0800, Nhat Pham wrote: > On Thu, Nov 30, 2023 at 11:57 AM Matthew Wilcox wrote: > > > > On Thu, Nov 30, 2023 at 11:40:18AM -0800, Nhat Pham wrote: > > > This patch changes list_lru interface so that the caller must explicitly > > > specify numa node and memcg when adding and removing objects. The old > > > list_lru_add() and list_lru_del() are renamed to list_lru_add_obj() and > > > list_lru_del_obj(), respectively. > > > > Wouldn't it be better to add list_lru_add_memcg() and > > list_lru_del_memcg() and have: > > > > +bool list_lru_del(struct list_lru *lru, struct list_head *item) > > +{ > > + int nid = page_to_nid(virt_to_page(item)); > > + struct mem_cgroup *memcg = list_lru_memcg_aware(lru) ? > > + mem_cgroup_from_slab_obj(item) : NULL; > > + > > + return list_lru_del_memcg(lru, item, nid, memcg); > > +} > > > > Seems like _most_ callers will want the original versions and only > > a few will want the explicit memcg/nid versions. No? > > > > I actually did something along that line in earlier iterations of this > patch series (albeit with poorer naming - __list_lru_add() instead of > list_lru_add_memcg()). The consensus after some back and forth was > that the original list_lru_add() was not a very good design (the > better one was this new version that allows for explicit numa/memcg > selection). So I agreed to fix it everywhere as a prep patch. > > I don't have strong opinions here to be completely honest, but I do > think this new API makes more sense (at the cost of quite a bit of > elbow grease to fix every callsites and extra reviewing). Maybe I can shed some light since I was pushing for doing it this way. The quiet assumption that 'struct list_head *item' is (embedded in) a slab object that is also charged to a cgroup is a bit much, given that nothing in the name or documentation of the function points to that. It bit us in the THP shrinker where that list head is embedded in a tailpage (virt_to_page(page) is fun to debug). And it caused some confusion in this case as well, where the zswap entry is a slab object but not charged (the entry descriptor is not attractive for cgroup accounting, only the backing memory it points to.) Yes, for most users - at least right now - the current assumption is accurate. The thinking was just that if we do have to differentiate callers now anyway, we might as well make the interface a bit more self-documenting and harder to misuse going forward, even if it's a bit more churn now.