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 7BD1FC30658 for ; Fri, 5 Jul 2024 09:34:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 116A06B00A6; Fri, 5 Jul 2024 05:34:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0C83C6B00A7; Fri, 5 Jul 2024 05:34:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF7B46B00A8; Fri, 5 Jul 2024 05:34:50 -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 D1B056B00A6 for ; Fri, 5 Jul 2024 05:34:50 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 79413817E5 for ; Fri, 5 Jul 2024 09:34:50 +0000 (UTC) X-FDA: 82305189540.15.5E6B5F0 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf19.hostedemail.com (Postfix) with ESMTP id A1F211A0002 for ; Fri, 5 Jul 2024 09:34:48 +0000 (UTC) Authentication-Results: imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720172069; a=rsa-sha256; cv=none; b=Ex0CcGDcFbzVD1uyt38eGn2lnRIpd50E/axabWWQIeugQLcPKz2L1e9A/khID5Moazxsd2 WbC86AURr6oyBI+yOvug1R/0Kfe2h0J2WGypEOF083/hCGhg1yetlwTbFm6MPVCKaqRW8E gw0u2wcv3Sr8tQKnR88G09qJ97+mwVg= ARC-Authentication-Results: i=1; imf19.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf19.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=1720172069; 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=9XGGc10XFXbAxxIQL/fDXXS61ySslc2aio8IxjBTUZM=; b=jRkawb8cmKbzvoTN5jcXguRnCNrFJZ9+XWa0sVGYC+RjZlb3qFmZdOePl9+1EeKHlxm0hZ ZvkhupNe+EJuX6ylAmpAoctYlRDBKg7Dq50Ex0LtKO442dgmwLzY61Rgl0RM93VWH10zAc UkUjkwhPR3qessneNDykyFXHAcK5IYU= 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 1DF55367; Fri, 5 Jul 2024 02:35:13 -0700 (PDT) Received: from [10.57.74.223] (unknown [10.57.74.223]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 569DD3F762; Fri, 5 Jul 2024 02:34:45 -0700 (PDT) Message-ID: Date: Fri, 5 Jul 2024 10:34:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] mm: Fix khugepaged activation policy Content-Language: en-GB To: Andrew Morton Cc: Jonathan Corbet , David Hildenbrand , Barry Song , Baolin Wang , Lance Yang , Yang Shi , linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20240704091051.2411934-1-ryan.roberts@arm.com> <20240704113325.fb9f1b04f99abaac315b5c88@linux-foundation.org> From: Ryan Roberts In-Reply-To: <20240704113325.fb9f1b04f99abaac315b5c88@linux-foundation.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: A1F211A0002 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: hm735smfh8asg7fhew3rhfm88qniji5i X-HE-Tag: 1720172088-664584 X-HE-Meta: U2FsdGVkX19hIwSyJrpkug1TrLqIQlcv5Q9NBEHTsJRhhyW5HWk2jew3p8FEqYv9Q/VGNTwqJV/aDsjCSeJ9HA7WSAqvE3Jm8kpSvZLFp7tKckAj23ByGmkQkkVnAkTCs00g+Qjtn9zNnp3qb+SC2KLHD7UsZNVpL9BJ24GRM5wVYSDLBZK8vo2q7GwUXozJuWKa07/JWlzv8TWPdunPLw9EbRxWl3CJSfwdlhffwJGNkPsG8YthWW7MDpjTe9x5DtMnJnO+9WJjBewLfymv46oaNy7l4xIlIupycCU12r9bXx7N70NW8ype5iUqBgzAqRJ1+OrMFdiEY0VnLZDFw6OzkzPz9mlbWkZn1amyztA5LG2532ifCkDQ0Tn+KUMY4fUW0ihQLMxo0hniPFZeyVFXFNWlb+WCEZTcpeCw14RGNywDoEPbq/x76Ui/TyAy0r/ew/loOmgp+QEEOjbCyKXaR0YHhXcila00Gh3z/YxphT1iE63cQlrVBDuQELuTEurudVFE6rKNC5rqstzPy2EEIFgOAKUrUIqz3S2EBH+Qjtu6QjKf9/MFgPNa1MnyMBYvgkw76Q1CT/Jf/Z7oV4X+1XCyrtkGtYSNFAc3vnHUd6PehI2mkQoABKRDLG/VtOAc4J5VpAs4TmzA9UXmow/nCC5TfOBnpuHx1pRqigBjWYeXAuLoWbhIr2aNQ9BxK2p2iLAQSzhtuesDmwUnyOzNPtYIxd/OcufGtWoemZGXrUsHdamRL4vK/fB77Bwm1DxCd2M1RcWgm9yddCmDZA2QBu4NmPTdDqWa7G9BnbV2odqfD/G7kuACHhbqHfYtqu1Pk5lifojSprdN0MD7a0g25HE+ij6z7Cw+J865HA6nPhkfOBWBPC+NgpEDcX0xCa9PiFQNoDjdCdydmCk1gzjTSKfrZCk5X34+LyELCMeESrmHRoW51V9j9NvHipKI80+Mbt/8t0wElhk5shf c/JPL2d9 GIjvbjVk5YBtV/NwedHdF2b7pkNz8PReosuGivCmaK3OZ0w2A2kCrqga9q4CdIdYS6mTMKTDvf4rT/ti5ouaC4KSs79S0FQWd+8UpXZwuI8vH8BTM9hnaf+HRZcybPPpxy7BoAkixFKl/W2SjubJnBxbhcJSC5nVlXem+/g0wPXGSanDx0T6HL68qJpUOJ2LxZPMGrX9Dwu35Qq4= 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 04/07/2024 19:33, Andrew Morton wrote: > On Thu, 4 Jul 2024 10:10:50 +0100 Ryan Roberts wrote: > >> Since the introduction of mTHP, the docuementation has stated that >> khugepaged would be enabled when any mTHP size is enabled, and disabled >> when all mTHP sizes are disabled. There are 2 problems with this; 1. >> this is not what was implemented by the code and 2. this is not the >> desirable behavior. >> >> Desirable behavior is for khugepaged to be enabled when any PMD-sized >> THP is enabled, anon or file. (Note that file THP is still controlled by >> the top-level control so we must always consider that, as well as the >> PMD-size mTHP control for anon). khugepaged only supports collapsing to >> PMD-sized THP so there is no value in enabling it when PMD-sized THP is >> disabled. So let's change the code and documentation to reflect this >> policy. >> >> Further, per-size enabled control modification events were not >> previously forwarded to khugepaged to give it an opportunity to start or >> stop. Consequently the following was resulting in khugepaged eroneously >> not being activated: >> >> echo never > /sys/kernel/mm/transparent_hugepage/enabled >> echo always > /sys/kernel/mm/transparent_hugepage/hugepages-2048kB/enabled >> >> ... >> >> -static inline bool hugepage_flags_enabled(void) >> +static inline bool hugepage_pmd_enabled(void) >> { >> /* >> - * We cover both the anon and the file-backed case here; we must return >> - * true if globally enabled, even when all anon sizes are set to never. >> - * So we don't need to look at huge_anon_orders_inherit. >> + * We cover both the anon and the file-backed case here; file-backed >> + * hugepages, when configured in, are determined by the global control. >> + * Anon pmd-sized hugepages are determined by the pmd-size control. >> */ >> - return hugepage_global_enabled() || >> - READ_ONCE(huge_anon_orders_always) || >> - READ_ONCE(huge_anon_orders_madvise); >> + return (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && hugepage_global_enabled()) || >> + test_bit(PMD_ORDER, &huge_anon_orders_always) || >> + test_bit(PMD_ORDER, &huge_anon_orders_madvise) || >> + (test_bit(PMD_ORDER, &huge_anon_orders_inherit) && hugepage_global_enabled()); >> } > > That's rather a mouthful. Is this nicer? Sure, I'll take your version into v3. > > static inline bool hugepage_pmd_enabled(void) > { > /* > * We cover both the anon and the file-backed case here; file-backed > * hugepages, when configured in, are determined by the global control. > * Anon pmd-sized hugepages are determined by the pmd-size control. > */ > if (IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && > hugepage_global_enabled()) > return true; > if (test_bit(PMD_ORDER, &huge_anon_orders_always)) > return true; > if (test_bit(PMD_ORDER, &huge_anon_orders_madvise)) > return true; > if (test_bit(PMD_ORDER, &huge_anon_orders_inherit) && > hugepage_global_enabled()) > return true; > return false; > } > > Also, that's a pretty large function to be inlined. It could be a > non-inline function static to khugepaged.c. But I suppose that's a > separate patch. Yeah fair point. I'll respin it now as a static in khugepaged.c.