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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8110CC4167B for ; Thu, 30 Nov 2023 12:01:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ERZaeb/ynNdIgnLBBeJP9KyB/L8vhovxjLmMGzcWPSk=; b=g96QQrVoEXhGSQ mAoba+1ny5uDidCRZZlr4V9kJSyifgYEXoSdtgH0jXSyvY6LkyWZA9vPQLPqFTa9CbNKMthxuX/xp 0CyXl69+IoZKebqkdhglZvHTf4OpxoqdS+OK9MjOhFUS51wEvt72O9HeQR0unJyo9B8gtectH+E2x 71AUQOUM98KqCyoXQPJvFDvxYBO25RjvumHqrb5CANj1I3GHv18ayaTtRVIZJgFOUJlJuHRohhbBr HmCjM5jJoVJeT2iHUpYHV7Mk41Bb7njH8zUIsEo99TCRd68AQI21AlIgXzl0afsj1/nKFYug7UBrQ mWnnLft69nnD1LSW8EtQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8fir-00AmO2-1T; Thu, 30 Nov 2023 12:00:49 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r8fio-00AmLe-2j for linux-arm-kernel@lists.infradead.org; Thu, 30 Nov 2023 12:00:48 +0000 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: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231130_040046_977049_E245366E X-CRM114-Status: GOOD ( 30.13 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org >> 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel