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 78E11D2C56B for ; Tue, 22 Oct 2024 15:10:32 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DC9DA6B00A1; Tue, 22 Oct 2024 11:10:31 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D78E56B00A2; Tue, 22 Oct 2024 11:10:31 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C40CB6B00A3; Tue, 22 Oct 2024 11:10:31 -0400 (EDT) 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 A75E66B00A1 for ; Tue, 22 Oct 2024 11:10:31 -0400 (EDT) Received: from smtpin28.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id DCF1A1A020B for ; Tue, 22 Oct 2024 15:10:01 +0000 (UTC) X-FDA: 82701574494.28.54582D9 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by imf15.hostedemail.com (Postfix) with ESMTP id 4FC29A0030 for ; Tue, 22 Oct 2024 15:10:10 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.255 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=1729609627; 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=rzCnTJ477omyykSZ3bGYBZoh1wERE+0ckYHIU8I8x4Y=; b=4JbEzAeX+dM6dj85CQfx3jQ5jYfWuYg4wHBZuF5s0wJblhTMpTlapaJ9IZ9uWWMmec8XZI ZQgAwpIWqpLkxAvFyapwXhJDoNw6SzYsNfGgCYh1NvzoDOGulH5M+98QkoegrzjhnFsBOP /E8ryL3YepHK8I/UNYOVi8PiOLga2rE= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=none; spf=pass (imf15.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.255 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=1729609627; a=rsa-sha256; cv=none; b=icXk/S+o6t8mkBTnIdbt1cSg5VS/ehmxDDYoJRqFjOqZ53Z2kd9OPH+zwb8IvNLUH5FHXD h5JtuYSW0ToRsCHEo3HJeyQUlPJkxGO336vzH0b6rHhPQTI42S6/sgyPhu9fztEAaTYRKR EfSH/BS28sQXC/Vr5bpBe2E4CXQdeng= Received: from mail.maildlp.com (unknown [172.19.163.48]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4XXwY25fnYz1T8TS; Tue, 22 Oct 2024 23:08:22 +0800 (CST) Received: from dggpemf100008.china.huawei.com (unknown [7.185.36.138]) by mail.maildlp.com (Postfix) with ESMTPS id 63B79180064; Tue, 22 Oct 2024 23:10:23 +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; Tue, 22 Oct 2024 23:10:22 +0800 Message-ID: <06d99b89-17ad-447e-a8f1-8e220b5688ac@huawei.com> Date: Tue, 22 Oct 2024 23:10:22 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mm: shmem: convert to use folio_zero_range() To: Barry Song <21cnbao@gmail.com> CC: Matthew Wilcox , Andrew Morton , Hugh Dickins , David Hildenbrand , Baolin Wang , References: <20241017142504.1170208-1-wangkefeng.wang@huawei.com> <20241017142504.1170208-2-wangkefeng.wang@huawei.com> <789aba5c-e2dd-4b4c-bfac-8d534c7a9211@huawei.com> <198b9258-8d5d-4b13-9bc5-21f170b43940@huawei.com> Content-Language: en-US From: Kefeng Wang In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggpemf100008.china.huawei.com (7.185.36.138) X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 4FC29A0030 X-Stat-Signature: 6qrowgrfrhzzkckz688du7p1omqenb83 X-Rspam-User: X-HE-Tag: 1729609810-746334 X-HE-Meta: U2FsdGVkX1+EN7bRYoQ3XU6yP8visTWzZwshKs+iuud3VmotQ2PtQ57ZiqAbyr1hiiIzwDoeOswmK0cgrOIvdq/xDEaiCSLvguclgkUYxJSFtzT53y52RSMzfLf6EA4ridmzYQnn6YHxclvCXIGOsoQqhiRUwH7y3lcDDc1FxdAaNk6z5K7CwFI0NmSMwBgm09GBG0u4lh6rjafM/wMlyY+MrsU2JqkoiIYKvuU/W04HA66cB4I0TmP5Jhc2BPLxFBlzrV2pwi1ZcpE4E2hd8dYTxBZTCxrjIAd01glzva1BJWI7kkafgLuEWFPppiRmxX7Me/Ju5mtzyr7NcAChpFHryjhFv/XPw5YT/LZUNmuMLTm9fZ753CCMQmWSuoE5nj02Jc7SuBi3iB+l5alzkE61b7aSzM54tFjMov7nlaj8bIAMMAL4Do2hHXkIT6TuU9uoidAP6SutVENuvBdSw0OsVCO2jv4m/nBUkm0pdfBpTD2DNwiuf8Pf8sxL2qp/vlyYESGoJPFVY42i+08rFBkylSH6tJzO1FqkQigLxFFcSAuIXUr96m62ffpeuHLPqi9YyIhoDHuJMTjT/kpoiBemjWSl4iROHO/e4pvdH0M3jbYQRCIbJdpqaSflekKk+0Qb0CRPPmbJeIjGo26Di4AI4QPc3gAf2jTNExJ7vtJrx30qhXXcEBa6ueMJphJlvHqqUlQG6ooL7SSV+PJ4lOhN2rQ5M6rrEthr0412B/n4ys6XT3dQVMAuDPKZBvBCPjtkCzkOEgps1tK8hpDD4H+nWiYnd4qrhDVx1VJPhwKNfjWPLWfsPWdwKiI0OHIV28nn4bbSwTDQJlF13AnPI8aM89b4Sf84EUOYnqkQ1hY3yfB48KIwhaz4PTMC/d93eQYMAP1MKdIwZlNpqk9nhNe9iUOTsItzV2D4FGz0aVPE2iMM5aA7DOMCb7INWKNynRcryo75TPWpX4vwNVT 6l/5g5O2 PlIBCWoyHv0sHTPl0oBHh7/SFlV11z+SDua1pDFamVv40xYAfFD80B6Dxnu0hzUUp9UJIjwGWPakz0DE1K5dKpzjvimdobM5MKVhrVIcstuizlH8wsJ4cyA1fzbKuhcXqp05q0cf8MetoYbUdDYK6TRBfuyIvuNPOS3Fen5NufazfXRmDMh+ukrxjwLKlSz8QLfUQaHQOaaZhZtO95Susl+LtaOKyWYmlph4RfpMDnVUlX5F+37/Q+b22b1ZC2XDiKhI1Oo9k5pziuefSSVyp/ruCJQzehcLH+BKS 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/22 4:32, Barry Song wrote: > On Tue, Oct 22, 2024 at 4:33 AM Kefeng Wang wrote: >> >> >> >> On 2024/10/21 17:17, Barry Song wrote: >>> On Mon, Oct 21, 2024 at 9:14 PM Kefeng Wang wrote: >>>> >>>> >>>> >>>> On 2024/10/21 15:55, Barry Song wrote: >>>>> On Mon, Oct 21, 2024 at 8:47 PM Barry Song <21cnbao@gmail.com> wrote: >>>>>> >>>>>> On Mon, Oct 21, 2024 at 7:09 PM Kefeng Wang wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 2024/10/21 13:38, Barry Song wrote: >>>>>>>> On Mon, Oct 21, 2024 at 6:16 PM Kefeng Wang wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On 2024/10/21 12:15, Barry Song wrote: >>>>>>>>>> On Fri, Oct 18, 2024 at 8:48 PM Kefeng Wang wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 2024/10/18 15:32, Kefeng Wang wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On 2024/10/18 13:23, Barry Song wrote: >>>>>>>>>>>>> On Fri, Oct 18, 2024 at 6:20 PM 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 > > 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.