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 6F5A5D15DB3 for ; Mon, 21 Oct 2024 15:44:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E66EE6B0089; Mon, 21 Oct 2024 11:44:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E17526B008A; Mon, 21 Oct 2024 11:44:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CDEE76B008C; Mon, 21 Oct 2024 11:44:20 -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 AD6886B0089 for ; Mon, 21 Oct 2024 11:44:20 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 66E3F120228 for ; Mon, 21 Oct 2024 15:44:06 +0000 (UTC) X-FDA: 82698030660.22.5A3D6F2 Received: from mail-ed1-f41.google.com (mail-ed1-f41.google.com [209.85.208.41]) by imf24.hostedemail.com (Postfix) with ESMTP id D207118001B for ; Mon, 21 Oct 2024 15:44:14 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="fQ/CihcV"; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf24.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729525420; a=rsa-sha256; cv=none; b=nLiBg3i+5+rZXvb7Un1M8Ma9SB/z7K5DnNLfG3bBnan0hrdlZpEBygJi+FOlPDv0KYpaZ6 nMZ/5Qr/G8YuCVy3We0pZL9QILZosCBLJ+tXepFiN2GX7YvMG6IRnt1ABgyiuuXHz+lLf1 3xBeikFluezHz1jCMFR732n6qGbLr9k= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="fQ/CihcV"; dmarc=pass (policy=quarantine) header.from=suse.com; spf=pass (imf24.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.41 as permitted sender) smtp.mailfrom=mhocko@suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729525420; 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=TSEb+ymEnyGrSZp7cX2c6uEaBnPjBoaasTWPd/6WjFE=; b=u0IZz6zq2+ISykk2BBE8DvV0e1TUnzjKbLeVHi/HkD0R3BuXDBswvScUzj28kMY4dZL0dz WRsuA4OBb9QLNEUddcTl1BFEaMIX3cNKrco0vXFTtTsICCM92y/v6BFxiWbA7EyBP8Qe8i 6gmCrP7NSaDO7VbiOj919SS+A1kIZQk= Received: by mail-ed1-f41.google.com with SMTP id 4fb4d7f45d1cf-5c9428152c0so6271863a12.1 for ; Mon, 21 Oct 2024 08:44:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1729525456; x=1730130256; 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=TSEb+ymEnyGrSZp7cX2c6uEaBnPjBoaasTWPd/6WjFE=; b=fQ/CihcVhi1F+ZQnVge/BsLMElyiLfhD0kO4yoqSQD2Xb5e+QTffVF009JpZN/R3Sa qBPAYXkOVe3rRAIntOKs5NQJx+YDNpCrb+6iPZhFRY7SCw6bPkftwzNndLYrJ5Wj93dJ 0axS1Wp2NVXh61AVvvPDo/94njOahDQiqpvSzrqQw7k+bdIcTv/REzODUh52OVV0ngjI 7dGA1KH50/AroAuIlgizdMUKxkRPoRZkP+jrQenS3DxDZcEGRMkQXwmCjZ5LcRH/9C4H R/JENpAE6p/OkWJ9SG2NNof/vg6cRoxQf6SnK7j3Km/7tYUgx9tWjtJsSac+m+VfOD3W d7Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729525456; x=1730130256; 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=TSEb+ymEnyGrSZp7cX2c6uEaBnPjBoaasTWPd/6WjFE=; b=QfkflSX9/WJfxQ4G4uAfYzhk0Zbea/mB+2dFopcDjBA1eUiTggRx0dzsKYhUAxD9mG YrDKbuGUSKJd6ylKLRojpmQL3jhfrjwNipLbW5Rh8EpAENgIMaA16xA43Orm4H/1378N gFsvqTeXXiyKN13/C//L/8xUw6bJgzA/BmqHZ23F4c2MhOeNyN+9xUXBTUpnPpPcQGic ygjXsLSQUm6J8vHt1M4YYu1+hS3EXxw42TJ5+RDwGT1I5LuHiIKYiBsndN6jgtKs1EOF 9N5JWVZPEIe+wbrhKfcFOMXGZ9PudWG3SxYIiE9AFw1mwBWioMRctsTYS53JRA+k2WO9 xu5w== X-Forwarded-Encrypted: i=1; AJvYcCVhN5U2rzqEK8VDZo8KKxrrkxCgIdYyS5Xi/SC5/Wf5xv8K54hRCqxwyUfN+Zj1p3MpTY8c5Hy4NQ==@kvack.org X-Gm-Message-State: AOJu0Yxdz0R9PfcxVuvttYWsAFSeDPDuV5m+cIQpJdzESloNyCRG/VXS 5X+OpTeZf1jY6rmYSDxXaIcO5ToxEgg6itR5MAWmnRWR9+1ZirPbNan4mqvdJHk= X-Google-Smtp-Source: AGHT+IGj6Ce0bxqUPeEzNKA1yVTvwx2NhQ+jfM1ctq9xUAIhUZbv5kt1K1A/bnMh5GN+TfxgS7AQpA== X-Received: by 2002:a05:6402:3509:b0:5c8:9f3e:e57 with SMTP id 4fb4d7f45d1cf-5ca0ac628c2mr9720000a12.18.1729525455746; Mon, 21 Oct 2024 08:44:15 -0700 (PDT) Received: from localhost (109-81-89-238.rct.o2.cz. [109.81.89.238]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5cb66c737c4sm2028599a12.96.2024.10.21.08.44.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Oct 2024 08:44:15 -0700 (PDT) Date: Mon, 21 Oct 2024 17:44:14 +0200 From: Michal Hocko To: Joshua Hahn Cc: Shakeel Butt , Johannes Weiner , nphamcs@gmail.com, roman.gushchin@linux.dev, muchun.song@linux.dev, akpm@linux-foundation.org, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, lnyng@meta.com Subject: Re: [PATCH 0/1] memcg/hugetlb: Adding hugeTLB counters to memory controller Message-ID: References: <20241017160438.3893293-1-joshua.hahnjy@gmail.com> <20241018123122.GB71939@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Stat-Signature: xtppmkgb39xb98nmn67xnfshf7jtttdg X-Rspamd-Queue-Id: D207118001B X-Rspamd-Server: rspam02 X-HE-Tag: 1729525454-937112 X-HE-Meta: U2FsdGVkX18j1tSDR0yltbVsS28rcPpQ+JJ6rj53j55XYJ/NCQCJtJCko2fWgxEEy7A9G5VPkmHXaPDNLf6e+ECCGhkLU/sJC8pYYwxcqB4/6jjFRsl0gXZ4i7Dml8j5i165ODFxHTSZcN5l8BvEG5HZnV9OUzcNqVW/cov5NapAHO1cON4nUWjqxlymAcNvBCoeRChCXyTGO19b7uWlzzXCCGJNbnggE1CwRyRUJ9tq+/KzauqMf4rIgf3RDsGdsBz3+Ya+FpdNxjcCg8O+0b5mXlhUo6dvNvIOn2SjfeBFl4YyjwBmS+BwPHqWfLbQOgnGSLEEV1fSRDxRSF02IlzxaM1fhusbBoOl93bvCbj5ellV6zdZDtyyKoyjT8R+vf1fpY32xQa2Vy9gBCO+EsoHO/BA7m2lHLQQahtNnPMsRcUGIMjwvzThsHDlYRgITdUaeE4oS+VKf3qwawkJGhTp3A2PERFWOrLsxRG02RmplPvCoFPjUmx4Pfo2DDSMz9ZE+2zLK2EDccAVEMUV9e2SuZCYy6ui2hZ8Yk/hUNw9Kp/KgWKz5ErA5j3J4sF6tTW9Ro7xFHI5lMVNj0dq/BZTFLssT2U3jolNT73CCQnCCrGUi3fyAm3pyay274q02Ape0Ylxx/YmuJKAj3FcIgbstdMipZq+V0G2G4Mil6RhFn8CB9QFQSX9j7NqBYP8SxPquGb1IQE7XPfBz0hmLRob7c/FC56UZUC+s9t70ZShLHRX1OH7O/nc2fxMsrbFUzSt5Hf343kceNT5V4tRO4RV6Vx6KIuQCJ58hwU9GsvULJL0Q0oN+G1/i4aovj9lhOh8R2FBys0bK29TSXWvDIVjWfopqlNGC9fWYK+h6XEVzxxjfwrvMlqmlpyvUu6T4uP73AqN7G2qs70cDn49aAJrkDhy4kfCsLTQW8gDb6n897NBlA8GBhnguh12iID0J6SLdb5dyYaVl0QHZRH VyZb33yn Y4dIo1Osr4XyNbEIT0xMw4/koo69FrMg0LPbc2bpBn6wvO2q2iqe+bAIGSE0iaJD0bh3gTh++7Vgc4RLGXbvPFfbtOqMaCrMxyoLzNHZ9/Ec2PZ5g32ttgaX1mmOR05Oy1lQvoeEYYNBcgn4sf0cl4F89uQwniwcW6VtMRxBEW2jEHo/DFerwBGPTxKOitGJZRevOSX+Cou37xxvegNWEv9qT+eDSPRRrqS+hVEunMVQMs4wtXA7OeVMiK+QzE7KGMvUDBQxzrTy3PqpvhW+Pii6DRVN61vKuhaR2wgsGsm4quhVTfHVMjYRgr4cKTJ3v7t5FVcbT/WQ2HEIHqOUks5qOVsxQp2HRhnVOPgEusduIlXQ7mXFo0g8h+X3JCrj2LiyGcOAubiv6MoiMUX4jN1H6ohGQUnLdgVb4yAiiurwvmhjjas6XlR7vEwpaj1TQ0rV467t7hcjjjyXwRJvg2LjWuA== 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 21-10-24 10:51:43, Joshua Hahn wrote: > > On Mon, Oct 21, 2024 at 3:15 AM Michal Hocko wrote: > > > On Fri 18-10-24 14:38:48, Joshua Hahn wrote: > > > But even if we are okay with this, I think it might be overkill to > > > enable the hugeTLB controller for the convenience of being able to inspect > > > the hugeTLB usage for cgroups. This is especially true in workloads where > > > we can predict what usage patterns will be like, and we do not need to enforce > > > specific limits on hugeTLB usage. > > > > I am sorry but I do not understand the overkill part of the argument. > > Is there any runtime or configuration cost that is visible? > > I think an argument could be made that any amount of incremental overhead > is unnecessary. With that said however, I think a bigger reason why this is > overkill is that a user who wishes to use the hugeTLB counter (which this > patch achieves in ~10 lines) should not have to enable a ~1000 line feature, > as Johannes suggested. > > A diligent user will have to spend time learning how the hugeTLB controller > works and figuring out the settings that will basically make the controller > do no enforcing (and basically, the same as if the feature was not enabled). > A not-so-diligent user will not spend the time to make sure that the configs > make sense, and may run into unexpected & unwanted hugeTLB behavior [1]. Heh, a lazy user would just enable the controller and hope for the best. And it would actually worked out of the box because there is no limit imposed by default so the only downside is a theoretical overhead due to charging. Anyway, I get the point and I guess it is fair to say the half baked memcg accounting is not optimal because it only provides half baked insight and you aim to fix that. This is fair intentention and justification. I have to say I really disliked this extension to the memcg when it was proposed but it was claimed this was good enough for people who know what they are doing. > > TL;DR > > 1) you need to make the stats accounting aligned with the existing > > charge accounting. > > 2) make the stat visible only when feature is enabled > > 3) work more on the justification - i.e. changelog part and give us a > > better insight why a) hugetlb cgroup is seen is a bad choice and b) why > > the original limitation hasn't proven good since the feature was > > introduced. > > > > Makes sense? > > -- > > Michal Hocko > > SUSE Labs > > Hi Michal, > > Thank you for your input. Yes -- this makes sense to me. I apologize, as it > seems that I definitely left a lot to be assumed & inferred, based on my > original patch changelog. > > In my next version of this patch, I am planning to add the changes that have > been suggested by Johannes, write code with the try_charge cleanup that > Shakeel suggested in mind, and change the behavior to make sense only when > hugeTLB accounting is enabled, as you suggested as well. I'll also update > the changelog & commit message and add any information that will hopefully > make future reviewers aware of the motivation for this patch. Thanks a lot! > Please let me know if you have any remaining concerns with the implementation > or motivation, and I will be happy to incorporate your ideas into the next > version as well. I think clarification and fixing the reporting is good enough. This still won't make the hugetlb sneaking into memcg more likeable to me but nothing that would force me awake during nights ;) Thanks! -- Michal Hocko SUSE Labs