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 2870EC3DA61 for ; Mon, 22 Jul 2024 01:51:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 789966B0082; Sun, 21 Jul 2024 21:51:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 73A9A6B0083; Sun, 21 Jul 2024 21:51:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6287B6B0085; Sun, 21 Jul 2024 21:51:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 45C9F6B0082 for ; Sun, 21 Jul 2024 21:51:19 -0400 (EDT) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id E3C8D1413D5 for ; Mon, 22 Jul 2024 01:51:18 +0000 (UTC) X-FDA: 82365711036.15.CDA3076 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.20]) by imf30.hostedemail.com (Postfix) with ESMTP id 4F3E380007 for ; Mon, 22 Jul 2024 01:51:16 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=LFX3B4FB; spf=pass (imf30.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.20 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1721613054; a=rsa-sha256; cv=none; b=Av/refWCzwqThoiiu2zJGdrGSCFHuuqn/Ggohx10OSN7smPItDtgmNb3Xq+2pb0fl9fKUV GYG7vY8IMBJIjiXeWT1nTP7wCZ9umVmvPszsKJGBvMOcMqYDaU6z74+AIgIYMK8XFerGsk 0CZnjEpGK/L6o9h9I9PQ4HKyNCFYVQU= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=LFX3B4FB; spf=pass (imf30.hostedemail.com: domain of ying.huang@intel.com designates 198.175.65.20 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1721613054; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=HEsS2Dw1iijoGogBbC3comlmYcdUCvD7duwgLtCrI/c=; b=qromIVwitiAZs/SilDZmC4Ichu44WNMN+WrTvnRbuGTjEnAqBevuMZK6nkfu8vfMCR5Wju 2n8AskGXkzdvQxQvQJuq1E+ZMHOYOle9F+aVnoOc86UwlGs22WJetMYXoVziQLNPoIlARH Lf1amSTS71GXPTP4WHJusJnAZppp0IU= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1721613076; x=1753149076; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=cAt92c2u4pJpgHPwgKHECJkZLu5EDukZ29rPgn9SoRA=; b=LFX3B4FBGMrdz8H3ZMXrOkM83gKl+Njmb3szBUABuREqZZr9hskmCT2v bEwd5/EwraxnwYIwbJ5TegYxKosRtH/zH0LMFP8cALu8x3xEv0BLwz7WL koni/yu6G9KlFwhc57bwNw0c/dbrMPfoPS+KqsWrfuRRbo90Z5h3j3M2g lga9S5IlT7LZMbhpxEouASB4E2+nm51Dg+nrYrIuzrxXL37QLN19+btCb 9Ex0nOtUL+onhUNUN22SQjzJJtJ2o1dbEe/mWEzHuIRyRX1hlIik+Rryb +qD6o36/2nqMWH3qj9obizh3Svlla6WH4SOmF53HI78Wm3kBCjD4KL31E g==; X-CSE-ConnectionGUID: pZ+iK2GVTt+AQfl/9hSQKw== X-CSE-MsgGUID: Ogr4XTUxSb2A/rfH5x0IUA== X-IronPort-AV: E=McAfee;i="6700,10204,11140"; a="18975338" X-IronPort-AV: E=Sophos;i="6.09,227,1716274800"; d="scan'208";a="18975338" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by orvoesa112.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jul 2024 18:51:15 -0700 X-CSE-ConnectionGUID: 86LHyG1RSpu5YW+EhjNrUg== X-CSE-MsgGUID: +w7KXqlTTmyePRTA4d0fMg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,227,1716274800"; d="scan'208";a="51439474" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmviesa007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Jul 2024 18:51:13 -0700 From: "Huang, Ying" To: Zi Yan Cc: David Hildenbrand , , Andrew Morton , Baolin Wang , Subject: Re: [RFC PATCH 3/3] mm/migrate: move common code to numa_migrate_check (was numa_migrate_prep) In-Reply-To: <735B3DEE-5C4A-43BD-B003-17F4B1F0DC98@nvidia.com> (Zi Yan's message of "Fri, 19 Jul 2024 16:19:04 -0400") References: <20240712024455.163543-1-zi.yan@sent.com> <20240712024455.163543-4-zi.yan@sent.com> <87zfqfw0yw.fsf@yhuang6-desk2.ccr.corp.intel.com> <735B3DEE-5C4A-43BD-B003-17F4B1F0DC98@nvidia.com> Date: Mon, 22 Jul 2024 09:47:40 +0800 Message-ID: <87sew2jiyr.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Stat-Signature: rnksz9q8kb19ejrcdp9eborybeyfpmnq X-Rspamd-Queue-Id: 4F3E380007 X-Rspam-User: X-Rspamd-Server: rspam10 X-HE-Tag: 1721613076-36375 X-HE-Meta: U2FsdGVkX19193WmzAtT34cITGBLvwZ01p/0pOVFfSdUtOqKFZJg5clyMxDz92bvVcv1S4H1rD+OJ4K7QfTB2aVG5fLCH67v9ckF5fXUrW749yoFe+CM+Emfq8U1DllYjzVWZSRmfw9tpWOTxLUCNM3AWjkuhzslHaPiV16q3FIQdjKyPGjwOYKtLD0IKk3+XaYLSHhtsKzWuHxQJwwwmE5G71BEuINFdPDGReFf2g3YQQUAkWxOZRrs8b0GCkDTL6x+iO73UxqJgq0Do2bFc3Qyu5LCpCKWBARoaBGfSvuv6SpCf2XkVNDRxM7JXZAumdB2erxbzOPMVcTXHP3o1NfDN3or5Frxfu4M9F93ogPxhKpWg95z7bcI1Obwj6Rh5XUj4iqC6pAFqO04YXj/LeReM0TTs1f2LMzb/V755qYiF7RSRlwjCxRzP5u5M1wCCNwaZn1cwarccOoaOir1rdeTvm1q5bBwyZQGZDSMP72ZZsmsJMLidFK9QgjD6UeOPKowZRCM0qI4zFXgBGzjMEe8trFLp9q0huugSm0lHh69lh3ZrwHxHXer2ccXh7m5MN8eU3AiiTHB8OqHw+V4yXtX1qEDr7ZWjXhyDvbVfv4Ov/FSZon4lFOpCxvr4GtBpF7d2e81nAvFAgt6FbM4aIxEZ5Of716+ehsL+BjiLwMp5m0aVHvyHQ2YdhfO9FAV7sgUP8MpQARASRnVgiZCInyTM/mxepSQS3gj7MF1olkgUGjx9yug/ksnfyUkKUkzVa9pwOLdFHquOEUBI34tbxwG1nGZ2oyaR03qhNQ8vxrdd21ApbiMBmDPBXK2GtGmFQ6KQBs8WKEMtuxfVvwvFjzLFL6eCTDKrtkFQ33SlfuPd/WZ+4MkM7ssE+2DxTc07+4sVQl0zYtyJ4pcE6HbeZyMP+tOHoWctuZeTvqPL55hCR/sTw/9h8Hwr7H0FFbelZy/D04/GVspVdTvTGg /1Nkj02P cidi1khpmocu1FN+X8qwfabaHclljnv/n55JEmWfwsmzGEWdrxYb2lBBihJ79vHN4kpno5Ktw5odxawmq80ZBkxTlDEUdjZBnqU9BkewC8Xl+B8vGaDJ9bwyRpO9ikHFzGkKyItX/tdt/+nR82ZUz/SrJmkzAcNIcqdLz4y42pclpdx4Aghpg5lXrP6/KPHfT4qWdfPbh2H4U7u8VAaB9BQgnBw2F3gGiV2GVfbLBItParfH27tiwgsvpXLLsCVbT7DIkjBJNrQKOJfcvsQXgkZ8kTrtWr3FyuecY/1nGKH/CHdk= 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: Zi Yan writes: > On 18 Jul 2024, at 4:36, Huang, Ying wrote: > >> Zi Yan writes: >> >>> From: Zi Yan >>> >>> do_numa_page() and do_huge_pmd_numa_page() share a lot of common code. To >>> reduce redundancy, move common code to numa_migrate_prep() and rename >>> the function to numa_migrate_check() to reflect its functionality. >>> >>> There is some code difference between do_numa_page() and >>> do_huge_pmd_numa_page() before the code move: >>> >>> 1. do_huge_pmd_numa_page() did not check shared folios to set TNF_SHARED. >>> 2. do_huge_pmd_numa_page() did not check and skip zone device folios. >>> >>> Signed-off-by: Zi Yan >>> --- >>> mm/huge_memory.c | 28 ++++++----------- >>> mm/internal.h | 5 +-- >>> mm/memory.c | 81 +++++++++++++++++++++++------------------------- >>> 3 files changed, 52 insertions(+), 62 deletions(-) >>> >>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >>> index 8c11d6da4b36..66d67d13e0dc 100644 >>> --- a/mm/huge_memory.c >>> +++ b/mm/huge_memory.c >>> @@ -1670,10 +1670,10 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >>> pmd_t pmd; >>> struct folio *folio; >>> unsigned long haddr = vmf->address & HPAGE_PMD_MASK; >>> - int nid = NUMA_NO_NODE; >>> - int target_nid, last_cpupid = (-1 & LAST_CPUPID_MASK); >>> + int target_nid = NUMA_NO_NODE; >>> + int last_cpupid = (-1 & LAST_CPUPID_MASK); >>> bool writable = false; >>> - int flags = 0; >>> + int flags = 0, nr_pages; >>> >>> vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd); >>> if (unlikely(!pmd_same(oldpmd, *vmf->pmd))) { >>> @@ -1693,21 +1693,13 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >>> writable = true; >>> >>> folio = vm_normal_folio_pmd(vma, haddr, pmd); >>> - if (!folio) >>> + if (!folio || folio_is_zone_device(folio)) >> >> This change appears unrelated. Can we put it in a separate patch? >> >> IIUC, this isn't necessary even in do_numa_page()? Because in >> change_pte_range(), folio_is_zone_device() has been checked already. >> But It doesn't hurt too. >> >>> goto out_map; >>> >>> - /* See similar comment in do_numa_page for explanation */ >>> - if (!writable) >>> - flags |= TNF_NO_GROUP; >>> + nr_pages = folio_nr_pages(folio); >>> >>> - nid = folio_nid(folio); >>> - /* >>> - * For memory tiering mode, cpupid of slow memory page is used >>> - * to record page access time. So use default value. >>> - */ >>> - if (folio_has_cpupid(folio)) >>> - last_cpupid = folio_last_cpupid(folio); >>> - target_nid = numa_migrate_prep(folio, vmf, haddr, nid, &flags); >>> + target_nid = numa_migrate_check(folio, vmf, haddr, writable, >>> + &flags, &last_cpupid); >>> if (target_nid == NUMA_NO_NODE) >>> goto out_map; >>> if (migrate_misplaced_folio_prepare(folio, vma, target_nid)) { >>> @@ -1720,8 +1712,8 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >>> >>> if (!migrate_misplaced_folio(folio, vma, target_nid)) { >>> flags |= TNF_MIGRATED; >>> - nid = target_nid; >>> } else { >>> + target_nid = NUMA_NO_NODE; >>> flags |= TNF_MIGRATE_FAIL; >>> vmf->ptl = pmd_lock(vma->vm_mm, vmf->pmd); >>> if (unlikely(!pmd_same(oldpmd, *vmf->pmd))) { >>> @@ -1732,8 +1724,8 @@ vm_fault_t do_huge_pmd_numa_page(struct vm_fault *vmf) >>> } >>> >>> out: >>> - if (nid != NUMA_NO_NODE) >>> - task_numa_fault(last_cpupid, nid, HPAGE_PMD_NR, flags); >>> + if (target_nid != NUMA_NO_NODE) >>> + task_numa_fault(last_cpupid, target_nid, nr_pages, flags); >> >> This appears a behavior change. IIUC, there are 2 possible issues. >> >> 1) if migrate_misplaced_folio() fails, folio_nid() should be used as >> nid. "target_nid" as variable name here is confusing, because >> folio_nid() is needed in fact. >> >> 2) if !pmd_same(), task_numa_fault() should be skipped. The original >> code is buggy. >> >> Similar issues for do_numa_page(). >> >> If my understanding were correct, we should implement a separate patch >> to fix 2) above. And that may need to be backported. > > Hmm, the original code seems OK after I checked the implementation. > There are two possible !pte_same()/!pmd_same() locations: > 1) at the beginning of do_numa_page() and do_huge_pmd_numa_page() and the faulted > PTE/PMD changed before the folio can be checked, task_numa_fault() should not be > called. Yes. > 2) when migrate_misplaced_folio() failed and the PTE/PMD changed, but the folio > has been determined and checked. task_numa_fault() should be called even if > !pte_same()/!pmd_same(), IIUC, if !pte_same()/!pmd_same(), the fault has been processed on another CPU. For example, do_numa_page()/do_huge_pmd_numa_page() has been called on another CPU and task_numa_fault() has been called for the PTE/PMD already. > Let me know if I get this wrong. Thanks. > -- Best Regards, Huang, Ying