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 85DB6C3DA41 for ; Tue, 9 Jul 2024 07:59:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 181C46B009C; Tue, 9 Jul 2024 03:59:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 131826B009D; Tue, 9 Jul 2024 03:59:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F3AB26B009E; Tue, 9 Jul 2024 03:59:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id D4E126B009C for ; Tue, 9 Jul 2024 03:59:15 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 74C191617A8 for ; Tue, 9 Jul 2024 07:59:15 +0000 (UTC) X-FDA: 82319463870.23.2DC7FC3 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf29.hostedemail.com (Postfix) with ESMTP id 9839F12000A for ; Tue, 9 Jul 2024 07:59:13 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf29.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720511919; 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; bh=SeP0nSeQfct1ZjsKbyV/Qu8fR2Tt1NnUwy9Q+LGLRUk=; b=8isB9v230COoHWbdOVCG4Gvhy/QppvU/GiCG6RKKdopAqfJovLBYBK7MGA17/n9mqdXirR mESLYYP1GQsLI4E258mTO4hFfe20nV0oT6VeGIV1cA1hdavIdKgyudSaidvJqe5RHgMtHP oe8xjuzOgp4WoJiLqoqyIXSVj8rZDbE= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720511919; a=rsa-sha256; cv=none; b=lXjXPhJSikJT/FhlXLjScrat3+zsnv+zUPScRvVD5PUwbaFmJwBaZW0MHYgK1UQgjEDBW2 pGksE7xqEfBuYPMdAAr+/NgbXh7p1ChY801L8A3GkaaKHH/2SA0QRu/OJTs3lGC+IDSM6j 1v694I1Llc5Sd2wJRuxp8Nz5LdGI/Jw= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf29.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 747461042; Tue, 9 Jul 2024 00:59:37 -0700 (PDT) Received: from [10.57.76.194] (unknown [10.57.76.194]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 278B13F762; Tue, 9 Jul 2024 00:59:09 -0700 (PDT) Message-ID: Date: Tue, 9 Jul 2024 08:59:08 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] mm: shmem: Rename mTHP shmem counters Content-Language: en-GB To: David Hildenbrand , Barry Song Cc: Andrew Morton , Hugh Dickins , Jonathan Corbet , Baolin Wang , Lance Yang , Matthew Wilcox , Zi Yan , Daniel Gomez , linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20240708112445.2690631-1-ryan.roberts@arm.com> <744749c3-4506-40d9-ac48-0dbc59689f92@arm.com> <10b201b1-53d3-4f62-be8e-996aa95d2b99@redhat.com> <0240add9-4c56-4f66-b761-494cc2cf8fb5@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 9839F12000A X-Stat-Signature: az8jwq5c8hp5j5br3ark9p5tr819d7x5 X-Rspam-User: X-HE-Tag: 1720511953-608163 X-HE-Meta: U2FsdGVkX190uSwsHXo5a9mzEQKsaAZDHJdaJYgVp76NL+ACqO35Nu5TtKvYpIdVa6jdbq66jT03YHLMzBCmtU1FGpEZ2CuLq712X/yZ3kbzPFc3KgwnGuRVpr/eWn04+vu4HXMK5LQIii3D7nhf1JMrZ81Xo9D5umStA8bMQWyuR0K5XHMyjk9t4ZZbOPfym973ZErHgTGf980bPZ2k5riP8Jc7qOPsf4j9TKY1onC3vjQLSnzJPO4cajm7AsdaGsUQK2ijwlPT1JaxnIsxW4+02+ux6exhuLQhE2BumdwPpKFywh140MyLp29ZZ+Nw16umvw6n3Ie4QKhXrlWuHe/fdPx5eWCQDQ3/ho+j6ri0UQ4eLld/Z8gZEtQSvtQeKGVL1KPghlv/ERADYR+JRL4AmKdxnggKj2FP24712OL1ZaIytc5RZYTnbAXmEohOYJsqb3vK8SWo0UmHDypf6ok84ughiUeGToLGrDwwxoDACcCLLc1lU7ERFZdC4Rp6MEAj4Rw1stzJhjKOzDd9tjg7n4WAhun9Lh4T5lbfBJq1K4vkJB0VnfvMPp1uY6C8YKnUTloK5202I86iqo07WQ+kSNu+HlyCEDgw2MLhdj4FJfCh6QX7zOWW4Ob+DOdsL1M5QUMShw8DqpcwsBNJ+heEUZlEMQGm+nde84HXnT/tTqZ/O03wsDlDy4AnHlTPaWRmhCe86hzfAmq1jtfohZft3fmwlvBNbCuVVfDStKESL9zVo0pktloFhokuBPzLi6917MPn9+YgiuHVrfYyTSdzouZaxBTQdn4XXsxjOyY+IIoTpCgrKnwcgMfc12Dlp1uOQKo3Ntcx4PdjYnc4se0FCztdy7JrxS0glHxLZmTNtK+h4JVktX/0Q3+fhigS/GcUEftr1XkxXLx9ADvIQTmpVRmHf7kQCY0fwSFPBWGG0e1Phyjx1q+F9yFYrjk9vwD/lFwEE9XfmIr4XCc KdsJM70j QQflNyEqOYwx4bQ+eRnlVPAVood2y24aJKLvam0VAxWuuHWhUBI37GP1/OLKLiXZ7+iCjByA+lLUm1yFFRaKIpfVVzq7ZuKACeSsNn+9PuzfQ4k/epZzdqs60zem7yz8QYyrwrdPaBwkFowylaa1+J8ZLMm+Acx+oDozD3Ku2EbAd2mr/qRnA/A1qIMSgP/55f+n7xHVSh2K3bTgQO16cC8c72PhLlTnlob1WejGhwM+xMrA= 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 09/07/2024 08:54, David Hildenbrand wrote: > On 09.07.24 09:47, Ryan Roberts wrote: >> On 08/07/2024 21:50, David Hildenbrand wrote: >>> On 08.07.24 14:29, Ryan Roberts wrote: >>>> On 08/07/2024 12:36, Barry Song wrote: >>>>> On Mon, Jul 8, 2024 at 11:24 PM Ryan Roberts wrote: >>>>>> >>>>>> The legacy PMD-sized THP counters at /proc/vmstat include >>>>>> thp_file_alloc, thp_file_fallback and thp_file_fallback_charge, which >>>>>> rather confusingly refer to shmem THP and do not include any other types >>>>>> of file pages. This is inconsistent since in most other places in the >>>>>> kernel, THP counters are explicitly separated for anon, shmem and file >>>>>> flavours. However, we are stuck with it since it constitutes a user ABI. >>>>>> >>>>>> Recently, commit 66f44583f9b6 ("mm: shmem: add mTHP counters for >>>>>> anonymous shmem") added equivalent mTHP stats for shmem, keeping the >>>>>> same "file_" prefix in the names. But in future, we may want to add >>>>>> extra stats to cover actual file pages, at which point, it would all >>>>>> become very confusing. >>>>>> >>>>>> So let's take the opportunity to rename these new counters "shmem_" >>>>>> before the change makes it upstream and the ABI becomes immutable. >>>>> >>>>> Personally, I think this approach is much clearer. However, I recall >>>>> we discussed this >>>>> before [1], and it seems that inconsistency is a concern? >>>> >>>> Embarrassingly, I don't recall that converstation at all :-| but at least >>>> what I >>>> said then is consistent with what I've done in this patch. >>>> >>>> I think David's conclusion from that thread was to call them FILE_, and add >>>> both >>>> shmem and pagecache counts to those counters, meaning we can keep the same name >>>> as legacy THP counters. But those legacy THP counters only count shmem, and I >>>> don't think we would get away with adding pagecache counts to those at this >>>> point? (argument: they have been around for long time and there is a risk that >>>> user space relies on them and if they were to dramatically increase due to >>>> pagecache addition now that could break things). In that case, there is still >>>> inconsistency, but its worse; the names are consistent but the semantics are >>>> inconsistent. >>>> >>>> So my vote is to change to SHMEM_ as per this patch :) >>> >>> I also forgot most of the discussion, but these 3 legacy counters are really >>> only (currently) incremented for shmem. I think my idea was to keep everything >>> as FILE_ for now, maybe at some point make the pagecache also use them, and then >>> maybe have separate FILE_ + SHMEM_. >>> >>> But yeah, likely it's best to only have "shmem" here for now, because who knows >>> what we can actually change about the legacy counters. But it's always though >>> messing with legacy stuff that is clearly suboptimal ... >> >> Sorry David, I've read your response a bunch of times and am still not >> completely sure what you are advocating for. >> >> To make my proposal crystal clear, I think we should leave the legacy counters >> alone - neither change their name nor what they count (which is _only_ shmem). I >> think we should rename the new mTHP counters to "shmem" and have them continue >> to only count shmem. Then additionally, I think we should introduce new "file" >> mTHP counters that count the page cache allocations (that's a future set of >> patches, which I'm working on together with user controls to determine which >> mTHP sizes can be used by page cache). >> >> As suggested by Barry, I propose to also improve the documentation for the >> legacy counters to make it clear that dispite their name being "file" they are >> actually counting "shmem". I'll do this for v2. > > Yes, and please. Likely we should document for the legacy ones (if not already > done) that they only track PMD-sized THPs. The PMD-ness is already documented for the legacy counters IIRC, but I'll double check. > >> >> David, would you support this approach? If so, I'd like to push this forwards >> asap so that it gets into v6.11 to avoid ever exposing the mthp counters with >> the "file" name. > > Yes, sorry for not being clear. No worries, it sounds like at least Lance understood exectly what you were saying, so likely I'm just being obtuse. Anyway, I'll respin with the docs improvements and get v2 out shortly. Thanks, Ryan