From: Zi Yan <ziy@nvidia.com>
To: Doug Berger <opendmb@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Jonathan Corbet <corbet@lwn.net>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Frank Rowand <frowand.list@gmail.com>,
Mike Kravetz <mike.kravetz@oracle.com>,
Muchun Song <songmuchun@bytedance.com>,
Mike Rapoport <rppt@kernel.org>, Christoph Hellwig <hch@lst.de>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Robin Murphy <robin.murphy@arm.com>, Borislav Petkov <bp@suse.de>,
"Paul E. McKenney" <paulmck@kernel.org>,
Neeraj Upadhyay <quic_neeraju@quicinc.com>,
Randy Dunlap <rdunlap@infradead.org>,
Damien Le Moal <damien.lemoal@opensource.wdc.com>,
Florian Fainelli <f.fainelli@gmail.com>,
David Hildenbrand <david@redhat.com>,
Oscar Salvador <osalvador@suse.de>,
Hari Bathini <hbathini@linux.ibm.com>,
Kees Cook <keescook@chromium.org>,
- <devicetree-spec@vger.kernel.org>,
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
Mel Gorman <mgorman@suse.de>,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-mm@kvack.org,
iommu@lists.linux.dev
Subject: Re: [PATCH 01/21] mm/page_isolation: protect cma from isolate_single_pageblock
Date: Tue, 13 Sep 2022 20:02:19 -0400 [thread overview]
Message-ID: <36E322BF-F052-4A8B-9FA5-4E0AA84E4AAF@nvidia.com> (raw)
In-Reply-To: <20220913195508.3511038-2-opendmb@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 7677 bytes --]
On 13 Sep 2022, at 15:54, Doug Berger wrote:
> The function set_migratetype_isolate() has special handling for
> pageblocks of MIGRATE_CMA type that protects them from being
> isolated for MIGRATE_MOVABLE requests.
>
> Since isolate_single_pageblock() doesn't receive the migratetype
> argument of start_isolate_page_range() it used the migratetype
> of the pageblock instead of the requested migratetype which
> defeats this MIGRATE_CMA check.
>
> This allows an attempt to create a gigantic page within a CMA
> region to change the migratetype of the first and last pageblocks
> from MIGRATE_CMA to MIGRATE_MOVABLE when they are restored after
> failure, which corrupts the CMA region.
>
> The calls to (un)set_migratetype_isolate() for the first and last
> pageblocks of the start_isolate_page_range() are moved back into
> that function to allow access to its migratetype argument and make
> it easier to see how all of the pageblocks in the range are
> isolated.
>
> Fixes: b2c9e2fbba32 ("mm: make alloc_contig_range work at pageblock granularity")
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
> mm/page_isolation.c | 75 +++++++++++++++++++++------------------------
> 1 file changed, 35 insertions(+), 40 deletions(-)
Thanks for the fix.
Why not just pass migratetype into isolate_single_pageblock() and use
it when set_migratetype_isolate() is used? That would have much
fewer changes. What is the reason of pulling skip isolation logic out?
Ultimately, I would like to make MIGRATE_ISOLATE a separate bit,
so that migratetype will not be overwritten during page isolation.
Then, set_migratetype_isolate() and start_isolate_page_range()
will not have migratetype to set in error recovery any more.
That is on my TODO.
>
> diff --git a/mm/page_isolation.c b/mm/page_isolation.c
> index 9d73dc38e3d7..8e16aa22cb61 100644
> --- a/mm/page_isolation.c
> +++ b/mm/page_isolation.c
> @@ -286,8 +286,6 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
> * @flags: isolation flags
> * @gfp_flags: GFP flags used for migrating pages
> * @isolate_before: isolate the pageblock before the boundary_pfn
> - * @skip_isolation: the flag to skip the pageblock isolation in second
> - * isolate_single_pageblock()
> *
> * Free and in-use pages can be as big as MAX_ORDER-1 and contain more than one
> * pageblock. When not all pageblocks within a page are isolated at the same
> @@ -302,9 +300,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
> * the in-use page then splitting the free page.
> */
> static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
> - gfp_t gfp_flags, bool isolate_before, bool skip_isolation)
> + gfp_t gfp_flags, bool isolate_before)
> {
> - unsigned char saved_mt;
> unsigned long start_pfn;
> unsigned long isolate_pageblock;
> unsigned long pfn;
> @@ -328,18 +325,6 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
> start_pfn = max(ALIGN_DOWN(isolate_pageblock, MAX_ORDER_NR_PAGES),
> zone->zone_start_pfn);
>
> - saved_mt = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
> -
> - if (skip_isolation)
> - VM_BUG_ON(!is_migrate_isolate(saved_mt));
> - else {
> - ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt, flags,
> - isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
> -
> - if (ret)
> - return ret;
> - }
> -
> /*
> * Bail out early when the to-be-isolated pageblock does not form
> * a free or in-use page across boundary_pfn:
> @@ -428,7 +413,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
> ret = set_migratetype_isolate(page, page_mt,
> flags, head_pfn, head_pfn + nr_pages);
> if (ret)
> - goto failed;
> + return ret;
> }
>
> ret = __alloc_contig_migrate_range(&cc, head_pfn,
> @@ -443,7 +428,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
> unset_migratetype_isolate(page, page_mt);
>
> if (ret)
> - goto failed;
> + return -EBUSY;
> /*
> * reset pfn to the head of the free page, so
> * that the free page handling code above can split
> @@ -459,24 +444,19 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
> while (!PageBuddy(pfn_to_page(outer_pfn))) {
> /* stop if we cannot find the free page */
> if (++order >= MAX_ORDER)
> - goto failed;
> + return -EBUSY;
> outer_pfn &= ~0UL << order;
> }
> pfn = outer_pfn;
> continue;
> } else
> #endif
> - goto failed;
> + return -EBUSY;
> }
>
> pfn++;
> }
> return 0;
> -failed:
> - /* restore the original migratetype */
> - if (!skip_isolation)
> - unset_migratetype_isolate(pfn_to_page(isolate_pageblock), saved_mt);
> - return -EBUSY;
> }
>
> /**
> @@ -534,21 +514,30 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> unsigned long isolate_start = ALIGN_DOWN(start_pfn, pageblock_nr_pages);
> unsigned long isolate_end = ALIGN(end_pfn, pageblock_nr_pages);
> int ret;
> - bool skip_isolation = false;
>
> /* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
> - ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false, skip_isolation);
> + ret = set_migratetype_isolate(pfn_to_page(isolate_start), migratetype,
> + flags, isolate_start, isolate_start + pageblock_nr_pages);
> if (ret)
> return ret;
> -
> - if (isolate_start == isolate_end - pageblock_nr_pages)
> - skip_isolation = true;
> + ret = isolate_single_pageblock(isolate_start, flags, gfp_flags, false);
> + if (ret)
> + goto unset_start_block;
>
> /* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
> - ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true, skip_isolation);
> + pfn = isolate_end - pageblock_nr_pages;
> + if (isolate_start != pfn) {
> + ret = set_migratetype_isolate(pfn_to_page(pfn), migratetype,
> + flags, pfn, pfn + pageblock_nr_pages);
> + if (ret)
> + goto unset_start_block;
> + }
> + ret = isolate_single_pageblock(isolate_end, flags, gfp_flags, true);
> if (ret) {
> - unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
> - return ret;
> + if (isolate_start != pfn)
> + goto unset_end_block;
> + else
> + goto unset_start_block;
> }
>
> /* skip isolated pageblocks at the beginning and end */
> @@ -557,15 +546,21 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
> pfn += pageblock_nr_pages) {
> page = __first_valid_page(pfn, pageblock_nr_pages);
> if (page && set_migratetype_isolate(page, migratetype, flags,
> - start_pfn, end_pfn)) {
> - undo_isolate_page_range(isolate_start, pfn, migratetype);
> - unset_migratetype_isolate(
> - pfn_to_page(isolate_end - pageblock_nr_pages),
> - migratetype);
> - return -EBUSY;
> - }
> + start_pfn, end_pfn))
> + goto unset_isolated_blocks;
> }
> return 0;
> +
> +unset_isolated_blocks:
> + ret = -EBUSY;
> + undo_isolate_page_range(isolate_start + pageblock_nr_pages, pfn,
> + migratetype);
> +unset_end_block:
> + unset_migratetype_isolate(pfn_to_page(isolate_end - pageblock_nr_pages),
> + migratetype);
> +unset_start_block:
> + unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
> + return ret;
> }
>
> /*
> --
> 2.25.1
--
Best Regards,
Yan, Zi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]
next prev parent reply other threads:[~2022-09-14 0:02 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-13 19:54 [PATCH 00/21] mm: introduce Designated Movable Blocks Doug Berger
2022-09-13 19:54 ` [PATCH 01/21] mm/page_isolation: protect cma from isolate_single_pageblock Doug Berger
2022-09-14 0:02 ` Zi Yan [this message]
2022-09-14 0:59 ` Doug Berger
2022-09-14 1:09 ` Zi Yan
2022-09-14 1:47 ` Doug Berger
2022-09-14 1:53 ` Zi Yan
2022-09-14 17:27 ` Doug Berger
2022-09-13 19:54 ` [PATCH 02/21] mm/hugetlb: correct max_huge_pages accounting on demote Doug Berger
2022-09-14 17:23 ` Mike Kravetz
2022-09-14 17:26 ` Florian Fainelli
2022-09-14 18:43 ` Mike Kravetz
2022-09-14 17:30 ` Doug Berger
2022-09-14 20:58 ` Andrew Morton
2022-09-14 21:11 ` Mike Kravetz
2022-09-13 19:54 ` [PATCH 03/21] mm/hugetlb: correct demote page offset logic Doug Berger
2022-09-13 23:34 ` Matthew Wilcox
2022-09-14 1:07 ` Doug Berger
2022-09-14 17:08 ` Mike Kravetz
2022-09-14 17:54 ` Doug Berger
2022-09-15 1:40 ` Muchun Song
2022-09-13 19:54 ` [PATCH 04/21] mm/hugetlb: refactor alloc_and_dissolve_huge_page Doug Berger
2022-09-13 19:54 ` [PATCH 05/21] mm/hugetlb: allow migrated hugepage to dissolve when freed Doug Berger
2022-09-13 19:54 ` [PATCH 06/21] mm/hugetlb: add hugepage isolation support Doug Berger
2022-09-13 19:54 ` [PATCH 07/21] lib/show_mem.c: display MovableOnly Doug Berger
2022-09-13 19:54 ` [PATCH 08/21] mm/vmstat: show start_pfn when zone spans pages Doug Berger
2022-09-13 19:54 ` [PATCH 09/21] mm/page_alloc: calculate node_spanned_pages from pfns Doug Berger
2022-09-13 19:54 ` [PATCH 10/21] mm/page_alloc.c: allow oversized movablecore Doug Berger
2022-09-13 19:54 ` [PATCH 11/21] mm/page_alloc: introduce init_reserved_pageblock() Doug Berger
2022-09-13 19:54 ` [PATCH 12/21] memblock: introduce MEMBLOCK_MOVABLE flag Doug Berger
2022-09-13 19:55 ` [PATCH 13/21] mm/dmb: Introduce Designated Movable Blocks Doug Berger
2022-09-13 19:55 ` [PATCH 14/21] mm/page_alloc: make alloc_contig_pages DMB aware Doug Berger
2022-09-13 19:55 ` [PATCH 15/21] mm/page_alloc: allow base for movablecore Doug Berger
2022-09-13 19:55 ` [PATCH 16/21] dt-bindings: reserved-memory: introduce designated-movable-block Doug Berger
2022-09-14 14:55 ` Rob Herring
2022-09-14 17:13 ` Doug Berger
2022-09-18 10:31 ` Krzysztof Kozlowski
2022-09-18 23:12 ` Doug Berger
2022-09-19 11:03 ` Krzysztof Kozlowski
2022-09-21 0:14 ` Doug Berger
2022-09-21 6:35 ` Krzysztof Kozlowski
2022-09-18 10:28 ` Krzysztof Kozlowski
2022-09-18 22:41 ` Doug Berger
2022-09-13 19:55 ` [PATCH 17/21] mm/dmb: introduce rmem designated-movable-block Doug Berger
2022-09-13 19:55 ` [PATCH 18/21] mm/cma: support CMA in Designated Movable Blocks Doug Berger
2022-09-13 19:55 ` [PATCH 19/21] dt-bindings: reserved-memory: shared-dma-pool: support DMB Doug Berger
2022-09-13 19:55 ` [PATCH 20/21] mm/cma: introduce rmem shared-dmb-pool Doug Berger
2022-09-13 19:55 ` [PATCH 21/21] mm/hugetlb: introduce hugetlb_dmb Doug Berger
2022-09-14 13:21 ` [PATCH 00/21] mm: introduce Designated Movable Blocks Rob Herring
2022-09-14 16:57 ` Doug Berger
2022-09-14 18:07 ` Rob Herring
2022-09-19 9:00 ` David Hildenbrand
2022-09-20 1:03 ` Doug Berger
2022-09-23 11:19 ` Mike Rapoport
2022-09-23 22:10 ` Doug Berger
2022-09-29 9:00 ` David Hildenbrand
2022-10-01 0:42 ` Doug Berger
2022-10-05 18:39 ` David Hildenbrand
2022-10-12 23:38 ` Doug Berger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=36E322BF-F052-4A8B-9FA5-4E0AA84E4AAF@nvidia.com \
--to=ziy@nvidia.com \
--cc=akpm@linux-foundation.org \
--cc=bp@suse.de \
--cc=corbet@lwn.net \
--cc=damien.lemoal@opensource.wdc.com \
--cc=david@redhat.com \
--cc=devicetree-spec@vger.kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=f.fainelli@gmail.com \
--cc=frowand.list@gmail.com \
--cc=hbathini@linux.ibm.com \
--cc=hch@lst.de \
--cc=iommu@lists.linux.dev \
--cc=keescook@chromium.org \
--cc=kosaki.motohiro@jp.fujitsu.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=m.szyprowski@samsung.com \
--cc=mgorman@suse.de \
--cc=mike.kravetz@oracle.com \
--cc=opendmb@gmail.com \
--cc=osalvador@suse.de \
--cc=paulmck@kernel.org \
--cc=quic_neeraju@quicinc.com \
--cc=rdunlap@infradead.org \
--cc=robh+dt@kernel.org \
--cc=robin.murphy@arm.com \
--cc=rppt@kernel.org \
--cc=songmuchun@bytedance.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).