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 41651D13570 for ; Mon, 28 Oct 2024 11:42:06 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A17686B0088; Mon, 28 Oct 2024 07:42:05 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 99FB66B0089; Mon, 28 Oct 2024 07:42:05 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 818EA6B008A; Mon, 28 Oct 2024 07:42:05 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5A4166B0088 for ; Mon, 28 Oct 2024 07:42:05 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 1CCB8AD674 for ; Mon, 28 Oct 2024 11:41:20 +0000 (UTC) X-FDA: 82722821958.14.9C27B70 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf05.hostedemail.com (Postfix) with ESMTP id F1470100013 for ; Mon, 28 Oct 2024 11:41:17 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730115643; a=rsa-sha256; cv=none; b=2x5Z1uvqXR4E0B6hVNWhN08fsdCZcnN171lhDgtuTKDUE7sBkrbP0UXjS5SN2hBBsjsCUv aUtRLzPt0buIlJUoUQWuT2owtIkAn2Bm5e3hqsd+VB8oPKtcs/QboGDnM9ZVh68X5H/7uc uGf2PAe4akMonvyBppI3dDQTZK/8+ws= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=none; spf=pass (imf05.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730115643; 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=c39YML5MA/GaQ2EtoyHVUEzToqeEhc3Dynb0dlqEcdw=; b=LmoGUqlF+GtX2hW1SqW3dMU62JZCe59um5wqfDhCW8QvVBmfu70mwkPFDozHSymAqducLJ U9YbG9L7B/RITJVoRIYXDPjOA3hRd9vqdbOYutkTcxW0e9DKuFtajZmB+JL/sXzxOEPG9t BHKmGe2fN/7qq9nMFN9OZbbwcvwgPU8= Received: from mail.maildlp.com (unknown [172.19.163.252]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4XcWdb6QpRz10P78; Mon, 28 Oct 2024 19:39:47 +0800 (CST) Received: from dggpemf100008.china.huawei.com (unknown [7.185.36.138]) by mail.maildlp.com (Postfix) with ESMTPS id 6E6B01800CF; Mon, 28 Oct 2024 19:41:56 +0800 (CST) Received: from [10.174.177.243] (10.174.177.243) by dggpemf100008.china.huawei.com (7.185.36.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.11; Mon, 28 Oct 2024 19:41:55 +0800 Message-ID: <2524689c-08f5-446c-8cb9-924f9db0ee3a@huawei.com> Date: Mon, 28 Oct 2024 19:41:55 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: shmem: convert to use folio_zero_range() From: Kefeng Wang To: "Huang, Ying" CC: Barry Song <21cnbao@gmail.com>, , , , , , References: <06d99b89-17ad-447e-a8f1-8e220b5688ac@huawei.com> <20241022225603.10491-1-21cnbao@gmail.com> <31afe958-91eb-484a-90b9-91114991a9a2@huawei.com> <87iktg3n2i.fsf@yhuang6-desk2.ccr.corp.intel.com> <5ccb295c-6ec9-4d00-8236-e3ba19221f40@huawei.com> <875xpg39q5.fsf@yhuang6-desk2.ccr.corp.intel.com> <1a37d4a9-eef8-4fe0-aeb0-fa95c33b305a@huawei.com> <871q042x1h.fsf@yhuang6-desk2.ccr.corp.intel.com> <86f9f4e8-9c09-4333-ae4f-f51a71c3aca7@huawei.com> <87ttcx0x4j.fsf@yhuang6-desk2.ccr.corp.intel.com> <9318769a-0d51-4c03-a808-fc3a3f09d492@huawei.com> Content-Language: en-US In-Reply-To: <9318769a-0d51-4c03-a808-fc3a3f09d492@huawei.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpemf100008.china.huawei.com (7.185.36.138) X-Stat-Signature: t8mgz8xbwd6td7dq8uxjd4yu1fmhf6ra X-Rspamd-Queue-Id: F1470100013 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1730115677-344108 X-HE-Meta: U2FsdGVkX1/rBcQ5TjleTk9f0ZpaYzenJM6aN7D+X+iNREYg3hWYVgsR30RC89rZYX3dXZjYg6UTpLjlZPvZBdDJxX/A2LvjjW0IO1Ke8T80Dzi5W7i0cAmjKknmzPzY9UT5ey+li0sZ/m7EjAdAvUWZVPE4ZIhyEpKvKGeEDhf1rNPXWTf+1Tx5bRCiBvqGoWwrzGfezmV2dNL1bUxo//tIQW17gkgke41iakNe9hn/9wAnQrX4AdzKDUON7rNIwLjOmQ17xvNY6M+jdly7d9Soqx82oIs8zWkHk+6Ck9BnVle18b7oDVmEKgSS29oA5Sq2txzG2LsItgFWZsu57mFrG4RnhfXeZqkwCMuKtOimOsjqAU7na+O5rQsvAXGYbIV++5skSNssMtJJw6pwNDLChcpMFYrJ/EPPoxNx9lVi5GsSvovoFc1UrQOPseBr33he3BDnQDeQqmpv54BYbrInskadtecZJJEKJJaaJbiPalif5vld9+36I42uG0Wo+BVjSOZ6eBuu8dmgROsSMph47bdsl5UPJE6PNsChAvgR92x5btW5l8kZ9kJK6ihTrdzUOyYqBmJhA2STGJzFf1qjSlI/AtKVsoFebnUG3He37Vv1xCFxsrLbczYN44V3fKbCDJnQMdxMmxwJ8dL9p7z7SVrYrlLcFputKqPveV2OKSuYmyghXQyrVdvb+gWBbpGabOYd4SlnF8fuHpDZHRMYiW4D/6R2NrXPiNxs32dwbmHggFBI82HV8RbFQElAdc/sg483w2JSJDP7NiPJ8vvsJQlJIWs/7d0Y0gCyTnwCnaARwKVsgEGqsqcGirhwki19Y6OVjWFM4JagYk6yadEGUA4IMU4rRHrOOIVpQWl8lGWNEa5P0OATT3sd6PIwNAt/l195EnnO1Bzhg79f6aTpzMzVQEViFtbWIdo2g3wpXXHePmOnSNz1hyxOW509CrGIh0Vca0qLUISH+2R vjvnKrKy FdkLMBt+sxKwu25WtQnHnnRhomEk7QQ6SC9wKJJ2rqYUhIb8n8FOLPjT4o42WWvep+V4guBLWNWRPrjlfVkLzZ4BUre5lJ6H/maAt55dnHWui6GhPTFwzVUfFsCw0CQAhRL2Cd4j4zPpu8rqd4hgPJTxg+Vj4PNmYirMwtwsBzFrvrONq53OVA+DKXFIZi/M8kb07imoIS8k7ccLDgRz3PZ+peqR2/J8ympR6vWDhqHT1hglP1pU4zB4k+wXhJS4BId2QWtVj0D2NDQ0= 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/10/28 14:37, Kefeng Wang wrote: > > > On 2024/10/28 10:39, Huang, Ying wrote: >> Kefeng Wang writes: >> >>> On 2024/10/25 20:21, Huang, Ying wrote: >>>> Kefeng Wang writes: >>>> >>>>> On 2024/10/25 15:47, Huang, Ying wrote: >>>>>> Kefeng Wang writes: >>>>>> >>>>>>> On 2024/10/25 10:59, Huang, Ying wrote: >>>>>>>> Hi, Kefeng, >>>>>>>> Kefeng Wang writes: >>>>>>>> >>>>>>>>> +CC Huang Ying, >>>>>>>>> >>>>>>>>> On 2024/10/23 6:56, Barry Song wrote: >>>>>>>>>> On Wed, Oct 23, 2024 at 4:10 AM Kefeng Wang >>>>>>>>>> wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> ... >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> On 2024/10/17 23:09, Matthew Wilcox wrote: >>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Oct 17, 2024 at 10:25:04PM +0800, >>>>>>>>>>>>>>>>>>>>>>>>>> Kefeng Wang wrote: >>>>>>>>>>>>>>>>>>>>>>>>>>> Directly use folio_zero_range() to cleanup code. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> Are you sure there's no performance regression >>>>>>>>>>>>>>>>>>>>>>>>>> introduced by this? >>>>>>>>>>>>>>>>>>>>>>>>>> clear_highpage() is often optimised in ways >>>>>>>>>>>>>>>>>>>>>>>>>> that we can't optimise for >>>>>>>>>>>>>>>>>>>>>>>>>> a plain memset().  On the other hand, if the >>>>>>>>>>>>>>>>>>>>>>>>>> folio is large, maybe a >>>>>>>>>>>>>>>>>>>>>>>>>> modern CPU will be able to do better than >>>>>>>>>>>>>>>>>>>>>>>>>> clear-one-page-at-a-time. >>>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Right, I missing this, clear_page might be >>>>>>>>>>>>>>>>>>>>>>>>> better than memset, I change >>>>>>>>>>>>>>>>>>>>>>>>> this one when look at the shmem_writepage(), >>>>>>>>>>>>>>>>>>>>>>>>> which already convert to >>>>>>>>>>>>>>>>>>>>>>>>> use folio_zero_range() from clear_highpage(), >>>>>>>>>>>>>>>>>>>>>>>>> also I grep >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(), there are some other to use >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(). >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> fs/bcachefs/fs-io-buffered.c: >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(folio, 0, >>>>>>>>>>>>>>>>>>>>>>>>> folio_size(folio)); >>>>>>>>>>>>>>>>>>>>>>>>> fs/bcachefs/fs-io-buffered.c: >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(f, >>>>>>>>>>>>>>>>>>>>>>>>> 0, folio_size(f)); >>>>>>>>>>>>>>>>>>>>>>>>> fs/bcachefs/fs-io-buffered.c: >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(f, >>>>>>>>>>>>>>>>>>>>>>>>> 0, folio_size(f)); >>>>>>>>>>>>>>>>>>>>>>>>> fs/libfs.c:     folio_zero_range(folio, 0, >>>>>>>>>>>>>>>>>>>>>>>>> folio_size(folio)); >>>>>>>>>>>>>>>>>>>>>>>>> fs/ntfs3/frecord.c: >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(folio, 0, >>>>>>>>>>>>>>>>>>>>>>>>> folio_size(folio)); >>>>>>>>>>>>>>>>>>>>>>>>> mm/page_io.c:   folio_zero_range(folio, 0, >>>>>>>>>>>>>>>>>>>>>>>>> folio_size(folio)); >>>>>>>>>>>>>>>>>>>>>>>>> mm/shmem.c:             folio_zero_range(folio, >>>>>>>>>>>>>>>>>>>>>>>>> 0, folio_size(folio)); >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>>> IOW, what performance testing have you done >>>>>>>>>>>>>>>>>>>>>>>>>> with this patch? >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> No performance test before, but I write a >>>>>>>>>>>>>>>>>>>>>>>>> testcase, >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> 1) allocate N large folios >>>>>>>>>>>>>>>>>>>>>>>>> (folio_alloc(PMD_ORDER)) >>>>>>>>>>>>>>>>>>>>>>>>> 2) then calculate the diff(us) when clear all N >>>>>>>>>>>>>>>>>>>>>>>>> folios >>>>>>>>>>>>>>>>>>>>>>>>>                clear_highpage/folio_zero_range/ >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_user >>>>>>>>>>>>>>>>>>>>>>>>> 3) release N folios >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> the result(run 5 times) shown below on my machine, >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> N=1, >>>>>>>>>>>>>>>>>>>>>>>>>                    clear_highpage >>>>>>>>>>>>>>>>>>>>>>>>>  folio_zero_range    folio_zero_user >>>>>>>>>>>>>>>>>>>>>>>>>               1      69                   74 >>>>>>>>>>>>>>>>>>>>>>>>>               177 >>>>>>>>>>>>>>>>>>>>>>>>>               2      57                   62 >>>>>>>>>>>>>>>>>>>>>>>>>               168 >>>>>>>>>>>>>>>>>>>>>>>>>               3      54                   58 >>>>>>>>>>>>>>>>>>>>>>>>>               234 >>>>>>>>>>>>>>>>>>>>>>>>>               4      54                   58 >>>>>>>>>>>>>>>>>>>>>>>>>               157 >>>>>>>>>>>>>>>>>>>>>>>>>               5      56                   62 >>>>>>>>>>>>>>>>>>>>>>>>>               148 >>>>>>>>>>>>>>>>>>>>>>>>> avg       58                   62.8 >>>>>>>>>>>>>>>>>>>>>>>>>   176.8 >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> N=100 >>>>>>>>>>>>>>>>>>>>>>>>>                    clear_highpage >>>>>>>>>>>>>>>>>>>>>>>>>  folio_zero_range    folio_zero_user >>>>>>>>>>>>>>>>>>>>>>>>>               1    11015                 11309 >>>>>>>>>>>>>>>>>>>>>>>>>               32833 >>>>>>>>>>>>>>>>>>>>>>>>>               2    10385                 11110 >>>>>>>>>>>>>>>>>>>>>>>>>               49751 >>>>>>>>>>>>>>>>>>>>>>>>>               3    10369                 11056 >>>>>>>>>>>>>>>>>>>>>>>>>               33095 >>>>>>>>>>>>>>>>>>>>>>>>>               4    10332                 11017 >>>>>>>>>>>>>>>>>>>>>>>>>               33106 >>>>>>>>>>>>>>>>>>>>>>>>>               5    10483                 11000 >>>>>>>>>>>>>>>>>>>>>>>>>               49032 >>>>>>>>>>>>>>>>>>>>>>>>> avg     10516.8               11098.4 >>>>>>>>>>>>>>>>>>>>>>>>>   39563.4 >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> N=512 >>>>>>>>>>>>>>>>>>>>>>>>>                    clear_highpage >>>>>>>>>>>>>>>>>>>>>>>>>  folio_zero_range   folio_zero_user >>>>>>>>>>>>>>>>>>>>>>>>>               1    55560                 60055 >>>>>>>>>>>>>>>>>>>>>>>>>              156876 >>>>>>>>>>>>>>>>>>>>>>>>>               2    55485                 60024 >>>>>>>>>>>>>>>>>>>>>>>>>              157132 >>>>>>>>>>>>>>>>>>>>>>>>>               3    55474                 60129 >>>>>>>>>>>>>>>>>>>>>>>>>              156658 >>>>>>>>>>>>>>>>>>>>>>>>>               4    55555                 59867 >>>>>>>>>>>>>>>>>>>>>>>>>              157259 >>>>>>>>>>>>>>>>>>>>>>>>>               5    55528                 59932 >>>>>>>>>>>>>>>>>>>>>>>>>              157108 >>>>>>>>>>>>>>>>>>>>>>>>> avg     55520.4               60001.4 >>>>>>>>>>>>>>>>>>>>>>>>>  157006.6 >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_user with many cond_resched(), so >>>>>>>>>>>>>>>>>>>>>>>>> time fluctuates a lot, >>>>>>>>>>>>>>>>>>>>>>>>> clear_highpage is better folio_zero_range as >>>>>>>>>>>>>>>>>>>>>>>>> you said. >>>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>>> Maybe add a new helper to convert all >>>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(folio, 0, >>>>>>>>>>>>>>>>>>>>>>>>> folio_size(folio)) >>>>>>>>>>>>>>>>>>>>>>>>> to use clear_highpage + flush_dcache_folio? >>>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>>>>> If this also improves performance for other >>>>>>>>>>>>>>>>>>>>>>>> existing callers of >>>>>>>>>>>>>>>>>>>>>>>> folio_zero_range(), then that's a positive outcome. >>>>>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> ... >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>>> hi Kefeng, >>>>>>>>>>>>>>>>>>>>> what's your point? providing a helper like >>>>>>>>>>>>>>>>>>>>> clear_highfolio() or similar? >>>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>>> Yes, from above test, using clear_highpage/ >>>>>>>>>>>>>>>>>>>> flush_dcache_folio is better >>>>>>>>>>>>>>>>>>>> than using folio_zero_range() for folio >>>>>>>>>>>>>>>>>>>> zero(especially for large >>>>>>>>>>>>>>>>>>>> folio), so I'd like to add a new helper, maybe name >>>>>>>>>>>>>>>>>>>> it folio_zero() >>>>>>>>>>>>>>>>>>>> since it zero the whole folio. >>>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>>> we already have a helper like folio_zero_user()? >>>>>>>>>>>>>>>>>>> it is not good enough? >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> Since it is with many cond_resched(), the performance >>>>>>>>>>>>>>>>>> is worst... >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> Not exactly? It should have zero cost for a preemptible >>>>>>>>>>>>>>>>> kernel. >>>>>>>>>>>>>>>>> For a non-preemptible kernel, it helps avoid clearing >>>>>>>>>>>>>>>>> the folio >>>>>>>>>>>>>>>>> from occupying the CPU and starving other processes, >>>>>>>>>>>>>>>>> right? >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> --- a/mm/shmem.c >>>>>>>>>>>>>>>> +++ b/mm/shmem.c >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> @@ -2393,10 +2393,7 @@ static int >>>>>>>>>>>>>>>> shmem_get_folio_gfp(struct inode >>>>>>>>>>>>>>>> *inode, pgoff_t index, >>>>>>>>>>>>>>>>                  * it now, lest undo on failure cancel >>>>>>>>>>>>>>>> our earlier guarantee. >>>>>>>>>>>>>>>>                  */ >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>                 if (sgp != SGP_WRITE && ! >>>>>>>>>>>>>>>> folio_test_uptodate(folio)) { >>>>>>>>>>>>>>>> -               long i, n = folio_nr_pages(folio); >>>>>>>>>>>>>>>> - >>>>>>>>>>>>>>>> -               for (i = 0; i < n; i++) >>>>>>>>>>>>>>>> -                       clear_highpage(folio_page(folio, >>>>>>>>>>>>>>>> i)); >>>>>>>>>>>>>>>> +               folio_zero_user(folio, vmf->address); >>>>>>>>>>>>>>>>                         flush_dcache_folio(folio); >>>>>>>>>>>>>>>>                         folio_mark_uptodate(folio); >>>>>>>>>>>>>>>>                 } >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> Do we perform better or worse with the following? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Here is for SGP_FALLOC, vmf = NULL, we could use >>>>>>>>>>>>>>> folio_zero_user(folio, >>>>>>>>>>>>>>> 0), I think the performance is worse, will retest once I >>>>>>>>>>>>>>> can access >>>>>>>>>>>>>>> hardware. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Perhaps, since the current code uses clear_hugepage(). >>>>>>>>>>>>>> Does using >>>>>>>>>>>>>> index << PAGE_SHIFT as the addr_hint offer any benefit? >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> when use folio_zero_user(), the performance is vary bad >>>>>>>>>>>>> with above >>>>>>>>>>>>> fallocate test(mount huge=always), >>>>>>>>>>>>> >>>>>>>>>>>>>              folio_zero_range   clear_highpage >>>>>>>>>>>>> folio_zero_user >>>>>>>>>>>>> real    0m1.214s             0m1.111s              0m3.159s >>>>>>>>>>>>> user    0m0.000s             0m0.000s              0m0.000s >>>>>>>>>>>>> sys     0m1.210s             0m1.109s              0m3.152s >>>>>>>>>>>>> >>>>>>>>>>>>> I tried with addr_hint = 0/index << PAGE_SHIFT, no obvious >>>>>>>>>>>>> different. >>>>>>>>>>>> >>>>>>>>>>>> Interesting. Does your kernel have preemption disabled or >>>>>>>>>>>> preemption_debug enabled? >>>>>>>>>>> >>>>>>>>>>> ARM64 server, CONFIG_PREEMPT_NONE=y >>>>>>>>>> this explains why the performance is much worse. >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> If not, it makes me wonder whether folio_zero_user() in >>>>>>>>>>>> alloc_anon_folio() is actually improving performance as >>>>>>>>>>>> expected, >>>>>>>>>>>> compared to the simpler folio_zero() you plan to implement. :-) >>>>>>>>>>> >>>>>>>>>>> Yes, maybe, the folio_zero_user(was clear_huge_page) is from >>>>>>>>>>> 47ad8475c000 ("thp: clear_copy_huge_page"), so original >>>>>>>>>>> clear_huge_page >>>>>>>>>>> is used in HugeTLB, clear PUD size maybe spend many time, but >>>>>>>>>>> for PMD or >>>>>>>>>>> other size of large folio, cond_resched is not necessary >>>>>>>>>>> since we >>>>>>>>>>> already have some folio_zero_range() to clear large folio, >>>>>>>>>>> and no issue >>>>>>>>>>> was reported. >>>>>>>>>> probably worth an optimization. calling cond_resched() for >>>>>>>>>> each page >>>>>>>>>> seems too aggressive and useless. >>>>>>>>> >>>>>>>>> After some test, I think the cond_resched() is not the root cause, >>>>>>>>> no performance gained with batched cond_resched(), even I kill >>>>>>>>> cond_resched() from process_huge_page, no improvement. >>>>>>>>> >>>>>>>>> But when I unconditionally use clear_gigantic_page() in >>>>>>>>> folio_zero_user(patched), there is big improvement with above >>>>>>>>> fallocate on tmpfs(mount huge=always), also I test some other >>>>>>>>> testcase, >>>>>>>>> >>>>>>>>> >>>>>>>>> 1) case-anon-w-seq-mt: (2M PMD THP) >>>>>>>>> >>>>>>>>> base: >>>>>>>>> real    0m2.490s    0m2.254s    0m2.272s >>>>>>>>> user    1m59.980s   2m23.431s   2m18.739s >>>>>>>>> sys     1m3.675s    1m15.462s   1m15.030s >>>>>>>>> >>>>>>>>> patched: >>>>>>>>> real    0m2.234s    0m2.225s    0m2.159s >>>>>>>>> user    2m56.105s   2m57.117s   3m0.489s >>>>>>>>> sys     0m17.064s   0m17.564s   0m16.150s >>>>>>>>> >>>>>>>>> Patched kernel win on sys and bad in user, but real is almost >>>>>>>>> same, >>>>>>>>> maybe a little better than base. >>>>>>>> We can find user time difference.  That means the original cache >>>>>>>> hot >>>>>>>> behavior still applies on your system. >>>>>>>> However, it appears that the performance to clear page from end to >>>>>>>> begin >>>>>>>> is really bad on your system. >>>>>>>> So, I suggest to revise the current implementation to use >>>>>>>> sequential >>>>>>>> clearing as much as possible. >>>>>>>> >>>>>>> >>>>>>> I test case-anon-cow-seq-hugetlb for copy_user_large_folio() >>>>>>> >>>>>>> base: >>>>>>> real    0m6.259s    0m6.197s    0m6.316s >>>>>>> user    1m31.176s   1m27.195s   1m29.594s >>>>>>> sys     7m44.199s   7m51.490s   8m21.149s >>>>>>> >>>>>>> patched(use copy_user_gigantic_page for 2M hugetlb too) >>>>>>> real    0m3.182s    0m3.002s    0m2.963s >>>>>>> user    1m19.456s   1m3.107s    1m6.447s >>>>>>> sys     2m59.222s   3m10.899s   3m1.027s >>>>>>> >>>>>>> and sequential copy is better than the current implementation, >>>>>>> so I will use sequential clear and copy. >>>>>> Sorry, it appears that you misunderstanding my suggestion.  I >>>>>> suggest to >>>>>> revise process_huge_page() to use more sequential memory clearing and >>>>>> copying to improve its performance on your platform. >>>>>> -- >>>>>> Best Regards, >>>>>> Huang, Ying >>>>>> >>>>>>>>> 2) case-anon-w-seq-hugetlb:(2M PMD HugeTLB) >>>>>>>>> >>>>>>>>> base: >>>>>>>>> real    0m5.175s    0m5.117s    0m4.856s >>>>>>>>> user    5m15.943s   5m7.567s    4m29.273s >>>>>>>>> sys     2m38.503s   2m21.949s   2m21.252s >>>>>>>>> >>>>>>>>> patched: >>>>>>>>> real    0m4.966s    0m4.841s    0m4.561s >>>>>>>>> user    6m30.123s   6m9.516s    5m49.733s >>>>>>>>> sys     0m58.503s   0m47.847s   0m46.785s >>>>>>>>> >>>>>>>>> >>>>>>>>> This case is similar to the case1. >>>>>>>>> >>>>>>>>> 3) fallocate hugetlb 20G (2M PMD HugeTLB) >>>>>>>>> >>>>>>>>> base: >>>>>>>>> real    0m3.016s    0m3.019s    0m3.018s >>>>>>>>> user    0m0.000s    0m0.000s    0m0.000s >>>>>>>>> sys     0m3.009s    0m3.012s    0m3.010s >>>>>>>>> >>>>>>>>> patched: >>>>>>>>> >>>>>>>>> real    0m1.136s    0m1.136s    0m1.136s >>>>>>>>> user    0m0.000s    0m0.000s    0m0.004s >>>>>>>>> sys     0m1.133s    0m1.133s    0m1.129s >>>>>>>>> >>>>>>>>> >>>>>>>>> There is big win on patched kernel, and it is similar to above >>>>>>>>> tmpfs >>>>>>>>> test, so maybe we could revert the commit c79b57e462b5 ("mm: >>>>>>>>> hugetlb: >>>>>>>>> clear target sub-page last when clearing huge page"). >>>>> >>>>> I tried the following changes, >>>>> diff --git a/mm/memory.c b/mm/memory.c >>>>> index 66cf855dee3f..e5cc75adfa10 100644 >>>>> --- a/mm/memory.c >>>>> +++ b/mm/memory.c >>>>> @@ -6777,7 +6777,7 @@ static inline int process_huge_page( >>>>>                   base = 0; >>>>>                   l = n; >>>>>                   /* Process subpages at the end of huge page */ >>>>> -               for (i = nr_pages - 1; i >= 2 * n; i--) { >>>>> +               for (i = 2 * n; i < nr_pages; i++) { >>>>>                           cond_resched(); >>>>>                           ret = process_subpage(addr + i * >>>>> PAGE_SIZE, i, >>>>>                           arg); >>>>>                           if (ret) >>>>> >>>>> Since n = 0, so the copying is from start to end now, but not >>>>> improvement for case-anon-cow-seq-hugetlb, >>>>> >>>>> and if use copy_user_gigantic_pager, the time reduced from 6s to 3s >>>>> >>>>> diff --git a/mm/memory.c b/mm/memory.c >>>>> index fe21bd3beff5..2c6532d21d84 100644 >>>>> --- a/mm/memory.c >>>>> +++ b/mm/memory.c >>>>> @@ -6876,10 +6876,7 @@ int copy_user_large_folio(struct folio *dst, >>>>> struct folio *src, >>>>>                   .vma = vma, >>>>>           }; >>>>> >>>>> -       if (unlikely(nr_pages > MAX_ORDER_NR_PAGES)) >>>>> -               return copy_user_gigantic_page(dst, src, addr_hint, >>>>>                   vma, nr_pages); >>>>> - >>>>> -       return process_huge_page(addr_hint, nr_pages, copy_subpage, >>>>> &arg); >>>>> +       return copy_user_gigantic_page(dst, src, addr_hint, vma, >>>>> nr_pages); >>>>>    } >>>> It appears that we have code generation issue here.  Can you check >>>> it? >>>> Whether code is inlined in the same way? >>>> >>> >>> No different, and I checked the asm, both process_huge_page and >>> copy_user_gigantic_page are inlined, it is strange... >> >> It's not inlined in my configuration.  And __always_inline below changes >> it for me. >> >> If it's already inlined and the code is actually almost same, why >> there's difference?  Is it possible for you to do some profile or >> further analysis? > > Yes, will continue to debug this. My bad, I has some refactor patch before using copy_user_large_folio(), ba3fda2a7b08 mm: use copy_user_large_page // good performance a88666ae8f4d mm: call might_sleep() in folio_zero/copy_user() 3ab7d4d405e9 mm: calculate the base address in the folio_zero/copy_user() 7b240664c07d mm: convert to folio_copy_user() // I made a mistake which use dst instead of src in copy_user_gigantic_page() 1a951e310aa9 mm: use aligned address in copy_user_gigantic_page() e095ce052607 mm: use aligned address in clear_gigantic_page() so please ignore the copy test result (case-anon-cow-seq-hugetlb) In summary: 1) for copying, no obvious different between copy_user_large_folio/process_huge_page(copying from last to start or copying from start to last) 2) for clearing, clear_gigantic_page is better than process_huge_page from my machine, and after clearing page from start to last(current, it process page from last to first), the performance is same to the clear_gigantic_page. > >> >>>> Maybe we can start with >>>> modified   mm/memory.c >>>> @@ -6714,7 +6714,7 @@ EXPORT_SYMBOL(__might_fault); >>>>     * operation.  The target subpage will be processed last to keep its >>>>     * cache lines hot. >>>>     */ >>>> -static inline int process_huge_page( >>>> +static __always_inline int process_huge_page( >>>>        unsigned long addr_hint, unsigned int nr_pages, >>>>        int (*process_subpage)(unsigned long addr, int idx, void *arg), >>>>        void *arg) >> >> -- >> Best Regards, >> Huang, Ying >> > >