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 DEAB6C31D97 for ; Thu, 4 Jul 2024 11:15:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3475D6B00AE; Thu, 4 Jul 2024 07:15:46 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F5F56B00B3; Thu, 4 Jul 2024 07:15:46 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1977E6B00B4; Thu, 4 Jul 2024 07:15:46 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id ED5816B00AE for ; Thu, 4 Jul 2024 07:15:45 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9160C41204 for ; Thu, 4 Jul 2024 11:15:45 +0000 (UTC) X-FDA: 82301815050.22.19F25A9 Received: from out30-99.freemail.mail.aliyun.com (out30-99.freemail.mail.aliyun.com [115.124.30.99]) by imf03.hostedemail.com (Postfix) with ESMTP id 6CAAC20020 for ; Thu, 4 Jul 2024 11:15:41 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b="sl/d/jMW"; spf=pass (imf03.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.99 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1720091710; 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=tN2ns8uWQRiOe8zzqBAzmjny+TBw0IJAiT3hxJ8URBU=; b=feeKt9KBR7WNWQgDoHC4LRrSIod0HGZjqvg/r3t013fEtblcKz1p/mBwAQLkCqDhTrU/wr 0RHlvnGecWhKzFXa+q6mwQslhNNPN5+UO8FaxG/o2KkSgWMYpERkw+7EQEv+qy4jQdYPoE Efxvmk50Z/p8H46q9d3L63VUDS1IHw0= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b="sl/d/jMW"; spf=pass (imf03.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.99 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com; dmarc=pass (policy=none) header.from=linux.alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1720091710; a=rsa-sha256; cv=none; b=TW/Y3qIONAoV/atwFWrASX6ymqWbYAjLQHiUkpFDQ9UuoUowr959Udi20GUtExinLSq+qZ x4fx48kDl6yOcj56r/T+NXMtuZ83f1rQmNJhJXDjbyKm/fkVkN8rtwxQuPGsJszf6ZSC8v wsK1HvvSkGJvWO/0j4wT9wU67q2BGxo= DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1720091738; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=tN2ns8uWQRiOe8zzqBAzmjny+TBw0IJAiT3hxJ8URBU=; b=sl/d/jMWYaxG4WGiHOmDkkEXSk0QCnLiWRtpTbmuxcSA7yMb+A4M7OGQGKMvtsXDbMwxY14G1U2eK4Zv0K+2l5nL4UR9NJP+zvKmrpPJCP9FWj9HaJYc/T7YKLtwbdCip5qRoShfah8EtgwCLksvTAiFTXtAx6iB2y4Ac4iez1w= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R211e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=maildocker-contentspam033045220184;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=15;SR=0;TI=SMTPD_---0W9qjM-5_1720091735; Received: from 30.97.56.65(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0W9qjM-5_1720091735) by smtp.aliyun-inc.com; Thu, 04 Jul 2024 19:15:36 +0800 Message-ID: <475332ea-a80b-421c-855e-a663d1d5bfc7@linux.alibaba.com> Date: Thu, 4 Jul 2024 19:15:35 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 4/6] mm: shmem: add mTHP support for anonymous shmem To: Ryan Roberts , akpm@linux-foundation.org, hughd@google.com Cc: willy@infradead.org, david@redhat.com, wangkefeng.wang@huawei.com, ying.huang@intel.com, 21cnbao@gmail.com, shy828301@gmail.com, ziy@nvidia.com, ioworker0@gmail.com, da.gomez@samsung.com, p.raghav@samsung.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <65796c1e72e51e15f3410195b5c2d5b6c160d411.1718090413.git.baolin.wang@linux.alibaba.com> <65c37315-2741-481f-b433-cec35ef1af35@arm.com> From: Baolin Wang In-Reply-To: <65c37315-2741-481f-b433-cec35ef1af35@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 6CAAC20020 X-Stat-Signature: ntdpg8rf36d1cinnmkof7qpmti7mc5ou X-Rspam-User: X-HE-Tag: 1720091741-497336 X-HE-Meta: U2FsdGVkX19zbcATudYukhuuinFb9A+V5Anz6Y4uL+bw8ywzEHW6HF2An+Vfl3i5dya0VwXQaHmbL7LRnBMYjypo62F9av3yiSjRCktXCCdWykTa+TAKYnyQtINu5EWk8Tk9bZBmi+84ZrvglNzsF6HUI1N5dfhhP8gi1B8u5KNkh7m6sChCL8PJn39GZ7pXqlthmJzZOQUkp8OThM0n3ptmGwEQno4SPaVIn8ALoxSZNiTMl9dIOgD352U2AcA64lX5aGtypWqwfkO+Ygnw4sZcnyLaw4p0RZ+QbG5WTucqdw9Tr3eT3ECPPUh8LR4mzoOoDoVetQhxWtb/dtdv3u+H7wAPboAU5d37hZSmG3PWker1YRqJlsViMfNi+xcSTD7SnK0O9yYbiv2RizaGgvW/LtvbDC8nszf+Louj9W9ZRgfvQQ3Mnj8kyxsABEjTUeqG7BZOXSC94Aouu4+gaM9/4j/LbljFyQfTtaVO82HofZxkrAks6Zp9t9ruhS1XPx2BACL85bC+MQN910MNHIqnoUjzmS5pu1xBcNAVuKLA6vmNpRA5JhvsQtkL3Y5dhtI3xZeMV7nQmalIAw4esQ+fDR1Aq23/7el8Eh5zzEIQfVH2bFVXe06rMKbyS5siQFCvYtimqoeQNQQIgT0yt45HKnbt9iATPwzMPCy13wY4+Xs7N8rE60PRKpiKUuBvfYyMySyJLzoPDTPzI86w7awtuYrNXW/9XhVUQLS44htD7sExoiADp4guybADXLeXt5zut12u577Bd5VWDagBLhUp/R9VSCnrw/ouvGcmcTo7P9TNk4CDUbWgfG4Dk864G1dQ1WMpaiMzVQ57QXCIxGCShC3f2DqtZGr5q333rl+70Fl4QovkvSnf4QXbc66sZkPvLWKNQhFvVZ2FGNJxHLbpNPk712DXJMk3hHSLPXjjT3yog0BDmWWDQLsFHD25jJZ3HADgGjkgwFAnGUM lrs5jyXT PsWFsxqL4/KP2fkhWEwVKaL7ixfiuIO5n8OLn0aKVJOTdzaV0KtRKNtFji6BJw3sy0trp2LkCAb0iP+L6UvutEmLHNyx/PD5skd1xenFO/DI17g49SOMoceYMHyzJgT5gNuENMcS5J0+YoijIhQkxslj1/YwFdqA1pPLkOffYNtuk3CGcPvHdp1M/S4o6SllOGJjhkOQyCe1vUI1rOawxb/dlWIvbA51Rf7mUPx92b6hYHc6CC471JHkvf04XtFt+a0Qwr6/PfYE4Il4fYSrG8dXOIidm2wgLzRRpL1pFz7RehJHRYjwSehHyoBPjYVAhhANC6U9fxy4SGDY3ojVf3ikzGGwBnvi1ZzG+3jU8KRiRiidcUMVm+NlfDvO2xTItCYAj6wD5uOAPIullqNN27XJigo6SkrfM5+hyQ72sErJX3Q0MaG+vF9hJNJ+LwQ9nr9qZUtsEN9FW1s0= 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 2024/7/4 01:25, Ryan Roberts wrote: > On 11/06/2024 11:11, Baolin Wang wrote: >> Commit 19eaf44954df adds multi-size THP (mTHP) for anonymous pages, that >> can allow THP to be configured through the sysfs interface located at >> '/sys/kernel/mm/transparent_hugepage/hugepage-XXkb/enabled'. >> >> However, the anonymous shmem will ignore the anonymous mTHP rule >> configured through the sysfs interface, and can only use the PMD-mapped >> THP, that is not reasonable. Users expect to apply the mTHP rule for >> all anonymous pages, including the anonymous shmem, in order to enjoy >> the benefits of mTHP. For example, lower latency than PMD-mapped THP, >> smaller memory bloat than PMD-mapped THP, contiguous PTEs on ARM architecture >> to reduce TLB miss etc. In addition, the mTHP interfaces can be extended >> to support all shmem/tmpfs scenarios in the future, especially for the >> shmem mmap() case. >> >> The primary strategy is similar to supporting anonymous mTHP. Introduce >> a new interface '/mm/transparent_hugepage/hugepage-XXkb/shmem_enabled', >> which can have almost the same values as the top-level >> '/sys/kernel/mm/transparent_hugepage/shmem_enabled', with adding a new >> additional "inherit" option and dropping the testing options 'force' and >> 'deny'. By default all sizes will be set to "never" except PMD size, >> which is set to "inherit". This ensures backward compatibility with the >> anonymous shmem enabled of the top level, meanwhile also allows independent >> control of anonymous shmem enabled for each mTHP. >> >> Signed-off-by: Baolin Wang > > [...] > > Hi Baolin, > > I'm currently trying to fix a bug where khugepaged is not started if only shmem > is enabled for THP. See discussion at [1]. It's been broken like this forever. > > Assuming anon and shmem THP are initially both disabled: > > # echo never > /sys/kernel/mm/transparent_hugepage/shmem_enabled > # echo never > /sys/kernel/mm/transparent_hugepage/enabled > > > Then shemem THP is enabled: > > # echo always > /sys/kernel/mm/transparent_hugepage/shmem_enabled > Thanks Ryan. Yes, this is a real problem. > As part of investigating the fix, I stumbled upon this patch, which I remember > reviewing an early version of but I've been out for a while and missed the > latter versions. See below for comments and questions; the answers to which will > help me figure out how to fix the above... > > [1] https://lore.kernel.org/linux-mm/20240702144617.2291480-1-ryan.roberts@arm.com/ > > >> >> +#ifdef CONFIG_TRANSPARENT_HUGEPAGE >> +static unsigned long shmem_allowable_huge_orders(struct inode *inode, >> + struct vm_area_struct *vma, pgoff_t index, >> + bool global_huge) >> +{ >> + unsigned long mask = READ_ONCE(huge_shmem_orders_always); >> + unsigned long within_size_orders = READ_ONCE(huge_shmem_orders_within_size); >> + unsigned long vm_flags = vma->vm_flags; >> + /* >> + * Check all the (large) orders below HPAGE_PMD_ORDER + 1 that >> + * are enabled for this vma. >> + */ >> + unsigned long orders = BIT(PMD_ORDER + 1) - 1; >> + loff_t i_size; >> + int order; >> + >> + if ((vm_flags & VM_NOHUGEPAGE) || >> + test_bit(MMF_DISABLE_THP, &vma->vm_mm->flags)) >> + return 0; >> + >> + /* If the hardware/firmware marked hugepage support disabled. */ >> + if (transparent_hugepage_flags & (1 << TRANSPARENT_HUGEPAGE_UNSUPPORTED)) >> + return 0; >> + >> + /* >> + * Following the 'deny' semantics of the top level, force the huge >> + * option off from all mounts. >> + */ >> + if (shmem_huge == SHMEM_HUGE_DENY) >> + return 0; > > I don't quite get this, I don't think its the desirable behaviour. This is > saying that if the top-level shemem_enabled control is set to 'deny', then all > mTHP sizes, regardless of their control's setting are disabled? > > The anon controls don't work like that; you can set the top-level control to > anything you like, but its only consumed if the per-size controls are inheriting it. IMO, 'deny' option is not similar like 'never' option. > > So for the deny case, wouldn't it be better to allow that as an option on all > the per-size controls (and implicitly let it be inherrited from the top-level)? From 'deny' option's semantics: "disables huge on shm_mnt and all mounts, for emergency use;" It is usually used for testing to shut down all huge pages from the old ages. Moreover, mTHP interfaces will be used as a huge order filter to support tmpfs, so I think it will make life easier to disable all huge orders for testing or emergency use, as well as to maintain the original semantics. >> + >> + /* >> + * Only allow inherit orders if the top-level value is 'force', which >> + * means non-PMD sized THP can not override 'huge' mount option now. >> + */ >> + if (shmem_huge == SHMEM_HUGE_FORCE) >> + return READ_ONCE(huge_shmem_orders_inherit); > > I vaguely recall that we originally discussed that trying to set 'force' on the > top level control while any per-size controls were set to 'inherit' would be an > error, and trying to set 'force' on any per-size control except the PMD-size > would be an error? Right. > I don't really understand this logic. Shouldn't we just be looking at the > per-size control settings (or the top-level control as a proxy for every > per-size control that has 'inherit' set)? ‘force’ will apply the huge orders for anon shmem and tmpfs, so now we only allow pmd-mapped THP to be forced. We should not look at per-size control settings for tmpfs now (mTHP for tmpfs will be discussed in future). > > Then for tmpfs, which doesn't support non-PMD-sizes yet, we just always use the > PMD-size control for decisions. > > I'm also really struggling with the concept of shmem_is_huge() existing along > side shmem_allowable_huge_orders(). Surely this needs to all be refactored into > shmem_allowable_huge_orders()? I understood. But now they serve different purposes: shmem_is_huge() will be used to check the huge orders for the top level, for *tmpfs* and anon shmem; whereas shmem_allowable_huge_orders() will only be used to check the per-size huge orders for anon shmem (excluding tmpfs now). However, as I plan to add mTHP support for tmpfs, I think we can perform some cleanups. >> + /* Allow mTHP that will be fully within i_size. */ >> + order = highest_order(within_size_orders); >> + while (within_size_orders) { >> + index = round_up(index + 1, order); >> + i_size = round_up(i_size_read(inode), PAGE_SIZE); >> + if (i_size >> PAGE_SHIFT >= index) { >> + mask |= within_size_orders; >> + break; >> + } >> + >> + order = next_order(&within_size_orders, order); >> + } >> + >> + if (vm_flags & VM_HUGEPAGE) >> + mask |= READ_ONCE(huge_shmem_orders_madvise); >> + >> + if (global_huge) > > Perhaps I've misunderstood global_huge, but I think its just the return value > from shmem_is_huge()? But you're also using shmem_huge directly in this Yes. > function. I find it all rather confusing. I think I have explained why need these logics as above. Since mTHP support for shmem has just started (tmpfs is still in progress). I will make it more clear in the following patches. > Sorry if all this was discussed and agreed in my absence! But I think this needs > to be sorted in order to do the bug fix for khugepaged properly. No worries. Thanks for your input.