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 827D6C369BA for ; Wed, 16 Apr 2025 07:33:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4063B2800AB; Wed, 16 Apr 2025 03:33:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B65D28008F; Wed, 16 Apr 2025 03:33:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2A5D92800AB; Wed, 16 Apr 2025 03:33:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 0DD3828008F for ; Wed, 16 Apr 2025 03:33:18 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 93CAFABE28 for ; Wed, 16 Apr 2025 07:33:18 +0000 (UTC) X-FDA: 83339091276.20.5B8F20A Received: from out30-101.freemail.mail.aliyun.com (out30-101.freemail.mail.aliyun.com [115.124.30.101]) by imf10.hostedemail.com (Postfix) with ESMTP id 9A8FDC0003 for ; Wed, 16 Apr 2025 07:33:15 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=cIWiJfXu; dmarc=pass (policy=none) header.from=linux.alibaba.com; spf=pass (imf10.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.101 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1744788797; 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:dkim-signature; bh=ppJPBuDksNCAtKuglT45gt364FEpjlLU/EygbbDtpIs=; b=f/NA3wRydu0FH7n98HNH/4dKVxPQSsXG6mrVXR0BxFyrTRncgGORCuc85QdvOzSwbh3HA4 yzY8IwPuTnLp9uBsdFcIXz3PPgLdGG7/mD40nZhReOyes1aBVc1j4U6UoFrwwcFFGjaBYB mOe572xJg0Mi82TVX5G1A8/p4+dD004= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1744788797; a=rsa-sha256; cv=none; b=0BTumLDuvWpVd0nTwnUtTJWtKLCNwIwONx+8IsYBSaWQsIEJJHDCfTrstDAc03sjDXtTbx i/FR15yYDYm5msh4uKVcYneoRxyYhOLpj9HLxzb0NhO+kShOLBpYWR1Zb8W1Cvi/8R4sST +zZQaYi8sJDeUzJ7xH06QGCGpXFfoIk= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=linux.alibaba.com header.s=default header.b=cIWiJfXu; dmarc=pass (policy=none) header.from=linux.alibaba.com; spf=pass (imf10.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.101 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1744788792; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=ppJPBuDksNCAtKuglT45gt364FEpjlLU/EygbbDtpIs=; b=cIWiJfXuQMxYpAI+RjdLQiQ6ReW9II4j3pek1Zd+ziJvrnbfeaTvp/osXbeivNFnKuZC3yb5SrgM6oqeCd2L45ggM5bPWCbf4zFY8P5q4suWan7LWjV10nV7wgsWv6/S07MbuSVECNS6Hi8mCWDpr5OI9Ni7jrl1U2Ex46sa8ds= Received: from 30.74.144.124(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0WX85919_1744788789 cluster:ay36) by smtp.aliyun-inc.com; Wed, 16 Apr 2025 15:33:10 +0800 Message-ID: Date: Wed, 16 Apr 2025 15:33:08 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] mempolicy: Optimize queue_folios_pte_range by PTE batching To: Dev Jain , David Hildenbrand , akpm@linux-foundation.org Cc: ryan.roberts@arm.com, willy@infradead.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, hughd@google.com, vishal.moola@gmail.com, yang@os.amperecomputing.com, ziy@nvidia.com References: <20250411081301.8533-1-dev.jain@arm.com> <09c77ab5-65fc-4bca-a7e5-2b11bba9330d@redhat.com> <9ed4c113-37eb-4e3d-98a1-f46f786aaea9@arm.com> From: Baolin Wang In-Reply-To: <9ed4c113-37eb-4e3d-98a1-f46f786aaea9@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 9A8FDC0003 X-Rspam-User: X-Stat-Signature: wrdxprkmbk66tuood9dk4pzf61ofnzxa X-HE-Tag: 1744788795-640783 X-HE-Meta: U2FsdGVkX1/ncWr6SBT1eoOzk4b2ya3VUwGcyWvQta6IH7mkn3imboCC6bQxnNIblBfVqSNktQWrs/rDkHdNmCVLb7AJFnuSiEp8ZAzZ8DTfSwPGIBabTtfG+DDj9KMJ/cAkzSq4dMMpFJBacVq6Ck2TrgOsnzm+d0A9m/YgLgQ7xWSdnoc4pIbkpzm8XlR3vx1nSAXLEMHgDgV8cPPdxIeEoiRF1G1r6uVHj7j+uJ1CZB9sowWO3b4H+2lTgD2AWAvvWqZQgQvxxHWdQ9JFzFkxMqn9FApn036BJrjZ5loe+Q8UHzcIkXG+PkAoJRrnRpX01CZ2j6AELawG34YXc+k0OMqRQeAueGoa608qSQSBGSV1SUeY+y8jXovy0eiLC49RC1kfOKNAte41ApsZjVVSfI2fsx1A2V/3reoMojyl4zBu23Vwwe6dffx05XLMvzNTG4QWp9PMMg1IxnPytGmOr7Xg5QR0KQHSG+4k5i6XmOZc3jzwfj85WbfaS4AJBVtIjXlyWEpT5okpWa9AHL/jTtp1rQ7cRP56jdgIaFoBtF461sYnZ5++8j8nwafS9FBqwLGKr+S22MrYBTnf6qsZVI3ZwSOzea+B6WPsiPHbYiYcJn01n0S+IKfQYYCL1wakq4JCtOvrGW5w7aqGqecgL+C2Wz64GWTizT4jEpBOOzJbxNV45pZBy7gdVh12oqKIsuf2oJh08ieY8mECfZ/HJdi4YulbtthgvUKan+FbyuCPKS0ZqJUWxaf5IRFvVc37MiKK4Ct/xHpPb+R8+pmaSpFIPk48hpw1mnSSjzZ93AUX4EDAjATVgB8n/g+/cLTNpSCOKtIMjcLG8kdiyzwaX2LaRsSv9wDnvWzF2LOBHUSDUmnfX2otvTeeW8aSZpU7tFaNKqhST8Gqg1GpbEIk0pzUE+aa+RHLuEkmxGIg9Bi4K38bPiBZodo4WFvfMyRaKpJ0DKcA7O5mBAB yXT//Td9 cUYc7JFpm5lLRtioU+PRyl4NP4m24PP6nT5eMjWw3brpflA+sSbSevQrxMoNOf4eREmneEUt3x101aVKk4aYl6RrVz0tgCdNh1kOGD0ptYTEQcZ2H47kTMCQrlU8DclCrhch7nokTjQ4rm1lMwdLCtK/jLSH/8jFpCVhGKK15t6G7S+ZFX2l7xjWNEEPHPBZJnfjaKONTy0lW9iFipNKGetG9N1mKTMGoVgayurMeduA+BaSgjoHcr5CxViok8O7VZ1ejXnBKib5ASXJ5FvFc3fI58eHw6oVSuXDbgFvGgVDz6RwBnTTxVi48MvjonBUIixQZ5kWnCcWbMSe4fYGN43pP0aIo3nEgp43X8+u8K1mqgZZP0sDy3MRqxHMHAujEfNFVcWKbFSxsA3jSLCRXhfKkEbW8pZOVBnnqhC83M3N4dDLwnDyeQdx8zP3k+4PJKOyyhaCde/j8r3+ZT5i1rC4RUlI1/koXXBRxH0az5JfM1Zw= 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 2025/4/15 19:47, Dev Jain wrote: > > > On 15/04/25 3:47 pm, David Hildenbrand wrote: >> On 11.04.25 10:13, Dev Jain wrote: >>> After the check for queue_folio_required(), the code only cares about >>> the >>> folio in the for loop, i.e the PTEs are redundant. Therefore, >>> optimize this >>> loop by skipping over a PTE batch mapping the same folio. >>> >>> Signed-off-by: Dev Jain >>> --- >>> Unfortunately I have only build tested this since my test environment is >>> broken. >>> >>>   mm/mempolicy.c | 12 +++++++++++- >>>   1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c >>> index b28a1e6ae096..b019524da8a2 100644 >>> --- a/mm/mempolicy.c >>> +++ b/mm/mempolicy.c >>> @@ -573,6 +573,9 @@ static int queue_folios_pte_range(pmd_t *pmd, >>> unsigned long addr, >>>       pte_t *pte, *mapped_pte; >>>       pte_t ptent; >>>       spinlock_t *ptl; >>> +    int max_nr; >>> +    const fpb_t fpb_flags = FPB_IGNORE_DIRTY | FPB_IGNORE_SOFT_DIRTY; >>> +    int nr = 1; >> >> Try sticking to reverse xmas tree, please. (not completely the case >> here, but fpb_flags can easily be moved all he way to the top) > > I thought that the initializations were to be kept at the bottom. > Asking for future patches, should I put all declarations in reverse-xmas > fashion (even those which I don't intend to touch w.r.t the patch > logic), or do I do that for only my additions? > >> >> Also, why are you initializing nr to 1 here if you reinitialize it below? > > Yup no need, I thought pte += nr will blow up due to nr not being > initialized, but it won't because it gets executed just before the start > of the second iteration. > >> >>  >       ptl = pmd_trans_huge_lock(pmd, vma);>       if (ptl) { >>> @@ -586,7 +589,8 @@ static int queue_folios_pte_range(pmd_t *pmd, >>> unsigned long addr, >>>           walk->action = ACTION_AGAIN; >>>           return 0; >>>       } >>  > -    for (; addr != end; pte++, addr += PAGE_SIZE) {> +    for (; >> addr != end; pte += nr, addr += nr * PAGE_SIZE) { >>> +        nr = 1; >>>           ptent = ptep_get(pte); >>>           if (pte_none(ptent)) >>>               continue; >>> @@ -607,6 +611,11 @@ static int queue_folios_pte_range(pmd_t *pmd, >>> unsigned long addr, >>>           if (!queue_folio_required(folio, qp)) >>>               continue; >>>           if (folio_test_large(folio)) { >>> +            max_nr = (end - addr) >> PAGE_SHIFT; >>> +            if (max_nr != 1) >>> +                nr = folio_pte_batch(folio, addr, pte, ptent, >>> +                             max_nr, fpb_flags, >>> +                             NULL, NULL, NULL); >> >> We should probably do that immediately after we verified that >> vm_normal_folio() have us something reasonable. > > But shouldn't we keep the small folio case separate to avoid the > overhead of folio_pte_batch()? > >> >>>               /* >>>                * A large folio can only be isolated from LRU once, >>>                * but may be mapped by many PTEs (and Copy-On-Write may >>> @@ -633,6 +642,7 @@ static int queue_folios_pte_range(pmd_t *pmd, >>> unsigned long addr, >>>               qp->nr_failed++; >>>               if (strictly_unmovable(flags)) >>>                   break; >>> +            qp->nr_failed += nr - 1; >> >> Can't we do qp->nr_failed += nr; above? > > I did not dive deep into the significance of nr_failed, but I did that > to keep the code, before and after the change, equivalent: > > Claim: if we reach qp->nr_failed++ for a single pte, we will reach here > for all ptes belonging to the same batch. Sorry, I missed the previous discussion (I replied to your new version). I think this claim is incorrect, we will skip remaining ptes belonging to the same batch with checking 'qp->large'. if (folio_test_large(folio)) { if (folio == qp->large) continue; qp->large = folio; }