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 01BD8C4167B for ; Thu, 30 Nov 2023 12:00:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9917F8D004C; Thu, 30 Nov 2023 07:00:54 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 941628D0001; Thu, 30 Nov 2023 07:00:54 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 830BB8D004C; Thu, 30 Nov 2023 07:00:54 -0500 (EST) 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 744938D0001 for ; Thu, 30 Nov 2023 07:00:54 -0500 (EST) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 3FAD2A018E for ; Thu, 30 Nov 2023 12:00:54 +0000 (UTC) X-FDA: 81514479228.21.5A3FCBB Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf09.hostedemail.com (Postfix) with ESMTP id ED03B14004E for ; Thu, 30 Nov 2023 12:00:46 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.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=1701345647; a=rsa-sha256; cv=none; b=v9Ru1nlFI2ko8S1x0o5Fma/UJbjdzIHUO17viIPsq8KBeMQprgjq6saJC15HHHngbzo9tt VJRia3sUyT+5y9Q1f+e6ocWDrYcEDMOQHVqhAE8j9JoJzbL76f3IaJuL6a+/Dbgzu6dpgU Qhrkx5tez5fpfzW97ZHqhPlLFQxvKUk= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=arm.com; spf=pass (imf09.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=1701345647; 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=MiXP7uvJ6Ydx5MtT/z1/x3tGCqGLK8jIAlDLUVwPHGQ=; b=xAyS9fYVz0BCAIAjOrYRNt3YdPyXXPlupIJZDae4XK8CbsS0o2a/KT9voxLaXCdOZvCBl0 vV3jDDiAJeh37uIvUwLhC/TrGXfoBsVkISWmMdAZYEGnLGO30xSX1gDAJS64Z825E2Bjhd R+Md2PQpg+s3YlcFygQ3H05HBUAusaQ= 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 9B5551042; Thu, 30 Nov 2023 04:01:32 -0800 (PST) Received: from [10.1.34.169] (XHFQ2J9959.cambridge.arm.com [10.1.34.169]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 008BB3F5A1; Thu, 30 Nov 2023 04:00:42 -0800 (PST) Message-ID: <1d2f8e43-447e-4af4-96ac-1eefea7d6747@arm.com> Date: Thu, 30 Nov 2023 12:00:41 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 14/14] arm64/mm: Add ptep_get_and_clear_full() to optimize process teardown Content-Language: en-GB To: Barry Song <21cnbao@gmail.com> Cc: akpm@linux-foundation.org, andreyknvl@gmail.com, anshuman.khandual@arm.com, ardb@kernel.org, catalin.marinas@arm.com, david@redhat.com, dvyukov@google.com, glider@google.com, james.morse@arm.com, jhubbard@nvidia.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, mark.rutland@arm.com, maz@kernel.org, oliver.upton@linux.dev, ryabinin.a.a@gmail.com, suzuki.poulose@arm.com, vincenzo.frascino@arm.com, wangkefeng.wang@huawei.com, will@kernel.org, willy@infradead.org, yuzenghui@huawei.com, yuzhao@google.com, ziy@nvidia.com References: <20231115163018.1303287-15-ryan.roberts@arm.com> <20231128081742.39204-1-v-songbaohua@oppo.com> <207de995-6d48-41ea-8373-2f9caad9b9c3@arm.com> <34da1e06-74da-4e45-b0b5-9c93d64eb64e@arm.com> From: Ryan Roberts In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: ED03B14004E X-Stat-Signature: gcp1mgmfd77c4us74r9g5pajb6ca5311 X-Rspam-User: X-HE-Tag: 1701345646-976508 X-HE-Meta: U2FsdGVkX19OVy8T2V9mowMxIsGj/Jb9YsASfjhw9CDMfCaat8e2jwWyQZowFu+h4PP5pNSYkZlwwLfO213Vd/j7wyF3+gVr6v5a8bLytCrlXDJaMumBpT4qYfIFgRfn6/q9Bw5pBpd39UbMfBnBtuo7HlWvqFSN/+dJD/FyNXwYN6nWvcAguXyeXyU17tTdVM03vZV21y3Hzi4xfmQi5f9tnkUBh9cGSnsTFd2MI2eyJ/eapLe9ad8tlysJqss6RWivn69pGDGgCCtRDUKgU/XIvOkFbd88d1vl4AByXfPNOzThHHmQJpAD+zGvLgkARMjCS4nwXsI7VdFqfYCBsovUq5GiOk4pr7FsN+BSNsQIfodq8utLKEkpx8tPUFXj3iM6+4aEUuCkwWTyM3BsXlQxAeHn0tAJB2FKCr+sL9h/EBDOkoic8cz3FD84T+daO5XioYEdQiqp06Kt+s2/Qz4BMiLKN5RTcc/3pjOmRWniEXKZyWeVXFD97N57ZQnKgwySAQP5QqUzbiwhaVqLf3YePbgqn5pfIvJimN8Wl+iinGkNbROwRwanIW5oy0sNOWZF5ncNz+CFSgCuqzjIwA+Co/+rIpO9fJLlEeEbEmu36D1JdZfaO/C7lPXbRNp/EixMDbzDjGKSm9NdZiofd1aB748H9qwQyY6T2CGTWrsu2kloOZKfX5cbpSwZmxuWRVyfnCkYzZHMaBspSHZDgMPnWNyYWlRX9Eyh/D9iIe0sQGKHCXRfjrJg+fzwLuSkP6B76Jw4XjHPBaInnAZdPl3NyK/Oaerh3KZMGD1rYF2Bz+1oHnBDp4AClgZ/CliBCgPxso7FQPeC5McJ5Nb7hsDvIWlKawjMhRPmD1BJfnsPdbNSNt/OwWxKW6eUJThdNHnZvIO/kMOpNNopnFfgvuuZF5X2dB+klU37ToAxg2xL764lEL25jDXVP4krBwpFbq1+eqPyuRm6W4jH5ET FTwciJwE APzNoN7VBUTkoayM3Bn7Ld6gW5D2LxAjaUHhyWO2MtbmL+98yhmmEyiDAguL9OgxrZhur8zrVEyGuM07igyceGNSKoKuIqNhSRI88Tp/qSWxbazCJJ4bWuPc2bBWnKzNQ6BpgDmvN3aycnn33AmbNHCBOqF2F6HJDWfV+Md6asP7wNhsSjCK9jpwju205HMa7+Qi5NH4usn7Wf7vcSaADwIJYgh4Dv0pQQZmCQ8rSOfocyoVDu5C+ttt76GeROwcvxgNqFXscD/4ut7PgWN6Aqxbz8D9xu23Fbsu2 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: >> Just because you found a pte that maps a page from a large folio, that doesn't >> mean that all pages from the folio are mapped, and it doesn't mean they are >> mapped contiguously. We have to deal with partial munmap(), partial mremap() >> etc. We could split in these cases (and in future it might be sensible to try), >> but that can fail (due to GUP). So we still have to handle the corner case. >> >> But I can imagine doing a batched version of ptep_get_and_clear(), like I did >> for ptep_set_wrprotects(). And I think this would be an improvement. >> >> The reason I haven't done that so far, is because ptep_get_and_clear() returns >> the pte value when it was cleared and that's hard to do if batching due to the >> storage requirement. But perhaps you could just return the logical OR of the >> dirty and young bits across all ptes in the batch. The caller should be able to >> reconstitute the rest if it needs it? >> >> What do you think? > > I really don't know why we care about the return value of ptep_get_and_clear() > as zap_pte_range() doesn't ask for any ret value at all. so why not totally give > up this kind of complex logical OR of dirty and young as they are useless in > this case? That's not the case in v6.7-rc1: static unsigned long zap_pte_range(struct mmu_gather *tlb, struct vm_area_struct *vma, pmd_t *pmd, unsigned long addr, unsigned long end, struct zap_details *details) { ... do { pte_t ptent = ptep_get(pte); ... if (pte_present(ptent)) { ... ptent = ptep_get_and_clear_full(mm, addr, pte, tlb->fullmm); arch_check_zapped_pte(vma, ptent); tlb_remove_tlb_entry(tlb, pte, addr); zap_install_uffd_wp_if_needed(vma, addr, pte, details, ptent); if (unlikely(!page)) { ksm_might_unmap_zero_page(mm, ptent); continue; } delay_rmap = 0; if (!PageAnon(page)) { if (pte_dirty(ptent)) { set_page_dirty(page); if (tlb_delay_rmap(tlb)) { delay_rmap = 1; force_flush = 1; } } if (pte_young(ptent) && likely(vma_has_recency(vma))) mark_page_accessed(page); } ... } ... } while (pte++, addr += PAGE_SIZE, addr != end); ... } Most importantly, file-backed mappings need the access/dirty bits to propagate that information back to the folio, so it will be written back to disk. x86 is also looking at the dirty bit in arch_check_zapped_pte(), and ksm is using it in ksm_might_unmap_zero_page(). Probably for your use case of anon memory on arm64 on a phone, you don't need the return value. But my solution is also setting cotnpte for file-backed memory, and there are performance wins to be had there, especially for executable mappings where contpte reduces iTLB pressure. (I have other work which ensures these file-backed mappings are in correctly-sized large folios). So I don't think we can just do a clear without the get part. But I think I have a solution in the shape of clear_ptes(), as described in the other thread, which gives the characteristics you suggest. > > Is it possible for us to introduce a new api like? > > bool clear_folio_ptes(folio, ptep) > { > if(ptes are contiguous mapped) { > clear all ptes all together // this also clears all CONTPTE > return true; > } > return false; > } > > in zap_pte_range(): > > if (large_folio(folio) && clear_folio_ptes(folio, ptep)) { > addr += nr - 1 > pte += nr -1 > } else > old path. > > >> >>> >>> zap_pte_range is the most frequent behaviour from userspace libc heap >>> as i explained >>> before. libc can call madvise(DONTNEED) the most often. It is crucial >>> to performance. >>> >>> and this way can also help drop your full version by moving to full >>> flushing the whole >>> large folios? and we don't need to depend on fullmm any more? >>> >>>> >>>> I don't think there is any correctness issue here. But there is a problem with >>>> fragility, as raised by Alistair. I have some ideas on potentially how to solve >>>> that. I'm going to try to work on it this afternoon and will post if I get some >>>> confidence that it is a real solution. >>>> >>>> Thanks, >>>> Ryan >>>> >>>>> >>>>> static inline pte_t __cont_pte_huge_ptep_get_and_clear_flush(struct mm_struct *mm, >>>>> unsigned long addr, >>>>> pte_t *ptep, >>>>> bool flush) >>>>> { >>>>> pte_t orig_pte = ptep_get(ptep); >>>>> >>>>> CHP_BUG_ON(!pte_cont(orig_pte)); >>>>> CHP_BUG_ON(!IS_ALIGNED(addr, HPAGE_CONT_PTE_SIZE)); >>>>> CHP_BUG_ON(!IS_ALIGNED(pte_pfn(orig_pte), HPAGE_CONT_PTE_NR)); >>>>> >>>>> return get_clear_flush(mm, addr, ptep, PAGE_SIZE, CONT_PTES, flush); >>>>> } >>>>> >>>>> [1] https://github.com/OnePlusOSS/android_kernel_oneplus_sm8550/blob/oneplus/sm8550_u_14.0.0_oneplus11/mm/memory.c#L1539 >>>>> >>>>>> + */ >>>>>> + >>>>>> + return __ptep_get_and_clear(mm, addr, ptep); >>>>>> +} >>>>>> +EXPORT_SYMBOL(contpte_ptep_get_and_clear_full); >>>>>> + >>>>> >>> > Thanks > Barry