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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 01E2CC433F5 for ; Tue, 17 May 2022 18:51:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1352407AbiEQSvV (ORCPT ); Tue, 17 May 2022 14:51:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34836 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1352404AbiEQSvS (ORCPT ); Tue, 17 May 2022 14:51:18 -0400 Received: from cmx-torrgo001.bell.net (mta-tor-001.bell.net [209.71.212.28]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 00EBC1262C for ; Tue, 17 May 2022 11:51:16 -0700 (PDT) X-RG-CM-BuS: 0 X-RG-CM-SC: 0 X-RG-CM: Clean X-Originating-IP: [70.50.7.94] X-RG-Env-Sender: dave.anglin@bell.net X-RG-Rigid: 627B9E56017DA09A X-CM-Envelope: MS4xfL6fbg6ulagVP7yWF2JCpMXGjRP4nwb0o4Lk3Ua1iODbIJBT2gs6u7X7VLtfzlP9gtcrSsDHoBttJ2cgC+2fsnRXPNXAciy1zRuC9TvCId7wm8Ucodvn yKCnlECbT302pJ1sJ60BeR25vLMxySfnMUaIeaWE+RxuKz+QNEa8xS0LB7z8mEvWU4iasuozCWggiYJgp0Aj0wcdxML/C37/aO95ovQa8UM3dMI+r5FjujJe RObh4+r/Kr0eg9LxeOiLJhD73JuiQPu5mBRFivdAbtv09wsfsYNqruARPddlAbpUc0L457lYeb7abFH+M8dSYUpGvZYfi7px5me+rvSsWrvG1Tg8biqAOvDr Q7bvhp7gHUbWYKd5pRiKn/eCTcLNSw== X-CM-Analysis: v=2.4 cv=B8zabMhM c=1 sm=1 tr=0 ts=6283ee9b a=9k1bCY7nR7m1ZFzoCuQ56g==:117 a=9k1bCY7nR7m1ZFzoCuQ56g==:17 a=IkcTkHD0fZMA:10 a=FBHGMhGWAAAA:8 a=_mrOiYxLZSmVmRtfNHYA:9 a=QEXdDO2ut3YA:10 a=9gvnlMMaQFpL9xblJ6ne:22 Received: from [192.168.2.49] (70.50.7.94) by cmx-torrgo001.bell.net (5.8.807) (authenticated as dave.anglin@bell.net) id 627B9E56017DA09A; Tue, 17 May 2022 14:51:07 -0400 Message-ID: Date: Tue, 17 May 2022 14:51:09 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH, V3] parisc: Rewrite cache flush code for PA8800/PA8900 Content-Language: en-US To: Helge Deller , Rolf Eike Beer Cc: linux-parisc@vger.kernel.org, Deller , James Bottomley References: <5568470.DvuYhMxLoT@daneel.sf-tec.de> <325ef4bc-5dd3-bae2-e435-c00768f85377@gmx.de> <2239732.ElGaqSPkdT@eto.sf-tec.de> <91bd7b0e-0830-37cc-270c-a3e9d3069e78@gmx.de> <5ea9b91f-1d64-027a-b00d-53e0ad2302ff@gmx.de> From: John David Anglin In-Reply-To: <5ea9b91f-1d64-027a-b00d-53e0ad2302ff@gmx.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-parisc@vger.kernel.org On 2022-05-17 2:11 p.m., Helge Deller wrote: > On 5/17/22 16:26, John David Anglin wrote: >> On 2022-05-17 9:24 a.m., Helge Deller wrote: >>> On 5/17/22 15:19, Rolf Eike Beer wrote: >>>> Am Montag, 16. Mai 2022, 23:49:10 CEST schrieb Helge Deller: >>>>> On 5/16/22 23:28, Rolf Eike Beer wrote: >>>>>> Am Montag, 16. Mai 2022, 17:14:47 CEST schrieb John David Anglin: >>>>>>> Originally, I was convinced that we needed to use tmpalias flushes >>>>>>> everwhere, for both user and kernel flushes. However, when I modified >>>>>>> flush_kernel_dcache_page_addr, to use a tmpalias flush, my c8000 >>>>>>> would crash quite early when booting. >>>>>>> >>>>>>> The PDC returns alias values of 0 for the icache and dcache. This >>>>>>> indicates that either the alias boundary is greater than 16MB or >>>>>>> equivalent aliasing doesn't work. I modified the tmpalias code to >>>>>>> make it easy to try alternate boundaries. I tried boundaries up to >>>>>>> 128MB but still kernel tmpalias flushes didn't work on c8000. >>>>>>> >>>>>>> This led me to conclude that tmpalias flushes don't work on PA8800 >>>>>>> and PA8900 machines, and that we needed to flush directly using the >>>>>>> virtual address of user and kernel pages. This is likely the major >>>>>>> cause of instability on the c8000 and rp34xx machines. >>>>>>> >>>>>>> Flushing user pages requires doing a temporary context switch as we >>>>>>> have to flush pages that don't belong to the current context. Further, >>>>>>> we have to deal with pages that aren't present. If a page isn't >>>>>>> present, the flush instructions fault on every line. >>>>>>> >>>>>>> Other code has been rearranged and simplified based on testing. For >>>>>>> example, I introduced a flush_cache_dup_mm routine. flush_cache_mm >>>>>>> and flush_cache_dup_mm differ in that flush_cache_mm calls >>>>>>> purge_cache_pages and flush_cache_dup_mm calls flush_cache_pages. >>>>>>> In some implementations, pdc is more efficient than fdc. Based on >>>>>>> my testing, I don't believe there's any performance benefit on the >>>>>>> c8000. >>>>>>> >>>>>>> V2: >>>>>>> 1) Add flush_cache_page_check_pte routine. >>>>>>> 2) Use it in copy_to_user_page and copy_from_user_page. >>>>>>> 3) flush_anon_page moved to cache.c and updated. >>>>>>> 4) Changed alias boundary to 64 MB for 64-bit kernels. Revised comment >>>>>>> >>>>>>>     regarding alias boundary for PA8800/PA8900 processors. >>>>>>> >>>>>>> 5) Removed struct mm_struct * argument from flush_cache_pages. >>>>>>> 6) Fixed thinko in flush_cache_range. It increased the number of pages >>>>>>> >>>>>>>     flushed and slowed performance. >>>>>>> >>>>>>> 7) Removed sync changes from pacache.S. >>>>>>> >>>>>>> V3: >>>>>>> 1) copy_to_user_page and copy_from_user_page moved to cache.c to >>>>>>> >>>>>>>     improve inlining. >>>>>>> >>>>>>> 2) Replaced copy_user_page with copy_user_highpage. >>>>>>> 3) Fixed cache threshold calculation on 32-bit kernels. >>>>>>> 4) Don't warn on inequivalent private mappings in flush_dcache_page. >>>>>>> 5) Return early from mm_total_size if size exceeds >>>>>>> >>>>>>>     parisc_cache_flush_threshold. >>>>>>> >>>>>>> 6) Disable non-access TLB warning in handle_nadtlb_fault. Warning >>>>>>> >>>>>>>     happens occassionally handling flushes for COW faults. >>>>>>> >>>>>>> 7) Remove flush_cache_dup_mm. >>>>>>> 8) Flush entire cache in flush_cache_mm and flush_cache_range on >>>>>>> >>>>>>>     processors with aliasing caches. Only flush small cache ranges >>>>>>>     on machines with PA8800/PA8900 processors. >>>>>>> >>>>>>> 9) Tested on rp3440, c8000 and c3750. >>>>>> Given how long these changelogs are, and how fragile the whole caching is >>>>>> I >>>>>> think it is a good idea to split this patch into smaller ones, to improve >>>>>> readability and being able to bisect it. >>>>> FWIW, I've done some cleanups to this patch and committed it to my for-next >>>>> tree. In case it's split up, please use the revised version. >>>> Why did you modify get_ptep()? Until now it was just moved around in the file, >>>> and IMHO it becomes less readable because all these needless variables are >>>> batched up at the start of the function now. The only point I would see in >>>> moving them all to the front is if there would be no nesting anymore, and >>>> every condition was inverted: >>> Dave's original patch did not moved the variables to the beginning. >>> That change was me - just because checkpatch complained otherwise. >>> >>> I agree that it's less readable. >> The get_ptep code was originally based on the vmalloc_to_page code in vmalloc.c. >> vmalloc_to_page code.  This code has now changed, so I think get_ptep needs updating. >> See get_old_pud in mremap.c.  It looks good to me. > So, what's the consense of this discussion now? > > I can easily split out the pr_warn("WARNING"). > Moving the get_ptep() back to the original place seems ok, and I'll keep > the strange indenting which checkpatch want. > Is that ok? It's okay with me.  But maybe we need to add pgd_leaf and pgd_bad checks (etc).  That's what is concerning me. Dave -- John David Anglin dave.anglin@bell.net