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 122B8C4167B for ; Tue, 5 Dec 2023 17:17:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 754B76B0089; Tue, 5 Dec 2023 12:17:07 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 704D36B008C; Tue, 5 Dec 2023 12:17:07 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5F3A46B0093; Tue, 5 Dec 2023 12:17:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 50A4F6B0089 for ; Tue, 5 Dec 2023 12:17:07 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 1D9F414018F for ; Tue, 5 Dec 2023 17:17:07 +0000 (UTC) X-FDA: 81533420094.29.B79989A Received: from mail-yb1-f181.google.com (mail-yb1-f181.google.com [209.85.219.181]) by imf15.hostedemail.com (Postfix) with ESMTP id 3BC19A0006 for ; Tue, 5 Dec 2023 17:17:04 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=JmRkxxvE; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.181 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701796624; a=rsa-sha256; cv=none; b=IHBPGof471bI6bw650nC4jto8pp5vFfVHhz4M6yAnDxc3/zEIf+PEZ++9y10KfA7y+5MtC z6z0BKBcvXycyJfsnHwFNxyNQheUZbUVMZvzr6GiKg/rt0SDNob4xRVX9tsmpLgvW7vfnA SS0toxOfgUyfrSRLbcvgnfNbq1Lw+Sk= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=JmRkxxvE; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.219.181 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701796624; 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=+j0Ow1vOCeL95TLnZYt84KH1KzVbnclXd3P1tOJjQK4=; b=f5lX/Vg8MY31q+8n8hGVMM/o4PyIALuKCQ4ftk5T+aum40KYRMLEzK5IINCDN7r+LqTnR9 Yc8driOlcWw5M7/S+B9mNPH8ddjhXeLp3Zt+pR+TBbWgJVVPL0v5xlfQ9GtSLvmGWqnCR6 i55CNKpVtQ1LoPc7iiZqRPG3ggUkl3Y= Received: by mail-yb1-f181.google.com with SMTP id 3f1490d57ef6-db4364ecd6aso3491704276.2 for ; Tue, 05 Dec 2023 09:17:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1701796623; x=1702401423; 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=+j0Ow1vOCeL95TLnZYt84KH1KzVbnclXd3P1tOJjQK4=; b=JmRkxxvEJyH5SAVJA6ZBJ8QWzEkyYqsHXxNShdurwTIHBrG5/ZItiD6KtCoW1MQZYj 5fSavkab2gxlyUIQMFfzkMDhR1oWTE4LjxBuA41MjBmGxRAdSgoNtwqKRjesHe4oSBVY aAMGikmrtJR61FOKaEy0Tqxp2YeeBGuEVtJ85a3H04+s9kYwCNwAhcfnlVL9WalMqBgH qgdzESbGmbsRqGDkJnZewLHCp6vo5QHemiZwlldNSHPBzxoN6hC7Y/gBlH5TLnafYAlV aAgF65EiTytxqxtgQdcl795SprE16TCq/Zh1TOM2jo3Abe4swrj68TcNmbQVYyUiVmY7 VskQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701796623; x=1702401423; 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=+j0Ow1vOCeL95TLnZYt84KH1KzVbnclXd3P1tOJjQK4=; b=nDKeFq6wGY7Ja4P3w6T8UPzu07i1a9MM4iOART8OER9D3HYxd83gDcBWcpskGdcTO0 YE04nl6Mozu/E7EaWQzagMBwMHRLfdku7bYXmW6Ly8f7hXzE3fmsYUdiWgdwHP92+fTX tej4LG0UkMR+7OrkyVqDwp/9CgCx/pOX6WP22q/oGlyvRAWz8pNqlwdfdSrfiTXkIgOJ qsbN0vij4UGCPKOpW4gw2kq2QCENu819Myb2VuNDKqca1i0DLWHS7D9xcLhad6UONclB C6h1CZfZXehxulCo9t788HbfBrDBuyhQRIAwC82nUOPFXI+LYuE4Kwg883q+UCbMIvZK K3Mg== X-Gm-Message-State: AOJu0Ywv+z2NufUrtqADjlhp9RcnJ6CPRw7T+CToy+CqwRQKwCk3dWdD VKPJCuYyOJt+ApaYW01hkbeZYQ== X-Google-Smtp-Source: AGHT+IHCOFtbuh+2xx33YQUO2m6y7idmJ/+9D8OhxohxpRR24h/b3yAyf/3HI6xxnibo2qd6edmAOg== X-Received: by 2002:a81:52d2:0:b0:5d9:987d:36e1 with SMTP id g201-20020a8152d2000000b005d9987d36e1mr1208999ywb.76.1701796623181; Tue, 05 Dec 2023 09:17:03 -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 o4-20020a0ccb04000000b0067a24c354bdsm2691901qvk.20.2023.12.05.09.17.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 09:17:02 -0800 (PST) Date: Tue, 5 Dec 2023 12:17:02 -0500 From: Johannes Weiner To: Chris Li Cc: Nhat Pham , 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, 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: <20231205171702.GB99931@cmpxchg.org> References: <20231130194023.4102148-1-nphamcs@gmail.com> <20231130194023.4102148-2-nphamcs@gmail.com> <20231130203522.GC543908@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 3BC19A0006 X-Stat-Signature: uuid1drwmm74s3tixohetugrc3t4eper X-Rspam-User: X-HE-Tag: 1701796624-234103 X-HE-Meta: U2FsdGVkX1+/MIxBEzxKSdDLXhod4P8nOzYQ5cjXLc/eP0q0qDh5FFoEOegmzSSWtBEzYQkBamXIZhWC08EPEbpyEQ/gf8r1lxkQ9BNWUL9ik3aUavB+ZTEMLlbB3ojOPBzdz9tbmIxKQbBxzUHX9gt5ACDDRHbByWECKqRWn2HQP/ot1DVsTkfncFPtBQbJ/wzNBooZICiAErtV6GZ82IxdEX1Wpi0IevnLJVeB5idwm40aIumCkAg5rYrP4Lfal0Wgtrc7ZCbkuY4mQPQmNL+TYjdKmoUA7/schomZXL0w5q1br4kyB3juIIFNnihiVLaqNIqTlKevkiMOZPdlgHJxBs56kYAvq44F7vPvb+/c8aAIoxULDnfJ94pH5CATSsJllx7j8dz5rfq/AgwHFK3lfMagFH1NP2dUPTrWH5fpsSB30oC28Di7aXQuPHnZQkeoeaGtxiUD6lkrnN9hoX7aDcVFRw8ZnhlqIZx1vtnwbVVsxGLn49DMlTzxDzTigbIYqxBjyt58uXm2AJ86LvVFpvIpHJvwk0Xwz/m+sGp9wrixH5WLahCyF78YduI4Z91nIZ29jKne72Fjg/MujzOifWQ9Vu85TF/geTvY+8LXLuO+8dOxHvcp1dHRLVcuLvKgf+sjP86QOrCa/BwBTxuCGP3BcmP6SzRsHF/1vtNbZp2gQckcTfjWgw9kghisrlCti1VL8YamXmfljVTFBx+8zMBltAef9eGcVoyU50gMAjYDBL4sN8G67cyAMruxatwMamOFkAnDunExuwrgpenw2F7biPpwwJ9vc7FXz1GtTzjsKrB2Ekeo1fNHA9JRTEIgVqc1j0vU5qjI673z1bgetbm6hVAROolgdaeVqSNRpumNZbvHrQpmwuqBkKuWP1XCPfcboN8IEkfu3cYHbexswPZt/t1rdm9a8ZJFfTDSc23bt88NSlXu0su9pYxYTlSgo0mpiNzq7ntfRZQ Y5xGrftH onvy+/n9axSpq1aQgTUuODga6N+s9hUWxeoX5y4lqJ8g3bExC01mT9fAIPZmT8Ph9QEDKBkbrkWOGtBxD4oDhiQ+JVBlEF9Acl65eiEQTZLvCKFIQ6KoEkTKes/xrEVM3cMCLqFV3x+J4mJX+t0uAiec+nbGFS4OUPzscqW/kLrx/b09zsmk8ZeqtJADFWpxJ1CF5FVEpq611i2NZymXjzcgootm63XHVAc7OcB/bk44UCRTHR+h0XmvJnWnWXSLdLTdMogwHcJH6wpoctVb1nx6PEvweimlrngXU3PLPBE6LzppmDCRbXA/MNrRzrvmpsxast4dRh+FTQPop44E4xU0WDBAJGgfIaW146ve1fBg4M6k67NTJXdL5QNIeicoGhnJNfM0/HkdsmDT1BwTFzwVjFbUO5lY8aSaS4DYMjvnu2HG2us35pgurEIDslBtrLmfyq4oPhEqRVcWtaEqfe1wHUlDh6F7yXuhnGeHUwi7Yj4cuMRa9IgURGqlTlxEm/MMawVq/1OjZ6+4db5iJQlLSUi3BvGoZj6f/DzzkDkGIK9e4ORdKD6khUOLdraK1NOEebTXNtWAW2mLfpfllp4uytf1LV8S2Urz8IH3o9WorMJI= 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 Mon, Dec 04, 2023 at 04:30:44PM -0800, Chris Li wrote: > On Thu, Nov 30, 2023 at 12:35 PM Johannes Weiner wrote: > > > > 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: > > That is my first thought as well. If we are having two different > flavors of LRU add, one has memcg and one without. The list_lru_add() > vs list_lru_add_memcg() is the common way to do it. > > > > > > > > +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. > > We can add it to the document if that is desirable. It would help, but it still violates the "easy to use, hard to misuse" principle. And I think it does the API layering backwards. list_lru_add() is the "default" API function. It makes sense to keep that simple and robust, then add add convenience wrappers for additional, specialized functionality like memcg lookups for charged slab objects - even if that's a common usecase. It's better for a new user to be paused by the require memcg argument in the default function and then go and find list_lru_add_obj(), than it is for somebody to quietly pass an invalid object to list_lru_add() and have subtle runtime problems and crashes (which has happened twice now already).