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 E8A41C54E49 for ; Thu, 7 Mar 2024 09:07:30 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7C4FE6B0132; Thu, 7 Mar 2024 04:07:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 774F36B0133; Thu, 7 Mar 2024 04:07:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 63C986B0134; Thu, 7 Mar 2024 04:07:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 542156B0132 for ; Thu, 7 Mar 2024 04:07:30 -0500 (EST) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id EDA95C1056 for ; Thu, 7 Mar 2024 09:07:29 +0000 (UTC) X-FDA: 81869664618.28.34ED950 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf12.hostedemail.com (Postfix) with ESMTP id 28AD040005 for ; Thu, 7 Mar 2024 09:07:27 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=none; spf=pass (imf12.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709802448; 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=7U6S0uMjoY5464xHIEpQ6A5eiW0P7LQ9cpiCqoLbS6A=; b=g5QkBoQwnE5yosMBgy1XjreFt5DgJiAatvOiSQe8XAIaL0KUQ4ZDEwQRmoHnQ6SCCtvplI kDnq7G0EO80BR5zLvNx7DH0siLa5BRsauQ3ZfpKlsYVhK0PRgetV8ODdrkvBEiGDSwKrQU 9OLddFsIMCbCR9SlfKvQg4BVYm8Qxk0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709802448; a=rsa-sha256; cv=none; b=yoBfLjLcmY1WqLmoe4faSYhhKaf4RpAJ0MsJ9hzq/+S4qIlM+ve+lA4CkKXRykIODF3Nax s3HY9b2Ilo12Xx/AtiSaNrmvnGRFZGUE7Y9WVjNActqTKA0X9qdRHXteQm2GukW7RnOIqx vYsRGAYkReGjpv+2GSg7/TrTTcFCO8M= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=none; spf=pass (imf12.hostedemail.com: domain of ryan.roberts@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=ryan.roberts@arm.com; dmarc=pass (policy=none) header.from=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 4AFA11FB; Thu, 7 Mar 2024 01:08:04 -0800 (PST) Received: from [10.57.68.241] (unknown [10.57.68.241]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3BB813F762; Thu, 7 Mar 2024 01:07:24 -0800 (PST) Message-ID: <03458c20-5544-411b-9b8d-b4600a9b802f@arm.com> Date: Thu, 7 Mar 2024 09:07:22 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/1] mm/madvise: enhance lazyfreeing with mTHP in madvise_free Content-Language: en-GB To: Barry Song <21cnbao@gmail.com>, Lance Yang , david@redhat.com, Vishal Moola Cc: akpm@linux-foundation.org, zokeefe@google.com, shy828301@gmail.com, mhocko@suse.com, fengwei.yin@intel.com, xiehuan09@gmail.com, wangkefeng.wang@huawei.com, songmuchun@bytedance.com, peterx@redhat.com, minchan@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20240307061425.21013-1-ioworker0@gmail.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 28AD040005 X-Rspam-User: X-Stat-Signature: 6ie84eo448c8ukr8hrrwbyyc6nwuc8be X-Rspamd-Server: rspam03 X-HE-Tag: 1709802447-70688 X-HE-Meta: U2FsdGVkX1/owcE7LAF1DavCneZ48Ly5ptnp8Q04nn7XKNxh48xy7Hgepp9Ipd32DNvYQalEN2wl2zGy0Z5J4SBMRGn6BWTP5WFe6tn7ejviTXVcyszaZ4tlf1ghLfuBZs9Awob9UcX3hLIg7qmPnVpfFL1bBWutvP938pD4/tD8XRaRV6sqEc59fch81taSMgqjUoXqvHJotLqDNa4arcpZMvGduhlr6J7BntbNYusUc7sr1mYBe6Speh+DVNgnLdGiLIivPG0AInvhf5NKE1+EOowOj34NeqVYfSVtTuiAzMPIYzYSVqj+L0C/FXEaPHL+qWVFrRpxDCTXNvZFsSYZeK+52km+oZcik33wKzL25mKzGYPtOvwbCp74ZNxPQAdiD7G2c3HDLg6N/tiq1nP7o2KXZrZlv+VDkxmGJo+ZmQRif4ORbKYjVNgIyBpOEm2BHwrKYHV3n+J9Vep+5n5luaNKGmaZRnqYInTA2fUs+Cy1+WiNyCD/bZ+/nmXqLbmaiGgsgAsj3HGRTpzqRYq7bZeQJZi3Pg6gcAOvgNHeSx3AYLJfr4UmODA0yGVzYWAD8CC4LmqCGCGATusHsCpbZSWtAhb2/kCXyp5VEu7KfRH/VyY8QwAcy74SDKwFFBLMuEwpZ/VPlnQWEPcNdLUfDFsEpZEaAapKyQirROeoDjEhr9c3IqopnpTCpzRFEzvPWW9/dX3W0zXhrWUnJZeayB5Z83PSix1eqpzDQZKQISACrQWnE7oIY0U5cffK4qd5CIY1WIpUoR9Qc1gTl2lKH6sk6W+a6pnFW5qWHa8eKtrg2X1euq2X2Ml6RAapTG4hUfdU9YuGfMEapYRagnlWrUEtYyhDQuB4TjcqGTz2oySQbe79Tg3nLR/9qIc5V9zPWkhwdSjs/N4qAN+fnLtukot7ag3hV97zBKUb4cxEB0wMVCdGEsnjW2Nc57WsuV2DXLi7X7ve0qBFwBO hLe0l/lE XPRI+zXJNH1oTcrcgk17KfiHrDYlOa3TSILbd7TpIfkN2t4JPyTNcJrSf56J/p/LQSKCKVJS/k9+FG14ElY9wYkplfhHh9e0d//MKoAd9+kSAWXEMOEhZ5KT/jx9ap2I/90qw31npj7ofz0QeQ2dogvc0libLmZe3+dHXR9uA+yu8WkpvpcLbsBWhaIKXVB9TnyiUoEvSyVPjmPBkw4hoiUxnK66Y0J0FVNNYf/Ci7zWzxXO6YHfoNXyhnCXSQXyoJmXtqBYEVIhhhVLr6TCDMBrL2T31scQb9XR/NE1lw+QHWuI9UdqeTNWHQWNPMRdKYiaK8C+ueoZM6SRL4uxStdjHAG0B7fw7l6cW8Ac8AWr242QzfOYjQXAMwKpo2IkfZcOESkcsDxqt+Nw= 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 07/03/2024 08:10, Barry Song wrote: > On Thu, Mar 7, 2024 at 9:00 PM Lance Yang wrote: >> >> Hey Barry, >> >> Thanks for taking time to review! >> >> On Thu, Mar 7, 2024 at 3:00 PM Barry Song <21cnbao@gmail.com> wrote: >>> >>> On Thu, Mar 7, 2024 at 7:15 PM Lance Yang wrote: >>>> >> [...] >>>> +static inline bool can_mark_large_folio_lazyfree(unsigned long addr, >>>> + struct folio *folio, pte_t *start_pte) >>>> +{ >>>> + int nr_pages = folio_nr_pages(folio); >>>> + fpb_t flags = FPB_IGNORE_DIRTY | FPB_IGNORE_SOFT_DIRTY; >>>> + >>>> + for (int i = 0; i < nr_pages; i++) >>>> + if (page_mapcount(folio_page(folio, i)) != 1) >>>> + return false; >>> >>> we have moved to folio_estimated_sharers though it is not precise, so >>> we don't do >>> this check with lots of loops and depending on the subpage's mapcount. >> >> If we don't check the subpage’s mapcount, and there is a cow folio associated >> with this folio and the cow folio has smaller size than this folio, >> should we still >> mark this folio as lazyfree? > > I agree, this is true. However, we've somehow accepted the fact that > folio_likely_mapped_shared > can result in false negatives or false positives to balance the > overhead. So I really don't know :-) > > Maybe David and Vishal can give some comments here. > >> >>> BTW, do we need to rebase our work against David's changes[1]? >>> [1] https://lore.kernel.org/linux-mm/20240227201548.857831-1-david@redhat.com/ >> >> Yes, we should rebase our work against David’s changes. >> >>> >>>> + >>>> + return nr_pages == folio_pte_batch(folio, addr, start_pte, >>>> + ptep_get(start_pte), nr_pages, flags, NULL); >>>> +} >>>> + >>>> static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr, >>>> unsigned long end, struct mm_walk *walk) >>>> >>>> @@ -676,11 +690,45 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr, >>>> */ >>>> if (folio_test_large(folio)) { >>>> int err; >>>> + unsigned long next_addr, align; >>>> >>>> - if (folio_estimated_sharers(folio) != 1) >>>> - break; >>>> - if (!folio_trylock(folio)) >>>> - break; >>>> + if (folio_estimated_sharers(folio) != 1 || >>>> + !folio_trylock(folio)) >>>> + goto skip_large_folio; >>> >>> >>> I don't think we can skip all the PTEs for nr_pages, as some of them might be >>> pointing to other folios. >>> >>> for example, for a large folio with 16PTEs, you do MADV_DONTNEED(15-16), >>> and write the memory of PTE15 and PTE16, you get page faults, thus PTE15 >>> and PTE16 will point to two different small folios. We can only skip when we >>> are sure nr_pages == folio_pte_batch() is sure. >> >> Agreed. Thanks for pointing that out. >> >>> >>>> + >>>> + align = folio_nr_pages(folio) * PAGE_SIZE; >>>> + next_addr = ALIGN_DOWN(addr + align, align); >>>> + >>>> + /* >>>> + * If we mark only the subpages as lazyfree, or >>>> + * cannot mark the entire large folio as lazyfree, >>>> + * then just split it. >>>> + */ >>>> + if (next_addr > end || next_addr - addr != align || >>>> + !can_mark_large_folio_lazyfree(addr, folio, pte)) >>>> + goto split_large_folio; >>>> + >>>> + /* >>>> + * Avoid unnecessary folio splitting if the large >>>> + * folio is entirely within the given range. >>>> + */ >>>> + folio_clear_dirty(folio); >>>> + folio_unlock(folio); >>>> + for (; addr != next_addr; pte++, addr += PAGE_SIZE) { >>>> + ptent = ptep_get(pte); >>>> + if (pte_young(ptent) || pte_dirty(ptent)) { >>>> + ptent = ptep_get_and_clear_full( >>>> + mm, addr, pte, tlb->fullmm); >>>> + ptent = pte_mkold(ptent); >>>> + ptent = pte_mkclean(ptent); >>>> + set_pte_at(mm, addr, pte, ptent); >>>> + tlb_remove_tlb_entry(tlb, pte, addr); >>>> + } >>> >>> Can we do this in batches? for a CONT-PTE mapped large folio, you are unfolding >>> and folding again. It seems quite expensive. I'm not convinced we should be doing this in batches. We want the initial folio_pte_batch() to be as loose as possible regarding permissions so that we reduce our chances of splitting folios to the min. (e.g. ignore SW bits like soft dirty, etc). I think it might be possible that some PTEs are RO and other RW too (e.g. due to cow - although with the current cow impl, probably not. But its fragile to assume that). Anyway, if we do an initial batch that ignores all that then do this bit as a batch, you will end up smeering all the ptes with whatever properties were set on the first pte, which probably isn't right. I've done a similar conversion for madvise_cold_or_pageout_pte_range() as part of my swap-out series v4 (hoping to post imminently, but still working out a latent bug that it triggers). I use ptep_test_and_clear_young() in that, which arm64 can apply per-pte but avoid doing a contpte unfold/fold. I know you have to clear dirty here too, but I think this pattern is preferable. FYI, my swap-out series also halfway-batches madvise_free_pte_range() so that I can batch free_swap_and_cache() for the swap entry case. Ideally the work you are doing here would be rebased on top of that and plug-in to the approach implemented there. (subject to others' views of course). I'll cc you when I post it. >> >> Thanks for your suggestion. I'll do this in batches in v3. >> >> Thanks again for your time! >> >> Best, >> Lance >> >>> >>>> + } >>>> + folio_mark_lazyfree(folio); >>>> + goto next_folio; >>>> + >>>> +split_large_folio: >>>> folio_get(folio); >>>> arch_leave_lazy_mmu_mode(); >>>> pte_unmap_unlock(start_pte, ptl); >>>> @@ -688,13 +736,28 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr, >>>> err = split_folio(folio); >>>> folio_unlock(folio); >>>> folio_put(folio); >>>> - if (err) >>>> - break; >>>> - start_pte = pte = >>>> - pte_offset_map_lock(mm, pmd, addr, &ptl); >>>> - if (!start_pte) >>>> - break; >>>> - arch_enter_lazy_mmu_mode(); >>>> + >>>> + /* >>>> + * If the large folio is locked or cannot be split, >>>> + * we just skip it. >>>> + */ >>>> + if (err) { >>>> +skip_large_folio: >>>> + if (next_addr >= end) >>>> + break; >>>> + pte += (next_addr - addr) / PAGE_SIZE; >>>> + addr = next_addr; >>>> + } >>>> + >>>> + if (!start_pte) { >>>> + start_pte = pte = pte_offset_map_lock( >>>> + mm, pmd, addr, &ptl); >>>> + if (!start_pte) >>>> + break; >>>> + arch_enter_lazy_mmu_mode(); >>>> + } >>>> + >>>> +next_folio: >>>> pte--; >>>> addr -= PAGE_SIZE; >>>> continue; >>>> -- >>>> 2.33.1 >>>> > > Thanks > Barry