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 C2F2AC77B75 for ; Fri, 21 Apr 2023 07:45:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E6A10900003; Fri, 21 Apr 2023 03:45:28 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DF31C900002; Fri, 21 Apr 2023 03:45:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C93F8900003; Fri, 21 Apr 2023 03:45:28 -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 B4408900002 for ; Fri, 21 Apr 2023 03:45:28 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 7069440656 for ; Fri, 21 Apr 2023 07:45:28 +0000 (UTC) X-FDA: 80704613136.19.C869343 Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by imf27.hostedemail.com (Postfix) with ESMTP id D549E4001A for ; Fri, 21 Apr 2023 07:45:25 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=d2wqz1Da; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf27.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.24 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682063126; 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=cdWMv+tpacBQKqocxcUBY5Fsh5XS4pvEYv4gMahMvlA=; b=jzhG0zsPhaR+d/CN/DYuNbKF18owX1L2XtLamV/jQ390hXLxpDASFnb5BswbEpft21xd4H W9CcjtFwwbi5h4r54zu/4xRYwDHhGiRYzkfhoSR09BvEKMNWPtA6hhk7IJjvmFSrhUpgko 9vufaFEf6MeSjZSs5yUBIG7j5cnB9JI= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=d2wqz1Da; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf27.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.24 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682063126; a=rsa-sha256; cv=none; b=hINkNQFC/Y1udRSwrNCKO9GXlb8eoBj6lhczpK/3avupCMLlv3X2zzcLxybMLr2MDJ++wQ 5ONNRVoWeV6pCXZnCgehRLMuncleL81l3hsOOr/tAYa1eAB++F63Y56siHS0ot6RmVylnM vJyykxsAcNven4gfqzcu63aMTrgZbao= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1682063126; x=1713599126; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=OQAsHnT4uuyLaswSUOBoKvMIQescywp9WfPVkAAvc6M=; b=d2wqz1DaUp/D+OGEfFMi1/buiUSXM+uo1EpvqNZy/hSUYlgqop3+pMbx 0Fl71QLwzE97lohwvmkIOfXojapn35eTm8k17lVX7/BKM/eg0K29t3hq8 AIzM6ju8Plyyu57U0SPog18XooTouQt6VE5Gf6SmvV2VPm8bQn0RDcILM smnj6GBuefEkoBek+2AN9WGDXVgOVhPSUaFij9nQ4lVtlNTcgtIK3FKUh CMB2R3Y/oZ1yAWmDeS36rYwWQPnPqGVRgiVxkxiwwlAWyqr6TU3anNDib 3beCY62FNVBE7Y/lwta1pD/nrTWto+UOoW5kyf6ZjNBjkRR7HXJrTWrxO g==; X-IronPort-AV: E=McAfee;i="6600,9927,10686"; a="347838707" X-IronPort-AV: E=Sophos;i="5.99,214,1677571200"; d="scan'208";a="347838707" Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Apr 2023 00:45:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10686"; a="866617145" X-IronPort-AV: E=Sophos;i="5.99,214,1677571200"; d="scan'208";a="866617145" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Apr 2023 00:45:17 -0700 From: "Huang, Ying" To: Baolin Wang Cc: David Hildenbrand , , , , , , Subject: Re: [PATCH] mm/page_alloc: consider pfn holes after pfn_valid() in __pageblock_pfn_to_page() References: <62e231a8f2e50c04dcadc7a0cfaa6dea5ce1ec05.1681296022.git.baolin.wang@linux.alibaba.com> <94bfa3cc-674e-25b0-e7e2-d74c970acef7@redhat.com> <87cz3zt3u6.fsf@yhuang6-desk2.ccr.corp.intel.com> <52dfdd2e-9c99-eac4-233e-59919a24323e@linux.alibaba.com> <874jp9uapj.fsf@yhuang6-desk2.ccr.corp.intel.com> Date: Fri, 21 Apr 2023 15:44:13 +0800 In-Reply-To: (Baolin Wang's message of "Fri, 21 Apr 2023 15:13:24 +0800") Message-ID: <87r0sdsmr6.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: D549E4001A X-Stat-Signature: 6m7xd4pecukd8tn8fs3wcdkcmf8o95xi X-HE-Tag: 1682063125-266746 X-HE-Meta: U2FsdGVkX1/SZ6cK5nSbH+zp2SybzRtwsMzMZwZUDx9z4wXzMy1ZO6hl3HT3mvahL3WT5/e4v5LpyEy8XOEkCqZZ4DCwGIy55sEuTizTHJzoVAA3otv9iNQXGkp1ekwd6Tu3vluiOIxGnIZ6O12ZTws7fiZ6T9SpCHevQ1+iGtSoWh0iEo71f9EPlFiSjN53vbEjtI9/uUoBjCPqJlUOXBbxPdsgx7FXkcpGZmG/Df3gkQThE0EM9KtPZIJMMH9/0xqBlz2enzfAbDGVUiZVkd3rG5gJQ1WbYHQWj3IbpDlrJgbh/9u376vPdmBZyxjwMY2MjuYMZ2XwVl0DbxtkiEwgJRiw4ZvWNZBSkBJ03xLW47JHOFAuAZfR8bA/x/8dNhE9hevuLfmPSwKaWOY9/6V/qaLOxgu4cObxogswcFQ0d4mTJ9A6gIS5+Zd9ESSePxnsYghD70GELEKJAVAsh43lctIiZIi57tdXvspkFRykZVS43971db9aJkId0FL7MrXTDbIDz+nsss9g4mLgG1d+XqhB2zHmdycRt9BniLA+Vz37H7shMXJHkU7I2bOxncGidvIidzmjabXaBoGM35EsnbSriSkIGsTiXnF7UQdw2pym0etWRvm2wgOsd81F3U5msSOUDhvmtebQw2Id26oXQyvxtEnxFqFu4keF5HjZA51IgPDWOicsx0r6qeByjeCJ3egtyaPmHiNDOTDbqPaVOo3VWebC1JbIZd5j8gOGLuT3ZHqlEqOuRCuJyFpqBYeFZGIfb9BwmEr9xG/LmfQ0zIUgbNdyIKANNN5m3naVV8IzmYB+zEyKeIDYY/erfflr+KrMFC4mgc4F/e98Nu0GQEItIz6RipEzdoWXYdC6iOWdvf3xHKvQTaLjN08ArRzkcZpuHuBe+u2pfUhOkTOCuYZxmuQvg0ONIH6CZlsqIO7vsr364jgbyJR8L2U/VVpLTCuxXpMKCkN30bE 76pHrHBp mb7/+bjwG2aKS+/NbuH8GRMuRaci0NpsxUmmNTcUsz89pwoa7Bj0X0A/2VKsTiPP44PI+1g3JkFvjZaKGg2D9JgNQTG1b1HDey70w1WvxLMBcec9ufekUrtRVe4vvSuFqwogHZZ2fPEcunTQqq9/jGxiKvlhmJ1pmugAdcmtpN+i0bnXaRwzrCeEMJ3pcYCnbS6y7MiQzf5LrvVPhym/bQqTiUqlZTUIrrYiQ 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: Baolin Wang writes: > On 4/21/2023 12:21 PM, Huang, Ying wrote: >> Baolin Wang writes: >> >>> On 4/20/2023 3:22 PM, Huang, Ying wrote: >>>> Baolin Wang writes: >>>> >>>>> On 4/12/2023 7:25 PM, David Hildenbrand wrote: >>>>>> On 12.04.23 12:45, Baolin Wang wrote: >>>>>>> Now the __pageblock_pfn_to_page() is used by set_zone_contiguous(), >>>>>>> which checks whether the given zone contains holes, and uses pfn_valid() >>>>>>> to check if the end pfn is valid. However pfn_valid() can not make sure >>>>>>> the end pfn is not a hole if the size of a pageblock is larger than the >>>>>>> size of a sub-mem_section, since the struct page getting by pfn_to_page() >>>>>>> may represent a hole or an unusable page frame, which may cause incorrect >>>>>>> zone contiguous is set. >>>>>>> >>>>>>> Though another user of pageblock_pfn_to_page() in compaction seems work >>>>>>> well now, it is better to avoid scanning or touching these offline pfns. >>>>>>> So like commit 2d070eab2e82 ("mm: consider zone which is not fully >>>>>>> populated to have holes"), we should also use pfn_to_online_page() for >>>>>>> the end pfn to make sure it is a valid pfn with usable page frame. >>>>>>> Meanwhile the pfn_valid() for end pfn can be dropped now. >>>>>>> >>>>>>> Moreover we've already used pfn_to_online_page() for start pfn to make >>>>>>> sure it is online and valid, so the pfn_valid() for the start pfn is >>>>>>> unnecessary, drop it. >>>>>> pageblocks are supposed to fall into a single memory section, so in >>>>>> mos > cases, if the start is online, so is the end. >>>>> >>>>> Yes, the granularity of memory hotplug is a mem_section. >>>>> >>>>> However, suppose the pageblock order is MAX_ORDER-1, and the size of a >>>>> sub-section is 2M, that means a pageblock will fall into 2 sub >>>>> mem-section, and if there is a hole in the zone, that means the 2nd >>>>> sub mem-section can be invalid without setting subsection_map bitmap. >>>>> >>>>> So the start is online can make sure the end pfn of a pageblock is >>>>> online, but a valid start pfn can not make sure the end pfn is valid >>>>> in the bitmap of ms->usage->subsection_map. >>>> arch_add_memory >>>> add_pages >>>> __add_pages >>>> sparse_add_section /* set subsection_map */ >>>> arch_add_memory() is only called by add_memory_resource() and >>>> pagemap_range() (called add_pages() too). In add_memory_resource(), >>>> check_hotplug_memory_range() will enforce a strict hotplug range >>>> alignment requirement (128 MB on x86_64). pagemap_range() are used for >>>> ZONE_DEVICE only. That is, for normal memory, hotplug granularity is >>>> much larger than 2MB. >>>> IIUC, the situation you mentioned above is impossible. Or do I miss >>>> something? >>> >>> Thanks for your input. Your example is correct, but this is not the >>> case I want to describe. My case is not about the memory hotplug, >>> instead about the early memory holes when initialzing the memory. Let >>> me try to describe explicity: >>> >>> First suppose the pageblock order is MAX_ORDER-1, and see below memory >>> layout as an example: >>> >>> [ 0.000000] Zone ranges: >>> [ 0.000000] DMA [mem 0x0000000040000000-0x00000000ffffffff] >>> [ 0.000000] DMA32 empty >>> [ 0.000000] Normal [mem 0x0000000100000000-0x0000001fa7ffffff] >>> [ 0.000000] Movable zone start for each node >>> [ 0.000000] Early memory node ranges >>> [ 0.000000] node 0: [mem 0x0000000040000000-0x0000001fa3c7ffff] >>> [ 0.000000] node 0: [mem 0x0000001fa3c80000-0x0000001fa3ffffff] >>> [ 0.000000] node 0: [mem 0x0000001fa4000000-0x0000001fa402ffff] >>> [ 0.000000] node 0: [mem 0x0000001fa4030000-0x0000001fa40effff] >>> [ 0.000000] node 0: [mem 0x0000001fa40f0000-0x0000001fa73cffff] >>> [ 0.000000] node 0: [mem 0x0000001fa73d0000-0x0000001fa745ffff] >>> [ 0.000000] node 0: [mem 0x0000001fa7460000-0x0000001fa746ffff] >>> [ 0.000000] node 0: [mem 0x0000001fa7470000-0x0000001fa758ffff] >>> [ 0.000000] node 0: [mem 0x0000001fa7590000-0x0000001fa7dfffff] >>> >>> Focus on the last memory range, and there is a hole for the range [mem >>> 0x0000001fa7590000-0x0000001fa7dfffff]. That means the last pageblock >>> will contain the range from 0x1fa7c00000 to 0x1fa7ffffff, since the >>> pageblock must be 4M aligned. And in this page block, these pfns will >>> fall into 2 sub-section (the sub-section size is 2M aligned). >>> >>> So, the 1st sub-section (indicates pfn range: 0x1fa7c00000 - >>> 0x1fa7dfffff ) in this pageblock is valid by >>> free_area_init()--->subsection_map_init(), but the 2nd sub-section >>> (indicates pfn range: 0x1fa7e00000 - 0x1fa7ffffff ) in this pageblock >>> is not valid. >>> >>> The problem is, if we just check the pageblock start of the hole pfn >>> (such as 0x1fa7dfffff) to make sure the hole pfn (0x1fa7dfffff) is >>> also valid, which is NOT correct. So that is what I mean "the start is >>> online can make sure the end pfn of a pageblock is online, but a valid >>> start pfn can not make sure the end pfn is valid in the bitmap of >>> ms->usage->subsection_map." >>> >>> Hope I make it clear. Does that make sense to you? Thanks. >> Thanks for your detailed description. You are right, it's possible >> that >> the second subsection of a pageblock is a hole. >> It's good to remove unnecessary pfn_valid(start_pfn) check in your >> original patch. But it appears unnecessary to replace > > OK. I will split this into a separate patch. Thanks! >> pfn_valid(end_pfn) with pfn_to_online_page(end_pfn). Yes, it's possible >> that there's a hole in a page block. But it appears that this will not >> break anything. Per my understanding, even if we had fixed this one, > > Yes, it will not break anything now, the worst case is the compaction > will waste more time to scan unnecessary hole pfns, though I did not > have a performance report to show this issue. I think the scanning should be fast. > Another concern is that the zone->contiguous is fragile IMO, and not > sure if there are pfn walkers will meet the holes though the > zone->contiguous = 1 in future. If there's any issue in the future, we can fix it at that time. > So at least we can add some comments for __pageblock_pfn_to_page() to > describe this issue? what do you think? I'm OK to add some comments there. >> there may be other smaller memory holes in a pageblock represented as >> reserved pages Best Regards, Huang, Ying