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 66B5FC3DA49 for ; Fri, 26 Jul 2024 23:31:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CFC0C6B0082; Fri, 26 Jul 2024 19:31:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id CB67E6B0083; Fri, 26 Jul 2024 19:31:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B73AD6B0088; Fri, 26 Jul 2024 19:31:33 -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 9A9EE6B0082 for ; Fri, 26 Jul 2024 19:31:33 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 045B0160138 for ; Fri, 26 Jul 2024 23:31:32 +0000 (UTC) X-FDA: 82383502866.08.712BDB2 Received: from mail-qt1-f176.google.com (mail-qt1-f176.google.com [209.85.160.176]) by imf15.hostedemail.com (Postfix) with ESMTP id BA363A0022 for ; Fri, 26 Jul 2024 23:31:30 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=U29mNIz0; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.176 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=1722036640; 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=biuKcawwBlNMD+laH28Ncnx2/ZwF1wLoS8xlXY0k0qo=; b=QuAhtBh1qzuEhvdQpZnX75KIKX3ebsuZUDPPunhkVYastnQEzbbRL869GMv/gU6Cvwf47I Fci76JRfJxKFXxXAmHhjGAGZ9UJS8NUJgnmdJkLSgklYpzRygN85l7CEOR0BOYef3QqOb0 JbSkpXrt4fAEGh3Za4Vd+7pZCyyw0GE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722036640; a=rsa-sha256; cv=none; b=o+z9v8tbH0iWcYjZdo3cc2Hl65ut2hzFIs31cl2BB2V1FzEnCkQ3kheGB6WLRONVi7r+ny x2UzloMM0lX37WO2eH6aZvse7W9G67OmIUlC/5KtJKNeCsCt5oIC3x7rMTbxrufDyurqGk KUuQEI3fpYgwjc4yGvA9j9mYSc64IuA= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=cmpxchg-org.20230601.gappssmtp.com header.s=20230601 header.b=U29mNIz0; spf=pass (imf15.hostedemail.com: domain of hannes@cmpxchg.org designates 209.85.160.176 as permitted sender) smtp.mailfrom=hannes@cmpxchg.org; dmarc=pass (policy=none) header.from=cmpxchg.org Received: by mail-qt1-f176.google.com with SMTP id d75a77b69052e-44ff50affc5so6996151cf.1 for ; Fri, 26 Jul 2024 16:31:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg-org.20230601.gappssmtp.com; s=20230601; t=1722036689; x=1722641489; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=biuKcawwBlNMD+laH28Ncnx2/ZwF1wLoS8xlXY0k0qo=; b=U29mNIz0ED8YsqitFWrFJC2XxNTU608YU53iQVhp8QdcUFPVXP/dowK5P1FnTFdRHT VzovHK4fklh3B+UfRUk1O7nVoewy3Oep3/dYAfRtXb78mfWzBMrOybfvGSyuj3Zsv5/K ZdI8iWeDsdmpzJNfBH9GLAhzQLVpZDckUI6QtBEkjrEQbY+QRVJTtk9FtOE96HqCkO5c pKPsadhPSL6wilaif9il1Yn5SEG+zbMSQDlomhNfbtRm3a+lCXGIiOcXWqGo53FMJ48G wZqfC9nRjbIw+nvFjpWCpCfrA1Q6KZjik485lOrRQpVs9LoyuWLhZnAsU1SJI05G8xoa WdFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722036689; x=1722641489; h=in-reply-to: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=biuKcawwBlNMD+laH28Ncnx2/ZwF1wLoS8xlXY0k0qo=; b=xFv91smJNPrpujmvv38MFQGXoRVM7p+8E4g3/LMh7p8R0CoITBrOpqCv6XGNLhx2Fg 7WwtYqtevXZ3Ya0WzUDdUz1PX5UMcht7CTWuKL64GjlpZ4aqj8W5Is3iQPkqQNFqzdW+ gMjLxx+iYYovlaG9RS76Xe4JEK8EYWnqqduHHjPJs4mQxq7Gym1EsZSqz4zNXkkfsKs2 mgcgXq4ZSI+Vs0Y9r+JGVBb/EQLMwDoFF8TABWbPPTD6xJ8R7Un2xOTQIrOYCaYtcHIl HRENR5XLnbt838LXgUNpl/tWeHje73HiwCb4Ukij5z0b4cCRzYPxuUDDr9nsZWrWJYqF o6hA== X-Forwarded-Encrypted: i=1; AJvYcCVObRz0i7znVETttKMK5aMtMa2dG6EIV2SJ/JAqt3yMNmJjrAearfYN59rKb/3imnsOMI5gVmA3lR5WvrNHR64IoLM= X-Gm-Message-State: AOJu0YymlMkrNEW3eUYlwJR/IUlOq7P58+217jR/+nI5nBNOZ8psNGZN PFijh4T3QK6b3hf6YjSqR/70uvhTNFyKpNrGaW8I4LZAMLCsSerhwh7c9hAHQL8= X-Google-Smtp-Source: AGHT+IGSYhJwCawC3mAvb2HTZ0jkpic6PitolVJPtOM5TcVmi2Zwx7iaFRACbetCiBic2bzak4rvEg== X-Received: by 2002:ac8:7dcf:0:b0:44f:9e6f:d00d with SMTP id d75a77b69052e-45004d79d8cmr17878461cf.11.1722036689529; Fri, 26 Jul 2024 16:31:29 -0700 (PDT) Received: from localhost ([2603:7000:c01:2716:da5e:d3ff:fee7:26e7]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-44fe8413194sm17045201cf.86.2024.07.26.16.31.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 26 Jul 2024 16:31:28 -0700 (PDT) Date: Fri, 26 Jul 2024 19:31:23 -0400 From: Johannes Weiner To: Roman Gushchin Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Michal Hocko , Shakeel Butt , Muchun Song Subject: Re: [PATCH v2 3/5] mm: memcg: merge multiple page_counters into a single structure Message-ID: <20240726233123.GG1702603@cmpxchg.org> References: <20240724202103.1210065-1-roman.gushchin@linux.dev> <20240724202103.1210065-4-roman.gushchin@linux.dev> <20240725214227.GC1702603@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: BA363A0022 X-Stat-Signature: muas8nuc8dtii6tsgn5ocrbsm31ybgbw X-HE-Tag: 1722036690-227496 X-HE-Meta: U2FsdGVkX1/QwZOWRkK1F3CaCneeJFQiT262N0vb4BAvR1h6E5siqn3q5U5i6yX2h/TiUW5GlTYEhVwz2C2v5FlupXWf8Ea/baoltANhGuK0nkt33PXCPo4rfTu/xufHxzWZr1dM35HueW+UGrMgx7hJn/cs6OtFZeoQ9QrSYCygtYrbXecSSrOqH0JQGwBbjGPVE7kR7ps+y0oqDbMp5jC7A9Vjd+BIxP3HmfZJfFT9fBkf6VGBTGrNXjIH3vFF5e9j6OK8kmDjD5FpJeB5pticp0vk36wWD3DkixemIw9EPuC4Cf4KgYmthfTrzaWsmK2fkEI3uisi7rUm7g10dD7r8rJUKfu1e9aiqZCshz7IvEPSNYiY4DexaCECv7SqODoF/SScLCgDerUMBZqqS7CelGF7dyiKzP2oEQ3L9FY5R8Vcq1A4oVf3MDv8HSu+vjUjw7vTzeGChVyUVThc8ZlNmIVu1xACMi5yFKQRUR8M7L94aHApp16Lj4gTqX5DbnRYbFYcRJD/meiugDOeDKgf2uSUEx2cHuCWLt5vsmZB5VBmh/8uijW70ZP/iEwiv0PcGmRc0QoALyCGLF4vcbcbJZ/+y4wsKq6GEZlI8MuCY+dhUO/4QIq0SgzG3gCOK65Zt9Vq9YuwLp6IM5ZwJLrI6VbhS5jx7B/MYvyBtobdhbjla3L8P+gSgICBCSq0UQc0Qkg7a7Di71low6ARs3lvhGKJ24jRkTPgdoSFiqmL11O7cz0zG11qAr1dWwKUtT/9VMTAJHYgps16yzlffU5EGmKNlwuOAPDMNmV0saUX4LVR0tIams4SMqpuj3w7WZJfOc8qmGVl9hZawTN8t3632YyghT98z0VdfHWAGQ5diAaqll2J4sBRzfiwhRC2qeXkX1TSizHDsXrFoZizqg3QGNnnWjc1FGLnqPMDQZgl3xDWmXNgjoWyUPuvFQ5ISrc/f9ryqu87Eq2uabk fB+AP2US LR+aXA384BzX/rENmxXUA4dloz2nrQxd2ghu1g3wYEpbNt38+wkba18VLAjKcZg5Twbw6aZZnQSSWSJHusoP2nIMjtPuvfSk/4d3r6julB2h3s9ALHvURcOzRKHU/4BW+VXvyYMUQ4IxbvNDpSAfGObFX42zZlqXZFjgrv6PFDeMb/OZfu5ak0KXlTt16EpRF7WjBP//uYUaMckU4lXiTblePjcG1bDQzUi+lKID8BXW9qQKlxi/MRNatW/tGlVO+TxUxJ8JyA7ZPyN+TKfwdzVvGlfNjvIC3svkhD//P80Ch9cFQUvHS/lrFcEa0WbfO2Tedmk1sXXwXraMWsLRvHjIdMlbRowLMXFElqySTAyZb4qsu6O9oK0pYid50gmNudJj9Rg5Wbd9mExrpLiUhbReXEk6yOnaRL5YZO5S57PAkzHTGplI4EjIpC45m/D8iUEQboTuUJ2QVZ50kcWyw+hdMpRWEgPFdjY8j 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 Thu, Jul 25, 2024 at 11:33:51PM +0000, Roman Gushchin wrote: > Hm, Idk, I do agree with what you're saying about the self-contained > piece of abstraction (and I had very similar thoughts in the process), > but there are also some complications. > > First, funny enough, the protection calculation code was just moved to > mm/page_counter.c by a8585ac68621 ("mm/page_counter: move calculating protection > values to page_counter"). The commit log says that it's gonna be used by the drm > cgroup controller, but the code is not (yet?) upstream apparently. I don't have > any insights here. If there will be the second user for the protection > functionality, moving it back to memcontrol.c is not feasible. If it comes to that, I think it can be its own abstraction as well, with a struct containing all the elow, low_usage, children_low_usage stuff etc. and API functions to propagate and get effective protection. Both memcg and drm can embed this into their css descriptors and use those helpers as necessary. > Second, I agree that it would be nice to get rid of the parent pointer in > struct page_cgroup entirely and use one in css. But Idk how to do it > without making the code way more messy or duplicate a lot of tree walks. > In C++ (or another language with generics) we could make struct page_counter > taking the number of counters and the set of features as template parameters. It shouldn't be more than a for loop right + the unwind on failure, right? *Somewhat* duplicative, but pretty simple code that's easy to wrap in a controller function and have it out of the way. Actually, we could keep a simple hierarchical version of the page counter functions, but expose non-hiearchical ones for users that want to do additional operations on each level. IOW, e.g. page_counter_charge_one(), page_counter_try_charge_one() etc, then implement page_counter_charge() et al on top of these. swap, memsw, kmem, tcpmem, hugetlb etc. could remain unchanged. The memory counter could have a memcg version of the charge function that uses *charge_one() and handles protection propagation. drm could do the same. That would keep the page_counter->parent pointer, but would save a bit of complexity for simple users at least. > I feel like with memcg1 code being factored out the benefit of this reorg > lowered, so how about me resending 2 first patches separately and spending > more time on thinking on the best long-term strategy? I have some ideas > here and I thought about this work as a preparatory step, but maybe you're > right and it makes sense to approach it more comprehensively. Sounds good to me. Thanks!