Re: [PATCH v1 2/2] mm: enforce pageblock_order < MAX_ORDER
On 14 Feb 2022, at 12:41, David Hildenbrand wrote: > Some places in the kernel don't really expect pageblock_order >= > MAX_ORDER, and it looks like this is only possible in corner cases: > > 1) CONFIG_DEFERRED_STRUCT_PAGE_INIT we'll end up freeing pageblock_order >pages via __free_pages_core(), which cannot possibly work. > > 2) find_zone_movable_pfns_for_nodes() will roundup the ZONE_MOVABLE >start PFN to MAX_ORDER_NR_PAGES. Consequently with a bigger >pageblock_order, we could have a single pageblock partially managed by >two zones. > > 3) compaction code runs into __fragmentation_index() with order >>= MAX_ORDER, when checking WARN_ON_ONCE(order >= MAX_ORDER). [1] > > 4) mm/page_reporting.c won't be reporting any pages with default >page_reporting_order == pageblock_order, as we'll be skipping the >reporting loop inside page_reporting_process_zone(). > > 5) __rmqueue_fallback() will never be able to steal with >ALLOC_NOFRAGMENT. > > pageblock_order >= MAX_ORDER is weird either way: it's a pure > optimization for making alloc_contig_range(), as used for allcoation of > gigantic pages, a little more reliable to succeed. However, if there is > demand for somewhat reliable allocation of gigantic pages, affected setups > should be using CMA or boottime allocations instead. > > So let's make sure that pageblock_order < MAX_ORDER and simplify. > > [1] https://lkml.kernel.org/r/87r189a2ks@linux.ibm.com > > Signed-off-by: David Hildenbrand > --- > drivers/virtio/virtio_mem.c | 9 +++-- > include/linux/cma.h | 3 +-- > include/linux/pageblock-flags.h | 7 +-- > mm/Kconfig | 3 +++ > mm/page_alloc.c | 32 > 5 files changed, 20 insertions(+), 34 deletions(-) LGTM. Thanks. Reviewed-by: Zi Yan -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] cma: factor out minimum alignment requirement
On 14 Feb 2022, at 12:41, David Hildenbrand wrote: > Let's factor out determining the minimum alignment requirement for CMA > and add a helpful comment. > > No functional change intended. > > Signed-off-by: David Hildenbrand > --- > arch/powerpc/include/asm/fadump-internal.h | 5 - > arch/powerpc/kernel/fadump.c | 2 +- > drivers/of/of_reserved_mem.c | 9 +++-- > include/linux/cma.h| 9 + > kernel/dma/contiguous.c| 4 +--- > mm/cma.c | 20 +--- > 6 files changed, 19 insertions(+), 30 deletions(-) LGTM. Thanks. Reviewed-by: Zi Yan -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/6] mm: make alloc_contig_range work at pageblock granularity
On 14 Feb 2022, at 2:59, Christophe Leroy wrote:
> Le 11/02/2022 à 17:41, Zi Yan a écrit :
>> From: Zi Yan
>>
>> alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
>> pageblocks with different migratetypes. It might unnecessarily convert
>> extra pageblocks at the beginning and at the end of the range. Change
>> alloc_contig_range() to work at pageblock granularity.
>>
>> Special handling is needed for free pages and in-use pages across the
>> boundaries of the range specified alloc_contig_range(). Because these
>> partially isolated pages causes free page accounting issues. The free
>> pages will be split and freed into separate migratetype lists; the
>> in-use pages will be migrated then the freed pages will be handled.
>>
>> Signed-off-by: Zi Yan
>> ---
>> include/linux/page-isolation.h | 2 +-
>> mm/internal.h | 3 +
>> mm/memory_hotplug.c| 3 +-
>> mm/page_alloc.c| 235 +
>> mm/page_isolation.c| 33 -
>> 5 files changed, 211 insertions(+), 65 deletions(-)
>>
>> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>> index 4ef7be6def83..78ff940cc169 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -54,7 +54,7 @@ int move_freepages_block(struct zone *zone, struct page
>> *page,
>>*/
>> int
>> start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>> - unsigned migratetype, int flags);
>> + unsigned migratetype, int flags, gfp_t gfp_flags);
>>
>> /*
>>* Changes MIGRATE_ISOLATE to MIGRATE_MOVABLE.
>> diff --git a/mm/internal.h b/mm/internal.h
>> index 0d240e876831..509cbdc25992 100644
>> --- a/mm/internal.h
>> +++ b/mm/internal.h
>> @@ -319,6 +319,9 @@ isolate_freepages_range(struct compact_control *cc,
>> int
>> isolate_migratepages_range(struct compact_control *cc,
>> unsigned long low_pfn, unsigned long end_pfn);
>> +
>> +int
>> +isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags, int
>> isolate_before_boundary);
>> #endif
>> int find_suitable_fallback(struct free_area *area, unsigned int order,
>> int migratetype, bool only_stealable, bool *can_steal);
>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>> index ce68098832aa..82406d2f3e46 100644
>> --- a/mm/memory_hotplug.c
>> +++ b/mm/memory_hotplug.c
>> @@ -1863,7 +1863,8 @@ int __ref offline_pages(unsigned long start_pfn,
>> unsigned long nr_pages,
>> /* set above range as isolated */
>> ret = start_isolate_page_range(start_pfn, end_pfn,
>> MIGRATE_MOVABLE,
>> - MEMORY_OFFLINE | REPORT_FAILURE);
>> + MEMORY_OFFLINE | REPORT_FAILURE,
>> + GFP_USER | __GFP_MOVABLE |
>> __GFP_RETRY_MAYFAIL);
>> if (ret) {
>> reason = "failure to isolate range";
>> goto failed_removal_pcplists_disabled;
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 62ef78f3d771..7a4fa21aea5c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -8985,7 +8985,7 @@ static inline void alloc_contig_dump_pages(struct
>> list_head *page_list)
>> #endif
>>
>> /* [start, end) must belong to a single zone. */
>> -static int __alloc_contig_migrate_range(struct compact_control *cc,
>> +int __alloc_contig_migrate_range(struct compact_control *cc,
>> unsigned long start, unsigned long end)
>> {
>> /* This function is based on compact_zone() from compaction.c. */
>> @@ -9043,6 +9043,167 @@ static int __alloc_contig_migrate_range(struct
>> compact_control *cc,
>> return 0;
>> }
>>
>> +/**
>> + * split_free_page() -- split a free page at split_pfn_offset
>> + * @free_page: the original free page
>> + * @order: the order of the page
>> + * @split_pfn_offset: split offset within the page
>> + *
>> + * It is used when the free page crosses two pageblocks with different
>> migratetypes
>> + * at split_pfn_offset within the page. The split free page will be put into
>> + * separate migratetype lists afterwards. Otherwise, the function achieves
>> + * nothing.
>> + */
>> +static inline void split_free_page(struct page *free_page,
>> +int order, unsigned long split_pfn_offset)
>> +{
>> +struct zone *zone = page_zone(free_page);
>> +unsigned long free_page_pfn = page_to_pfn(free_page);
>> +unsigned long pfn;
>> +unsigned long flags;
>> +int free_page_order;
>> +
>> +spin_lock_irqsave(>lock, flags);
>> +del_page_from_free_list(free_page, zone, order);
>> +for (pfn = free_page_pfn;
>> + pfn < free_page_pfn + (1UL << order);) {
>> +int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
>> +
>> +
Re: [PATCH v5 3/6] mm: make alloc_contig_range work at pageblock granularity
On 14 Feb 2022, at 2:26, Christoph Hellwig wrote: >> +int >> +isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags, int >> isolate_before_boundary); > > Please avoid the completely unreadably long line. i.e. > > int isolate_single_pageblock(unsigned long boundary_pfn, gfp_t gfp_flags, > int isolate_before_boundary); > > Same in various other spots. OK. Thanks for pointing it out. checkpatch.pl did not report any warning about this. It seems that the column limit has been relaxed to 100. Anyway, I will make it shorter. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 4/7] mm: make alloc_contig_range work at pageblock granularity
On 4 Feb 2022, at 8:56, Oscar Salvador wrote:
> On Wed, Jan 19, 2022 at 02:06:20PM -0500, Zi Yan wrote:
>> From: Zi Yan
>>
>> alloc_contig_range() worked at MAX_ORDER-1 granularity to avoid merging
>> pageblocks with different migratetypes. It might unnecessarily convert
>> extra pageblocks at the beginning and at the end of the range. Change
>> alloc_contig_range() to work at pageblock granularity.
>>
>> It is done by restoring pageblock types and split >pageblock_order free
>> pages after isolating at MAX_ORDER-1 granularity and migrating pages
>> away at pageblock granularity. The reason for this process is that
>> during isolation, some pages, either free or in-use, might have >pageblock
>> sizes and isolating part of them can cause free accounting issues.
>> Restoring the migratetypes of the pageblocks not in the interesting
>> range later is much easier.
>
> Hi Zi Yan,
>
> Due to time constraints I only glanced over, so some comments below
> about stuff that caught my eye:
Thanks for looking at the patches!
>
>> +static inline void split_free_page_into_pageblocks(struct page *free_page,
>> +int order, struct zone *zone)
>> +{
>> +unsigned long pfn;
>> +
>> +spin_lock(>lock);
>> +del_page_from_free_list(free_page, zone, order);
>> +for (pfn = page_to_pfn(free_page);
>> + pfn < page_to_pfn(free_page) + (1UL << order);
>
> It migt make sense to have a end_pfn variable so that does not have to
> be constantly evaluated. Or maybe the compiler is clever enough to only
> evualuate it once.
Sure. Will add end_pfn.
>
>> + pfn += pageblock_nr_pages) {
>> +int mt = get_pfnblock_migratetype(pfn_to_page(pfn), pfn);
>> +
>> +__free_one_page(pfn_to_page(pfn), pfn, zone, pageblock_order,
>> +mt, FPI_NONE);
>> +}
>> +spin_unlock(>lock);
>
> It is possible that free_page's order is already pageblock_order, so I
> would add a one-liner upfront to catch that case and return, otherwise
> we do the delete_from_freelist-and-free_it_again dance.
Make sense. Will do.
>
>> +/* Save the migratepages of the pageblocks before start and after end */
>> +num_pageblock_to_save = (alloc_start - isolate_start) /
>> pageblock_nr_pages
>> ++ (isolate_end - alloc_end) /
>> pageblock_nr_pages;
>> +saved_mt =
>> +kmalloc_array(num_pageblock_to_save,
>> + sizeof(unsigned char), GFP_KERNEL);
>> +if (!saved_mt)
>> +return -ENOMEM;
>> +
>> +num = save_migratetypes(saved_mt, isolate_start, alloc_start);
>> +
>> +num = save_migratetypes(_mt[num], alloc_end, isolate_end);
>
> I really hope we can put all this magic within start_isolate_page_range,
> and the counterparts in undo_isolate_page_range.
>
That is my plan too.
> Also, I kinda dislike the _mt thing. I thought about some other
> approaches but nothing that wasn't too specific for this case, and I
> guess we want that function to be as generic as possible.
>
I do not like it either. This whole save and restore thing should go away
once I make MIGRATE_ISOLATE a standalone bit, so that the original
migrateypes will not be overwritten during isolation. Hopefully, I can
work on it soon to get rid of this hunk completely.
>> +/*
>> + * Split free page spanning [alloc_end, isolate_end) and put the
>> + * pageblocks in the right migratetype list
>> + */
>> +for (outer_end = alloc_end; outer_end < isolate_end;) {
>> +unsigned long begin_pfn = outer_end;
>> +
>> +order = 0;
>> +while (!PageBuddy(pfn_to_page(outer_end))) {
>> +if (++order >= MAX_ORDER) {
>> +outer_end = begin_pfn;
>> +break;
>> +}
>> +outer_end &= ~0UL << order;
>> +}
>> +
>> +if (outer_end != begin_pfn) {
>> +order = buddy_order(pfn_to_page(outer_end));
>> +
>> +/*
>> + * split the free page has start page and put the
>> pageblocks
>> + * in the right migratetype list
>> + */
>> +VM_BUG_ON(outer_end + (1UL << order) <= begin_pfn);
>
> How could this possibily happen?
Right. It is not possible. Will remove it.
>
>> +{
>> +struct page *free_page = pfn_to_page(outer_end);
>> +
>> +split_free_page_into_pageblocks(free_page,
>> order, cc.zone);
>> +}
>> +outer_end += 1UL << order;
>> +} else
>> +outer_end = begin_pfn + 1;
>> }
>
> I think there are cases could optimize for. If the page has already been
> split in pageblock by the outer_start loop, we could skip this outer_end
> logic altogether.
>
> E.g: An order-10 page is split in two
Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
On 2 Feb 2022, at 7:25, David Hildenbrand wrote: > On 02.02.22 13:18, Oscar Salvador wrote: >> On Wed, Jan 19, 2022 at 02:06:19PM -0500, Zi Yan wrote: >>> From: Zi Yan >>> >>> Enable set_migratetype_isolate() to check specified sub-range for >>> unmovable pages during isolation. Page isolation is done >>> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all >>> pages within that granularity are intended to be isolated. For example, >>> alloc_contig_range(), which uses page isolation, allows ranges without >>> alignment. This commit makes unmovable page check only look for >>> interesting pages, so that page isolation can succeed for any >>> non-overlapping ranges. >> >> Another thing that came to my mind. >> Prior to this patch, has_unmovable_pages() was checking on pageblock >> granularity, starting at pfn#0 of the pageblock. >> With this patch, you no longer check on pageblock granularity, which >> means you might isolate a pageblock, but some pages that sneaked in >> might actually be unmovable. >> >> E.g: >> >> Let's say you have a pageblock that spans (pfn#512,pfn#1024), >> and you pass alloc_contig_range() (pfn#514,pfn#1024). >> has_unmovable_pages() will start checking the pageblock at pfn#514, >> and so it will mis pfn#512 and pfn#513. Isn't that a problem, if those >> pfn turn out to be actually unmovable? > > That's the whole idea for being able to allocate parts of an unmovable > pageblock that are movable. > > If the first part is unmovable but the second part is movable, nothing > should stop us from trying to allocate the second part. > > Of course, we want to remember the original migratetype of the > pageblock, to restore that after isolation -- otherwise we'll end up > converting all such pageblocks to MIGRATE_MOVABLE. The next patch does > that IIRC. Yes. A desirable optimization is to make MIGRATE_ISOLATE a standalone bit, so isolating a pageblock will not remove its original migratetype. It is on my todo list. > > However, devil is in the detail, and I still have to review those parts > of this series. Thanks. You can wait for my next version. I am planning to make start_isolate_page_range() accept any address range and move migratetype save and restore into it, so that the caller do not need to worry about alignment. Basically, start_isolate_page_range() will need to migrate compound pages at the beginning and/or the end of the given range [start_pfn, end_pfn) if start_pfn and/or end_pfn-1 is in the middle of a compound page. If start_pfn and/or end_pfn-1 is in the middle of a free page, the free page will need to be split and put into separate migratetype lists. > > > Note that there are no current users of alloc_contig_range() that > allocate < MAX_ORDER - 1 -- except CMA, but for CMA we immediately exit > has_unmovable_pages() either way. > > -- > Thanks, > > David / dhildenb -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
On 25 Jan 2022, at 8:21, Oscar Salvador wrote: > On Tue, Jan 25, 2022 at 02:19:46PM +0100, Oscar Salvador wrote: >> I know that this has been discussed previously, and the cover-letter already >> mentions it, but I think it would be great to have some sort of information >> about >> the problem in the commit message as well, so people do not have to go and >> find >> it somewhere else. > > Sorry, the commit already points it out, but I meant to elaborate some more. You got it right about the issue. And I will add more text in the commit message and function comments to clarify the situation. Thanks for your suggestions. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 3/7] mm: page_isolation: check specified range for unmovable pages
On 24 Jan 2022, at 4:55, Oscar Salvador wrote:
> On 2022-01-19 20:06, Zi Yan wrote:
>> From: Zi Yan
>>
>> Enable set_migratetype_isolate() to check specified sub-range for
>> unmovable pages during isolation. Page isolation is done
>> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all
>> pages within that granularity are intended to be isolated. For example,
>> alloc_contig_range(), which uses page isolation, allows ranges without
>> alignment. This commit makes unmovable page check only look for
>> interesting pages, so that page isolation can succeed for any
>> non-overlapping ranges.
>
> Hi Zi Yan,
>
> I had to re-read this several times as I found this a bit misleading.
> I was mainly confused by the fact that memory_hotplug does isolation on
> PAGES_PER_SECTION granularity, and reading the above seems to indicate that
> either do it at MAX_ORDER_NR_PAGES or at pageblock_nr_pages granularity.
You are right. Sorry for the confusion. I think it should be
“Page isolation is done at least on max(MAX_ORDER_NR_PAEGS,
pageblock_nr_pages) granularity.”
memory_hotplug uses PAGES_PER_SECTION. It is greater than that.
>
> True is that start_isolate_page_range() expects the range to be pageblock
> aligned and works in pageblock_nr_pages chunks, but I do not think that is
> what you meant to say here.
Actually, start_isolate_page_range() should expect max(MAX_ORDER_NR_PAEGS,
pageblock_nr_pages) alignment instead of pageblock alignment. It seems to
be an uncovered bug in the current code, since all callers uses at least
max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) alignment.
The reason is that if start_isolate_page_range() is only pageblock aligned
and a caller wants to isolate one pageblock from a MAX_ORDER-1
(2 pageblocks on x84_64 systems) free page, this will lead to MIGRATE_ISOLATE
accounting error. To avoid it, start_isolate_page_range() needs to isolate
the max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) aligned range.
>
> Now, to the change itself, below:
>
>
>> @@ -47,8 +51,8 @@ static struct page *has_unmovable_pages(struct zone
>> *zone, struct page *page,
>> return page;
>> }
>>
>> -for (; iter < pageblock_nr_pages - offset; iter++) {
>> -page = pfn_to_page(pfn + iter);
>> +for (pfn = first_pfn; pfn < last_pfn; pfn++) {
>
> You already did pfn = first_pfn before.
Got it. Will remove the redundant code.
>
>> /**
>> * start_isolate_page_range() - make page-allocation-type of range of pages
>> to
>> * be MIGRATE_ISOLATE.
>> - * @start_pfn: The lower PFN of the range to be isolated.
>> - * @end_pfn:The upper PFN of the range to be isolated.
>> + * @start_pfn: The lower PFN of the range to be checked for
>> + * possibility of isolation.
>> + * @end_pfn:The upper PFN of the range to be checked for
>> + * possibility of isolation.
>> + * @isolate_start: The lower PFN of the range to be isolated.
>> + * @isolate_end:The upper PFN of the range to be isolated.
>
> So, what does "possibility" means here. I think this need to be clarified a
> bit better.
start_isolate_page_range() needs to check if unmovable pages exist in the
range [start_pfn, end_pfn) but mark all pageblocks within [isolate_start,
isolate_end) MIGRATE_ISOLATE (isolate_* need to be max(MAX_ORDER_NR_PAEGS,
pageblock_nr_pages) aligned). But now I realize “possibility” here is very
confusing, since both ranges decide whether the isolation can succeed.
>
> From what you pointed out in the commit message I think what you are doing is:
>
> - alloc_contig_range() gets a range to be isolated.
> - then you pass two ranges to start_isolate_page_range()
> (start_pfn, end_pfn]: which is the unaligned range you got in
> alloc_contig_range()
> (isolate_start, isolate_end]: which got aligned to, let's say, to
> MAX_ORDER_NR_PAGES
>
> Now, most likely, (start_pfn, end_pfn] only covers a sub-range of
> (isolate_start, isolate_end], and that
> sub-range is what you really want to isolate (so (start_pfn, end_pfn])?
Correct.
I agree that isolate_start and isolate_end are pretty confusing here.
They are implementation details of start_isolate_page_range() and should
not be exposed. I will remove them from the parameter list and produce
them inside start_isolate_page_range(). They are pfn_max_align_down()
and pfn_max_align_up() of start_pfn and end_pfn, respectively.
In alloc_contig_range(), the code is still needed to save and restore
migrateypes for [isolate_start, start_pfn) and (end_pfn, isolate_end],
because [start_pfn, end_pfn) is not required to be max(MAX_ORDER_NR_PAEGS,
pageblock_nr_pages) aligned. Like I said in the patch, the code will
go away once MIGRATE_ISOLATE becomes a standalone bit without overwriting
existing migratetypes during page isolation. And then isolate_start
and isolate_end here will be completely transparent to callers of
Re: [PATCH v4 1/7] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
On 24 Jan 2022, at 9:02, Mel Gorman wrote:
> On Wed, Jan 19, 2022 at 02:06:17PM -0500, Zi Yan wrote:
>> From: Zi Yan
>>
>> This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
>> It prepares for the upcoming removal of the MAX_ORDER-1 alignment
>> requirement for CMA and alloc_contig_range().
>>
>> MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
>> MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
>> Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
>>
>> [1]
>> https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
>>
>> Signed-off-by: Zi Yan
>>
>>
>>
>> @@ -2484,6 +2483,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
>> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> +[MIGRATE_HIGHATOMIC] = { MIGRATE_TYPES }, /* Never used */
>> #ifdef CONFIG_CMA
>> [MIGRATE_CMA] = { MIGRATE_TYPES }, /* Never used */
>> #endif
>
> If it's never used, why is it added?
Just to make the fallbacks list complete, since MIGRATE_CMA and
MIGRATE_ISOLATE are in the list. Instead, I can remove MIGRATE_CMA and
MIGRATE_ISOLATE. WDYT?
>
> Otherwise looks fine so
>
> Acked-by: Mel Gorman
Thanks.
--
Best Regards,
Yan, Zi
signature.asc
Description: OpenPGP digital signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 7/8] drivers: virtio_mem: use pageblock size as the minimum virtio_mem size.
On 14 Jan 2022, at 8:44, David Hildenbrand wrote: > On 05.01.22 22:47, Zi Yan wrote: >> From: Zi Yan >> >> alloc_contig_range() now only needs to be aligned to pageblock_order, >> drop virtio_mem size requirement that it needs to be the max of >> pageblock_order and MAX_ORDER. >> >> Signed-off-by: Zi Yan >> --- >> drivers/virtio/virtio_mem.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c >> index a6a78685cfbe..2664dc16d0f9 100644 >> --- a/drivers/virtio/virtio_mem.c >> +++ b/drivers/virtio/virtio_mem.c >> @@ -2481,8 +2481,7 @@ static int virtio_mem_init_hotplug(struct virtio_mem >> *vm) >> * - Is required for now for alloc_contig_range() to work reliably - >> * it doesn't properly handle smaller granularity on ZONE_NORMAL. >> */ > > Please also update this comment. No problem. Thanks for pointing this out. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 5/8] mm: page_isolation: check specified range for unmovable pages during isolation.
On 14 Jan 2022, at 8:38, David Hildenbrand wrote: > On 05.01.22 22:47, Zi Yan wrote: >> From: Zi Yan >> >> Enable set_migratetype_isolate() to check specified sub-range for >> unmovable pages during isolation. Page isolation is done >> at max(MAX_ORDER_NR_PAEGS, pageblock_nr_pages) granularity, but not all >> pages within that granularity are intended to be isolated. For example, >> alloc_contig_range(), which uses page isolation, allows ranges without >> alignment. This commit makes unmovable page check only look for >> interesting pages, so that page isolation can succeed for any >> non-overlapping ranges. > > Are you handling if we start checking in the middle of a compound page > and actually have to lookup the head to figure out if movable or not? > Yes. has_unmovable_pages() has that check already. >> >> has_unmovable_pages() is moved to mm/page_isolation.c since it is only >> used by page isolation. > > Please move that into a separate patch upfront, makes this patch much > easier to review. Sure. Will do. Thanks. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 2/8] mm: compaction: handle non-lru compound pages properly in isolate_migratepages_block().
On 13 Jan 2022, at 9:57, Zi Yan wrote:
> On 12 Jan 2022, at 6:01, David Hildenbrand wrote:
>
>> On 05.01.22 22:47, Zi Yan wrote:
>>> From: Zi Yan
>>>
>>> In isolate_migratepages_block(), a !PageLRU tail page can be encountered
>>> when the page is larger than a pageblock. Use compound head page for the
>>> checks inside and skip the entire compound page when isolation succeeds.
>>>
>>
>> This will currently never happen, due to the way we always isolate
>> MAX_ORDER -1 ranges, correct?
>
> You are right.
>
>>
>> Better note that in the patch description, because currently it reads
>> like it's an actual fix "can be encountered".
>>
>
> Will do. This is a preparation patch for the upcoming commits.
I will drop this one too. Like you mentioned in [1], there are no
non-lru migratable compound pages. This is only used by my local
test code.
[1]
https://lore.kernel.org/linux-mm/970ca2a4-416d-7e8f-37c7-510c5b050...@redhat.com/
>
>>> Signed-off-by: Zi Yan
>>> ---
>>> mm/compaction.c | 10 +++---
>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/compaction.c b/mm/compaction.c
>>> index b4e94cda3019..ad9053fbbe06 100644
>>> --- a/mm/compaction.c
>>> +++ b/mm/compaction.c
>>> @@ -979,19 +979,23 @@ isolate_migratepages_block(struct compact_control
>>> *cc, unsigned long low_pfn,
>>> * Skip any other type of page
>>> */
>>> if (!PageLRU(page)) {
>>> + struct page *head = compound_head(page);
>>> /*
>>> * __PageMovable can return false positive so we need
>>> * to verify it under page_lock.
>>> */
>>> - if (unlikely(__PageMovable(page)) &&
>>> - !PageIsolated(page)) {
>>> + if (unlikely(__PageMovable(head)) &&
>>> + !PageIsolated(head)) {
>>> if (locked) {
>>> unlock_page_lruvec_irqrestore(locked,
>>> flags);
>>> locked = NULL;
>>> }
>>>
>>> - if (!isolate_movable_page(page, isolate_mode))
>>> + if (!isolate_movable_page(head, isolate_mode)) {
>>> + low_pfn += (1 << compound_order(head))
>>> - 1 - (page - head);
>>> + page = head;
>>> goto isolate_success;
>>> + }
>>> }
>>>
>>> goto isolate_fail;
>>
>>
>> --
>> Thanks,
>>
>> David / dhildenb
>
>
> --
> Best Regards,
> Yan, Zi
--
Best Regards,
Yan, Zi
signature.asc
Description: OpenPGP digital signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 3/8] mm: migrate: allocate the right size of non hugetlb or THP compound pages.
On 12 Jan 2022, at 6:04, David Hildenbrand wrote: > On 05.01.22 22:47, Zi Yan wrote: >> From: Zi Yan >> >> alloc_migration_target() is used by alloc_contig_range() and non-LRU >> movable compound pages can be migrated. Current code does not allocate the >> right page size for such pages. Check THP precisely using >> is_transparent_huge() and add allocation support for non-LRU compound >> pages. > > IIRC, we don't have any non-lru migratable pages that are coumpound > pages. Read: not used and not supported :) OK, but nothing prevents one writing a driver that allocates compound pages and provides address_space->migratepage() and address_space->isolate_page(). Actually, to test this series, I write a kernel module that allocates an order-10 page, gives it a fake address_space with migratepage() and isolate_page(), __SetPageMovable() on it, then call alloc_contig_range() on the page range. Apparently, my kernel module is not supported by the kernel, thus, I added this patch. Do you have an alternative test to my kernel module, so that I do not even need this patch myself? > Why is this required in the context of this series? It might not be required. I will drop it. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 2/8] mm: compaction: handle non-lru compound pages properly in isolate_migratepages_block().
On 12 Jan 2022, at 6:01, David Hildenbrand wrote:
> On 05.01.22 22:47, Zi Yan wrote:
>> From: Zi Yan
>>
>> In isolate_migratepages_block(), a !PageLRU tail page can be encountered
>> when the page is larger than a pageblock. Use compound head page for the
>> checks inside and skip the entire compound page when isolation succeeds.
>>
>
> This will currently never happen, due to the way we always isolate
> MAX_ORDER -1 ranges, correct?
You are right.
>
> Better note that in the patch description, because currently it reads
> like it's an actual fix "can be encountered".
>
Will do. This is a preparation patch for the upcoming commits.
>> Signed-off-by: Zi Yan
>> ---
>> mm/compaction.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/mm/compaction.c b/mm/compaction.c
>> index b4e94cda3019..ad9053fbbe06 100644
>> --- a/mm/compaction.c
>> +++ b/mm/compaction.c
>> @@ -979,19 +979,23 @@ isolate_migratepages_block(struct compact_control *cc,
>> unsigned long low_pfn,
>> * Skip any other type of page
>> */
>> if (!PageLRU(page)) {
>> +struct page *head = compound_head(page);
>> /*
>> * __PageMovable can return false positive so we need
>> * to verify it under page_lock.
>> */
>> -if (unlikely(__PageMovable(page)) &&
>> -!PageIsolated(page)) {
>> +if (unlikely(__PageMovable(head)) &&
>> +!PageIsolated(head)) {
>> if (locked) {
>> unlock_page_lruvec_irqrestore(locked,
>> flags);
>> locked = NULL;
>> }
>>
>> -if (!isolate_movable_page(page, isolate_mode))
>> +if (!isolate_movable_page(head, isolate_mode)) {
>> +low_pfn += (1 << compound_order(head))
>> - 1 - (page - head);
>> +page = head;
>> goto isolate_success;
>> +}
>> }
>>
>> goto isolate_fail;
>
>
> --
> Thanks,
>
> David / dhildenb
--
Best Regards,
Yan, Zi
signature.asc
Description: OpenPGP digital signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 1/8] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
On 13 Jan 2022, at 7:28, David Hildenbrand wrote:
> On 13.01.22 12:36, Mike Rapoport wrote:
>> On Wed, Jan 12, 2022 at 11:54:49AM +0100, David Hildenbrand wrote:
>>> On 05.01.22 22:47, Zi Yan wrote:
From: Zi Yan
This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
It prepares for the upcoming removal of the MAX_ORDER-1 alignment
requirement for CMA and alloc_contig_range().
MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
[1]
https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
Signed-off-by: Zi Yan
---
include/linux/mmzone.h | 6 ++
mm/page_alloc.c| 28 ++--
2 files changed, 24 insertions(+), 10 deletions(-)
>>
>> ...
>>
@@ -3545,8 +3553,8 @@ int __isolate_free_page(struct page *page, unsigned
int order)
struct page *endpage = page + (1 << order) - 1;
for (; page < endpage; page += pageblock_nr_pages) {
int mt = get_pageblock_migratetype(page);
- if (!is_migrate_isolate(mt) && !is_migrate_cma(mt)
- && !is_migrate_highatomic(mt))
+ /* Only change normal pageblock */
+ if (migratetype_has_fallback(mt))
set_pageblock_migratetype(page,
MIGRATE_MOVABLE);
}
>>>
>>> That part is a nice cleanup IMHO. Although the "has fallback" part is a
>>> bit imprecise. "migratetype_is_mergable()" might be a bit clearer.
>>> ideally "migratetype_is_mergable_with_other_types()". Can we come up
>>> with a nice name for that?
>>
>> migratetype_is_mergable() kinda implies "_with_other_types", no?
>>
>> I like migratetype_is_mergable() more than _has_fallback().
>>
>> My $0.02 to bikeshedding :)
>
> :)
>
> Yeah, for me migratetype_is_mergable() would also be good enough. I
> think I was at first thinking one could mistake it with a dedicated
> migratetype. But such functions are historically called
>
> is_migrate_cma/is_migrate_cma/
>
> --
> Thanks,
>
> David / dhildenb
OK. Will use migratetype_is_mergable() instead.
--
Best Regards,
Yan, Zi
signature.asc
Description: OpenPGP digital signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v3 1/8] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
On 12 Jan 2022, at 5:54, David Hildenbrand wrote:
> On 05.01.22 22:47, Zi Yan wrote:
>> From: Zi Yan
>>
>> This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
>> It prepares for the upcoming removal of the MAX_ORDER-1 alignment
>> requirement for CMA and alloc_contig_range().
>>
>> MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
>> MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
>> Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
>>
>> [1]
>> https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
>>
>> Signed-off-by: Zi Yan
>> ---
>> include/linux/mmzone.h | 6 ++
>> mm/page_alloc.c| 28 ++--
>> 2 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index aed44e9b5d89..0aa549653e4e 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -83,6 +83,12 @@ static inline bool is_migrate_movable(int mt)
>> return is_migrate_cma(mt) || mt == MIGRATE_MOVABLE;
>> }
>>
>> +/* See fallbacks[MIGRATE_TYPES][3] in page_alloc.c */
>> +static inline bool migratetype_has_fallback(int mt)
>> +{
>> +return mt < MIGRATE_PCPTYPES;
>> +}
>> +
>> #define for_each_migratetype_order(order, type) \
>> for (order = 0; order < MAX_ORDER; order++) \
>> for (type = 0; type < MIGRATE_TYPES; type++)
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 8dd6399bafb5..5193c953dbf8 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1042,6 +1042,12 @@ buddy_merge_likely(unsigned long pfn, unsigned long
>> buddy_pfn,
>> return page_is_buddy(higher_page, higher_buddy, order + 1);
>> }
>>
>> +static inline bool has_non_fallback_pageblock(struct zone *zone)
>> +{
>> +return has_isolate_pageblock(zone) || zone_cma_pages(zone) != 0 ||
>> +zone->nr_reserved_highatomic != 0;
>> +}
>
> Due to zone_cma_pages(), the unlikely() below will be very wrong on many
> setups. Previously, isolation really was a corner case. CMA and
> highatomic are less of a corner case ...
Got it.
>
> I'm not even sure if this check is worth having around anymore at all,
> or if it would be easier and cheaper to just always check the both
> migration types unconditionally. Would certainly simplify the code.
I will remove the if check below, since, like you said, the check is
no longer a corner case with added highatomic and CMA check.
>
> Side node: we actually care about has_free_non_fallback_pageblock(), we
> can only merge with free pageblocks. But that might not necessarily be
> cheaper to test/track/check.
>
I agree that what we are actually looking for is free pageblocks of these
migratetypes. But tracking them is nontrivial.
>> +
>> /*
>> * Freeing function for a buddy system allocator.
>> *
>> @@ -1117,14 +1123,15 @@ static inline void __free_one_page(struct page *page,
>> }
>> if (order < MAX_ORDER - 1) {
>> /* If we are here, it means order is >= pageblock_order.
>> - * We want to prevent merge between freepages on isolate
>> - * pageblock and normal pageblock. Without this, pageblock
>> - * isolation could cause incorrect freepage or CMA accounting.
>> + * We want to prevent merge between freepages on pageblock
>> + * without fallbacks and normal pageblock. Without this,
>> + * pageblock isolation could cause incorrect freepage or CMA
>> + * accounting or HIGHATOMIC accounting.
>> *
>> * We don't want to hit this code for the more frequent
>> * low-order merging.
>> */
>> -if (unlikely(has_isolate_pageblock(zone))) {
>> +if (unlikely(has_non_fallback_pageblock(zone))) {
>> int buddy_mt;
>>
>> buddy_pfn = __find_buddy_pfn(pfn, order);
>> @@ -1132,8 +1139,8 @@ static inline void __free_one_page(struct page *page,
>> buddy_mt = get_pageblock_migratetype(buddy);
>>
>> if (migratetype != buddy_mt
>> -&& (is_migrate_isolate(migratetype) ||
>> -is_migrate_isolate(buddy_mt)))
>> +&&
>> (!migratetype_has_fallback(migratetype) ||
>> +
>> !migratetype_has_fallback(buddy_mt)))
>> goto done_merging;
>> }
>> max_order = order + 1;
>> @@ -2484,6 +2491,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
>> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> +
Re: [RFC PATCH v2 0/7] Use pageblock_order for cma and alloc_contig_range alignment.
On 10 Dec 2021, at 13:36, David Hildenbrand wrote: > On 10.12.21 00:04, Zi Yan wrote: >> From: Zi Yan >> >> Hi all, > > Hi, > > thanks for working on that! > >> >> This patchset tries to remove the MAX_ORDER - 1 alignment requirement for CMA >> and alloc_contig_range(). It prepares for my upcoming changes to make >> MAX_ORDER >> adjustable at boot time[1]. >> >> The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range() >> isolates pageblocks to remove free memory from buddy allocator but isolating >> only a subset of pageblocks within a page spanning across multiple pageblocks >> causes free page accounting issues. Isolated page might not be put into the >> right free list, since the code assumes the migratetype of the first >> pageblock >> as the whole free page migratetype. This is based on the discussion at [2]. >> >> To remove the requirement, this patchset: >> 1. still isolates pageblocks at MAX_ORDER - 1 granularity; >> 2. but saves the pageblock migratetypes outside the specified range of >>alloc_contig_range() and restores them after all pages within the range >>become free after __alloc_contig_migrate_range(); >> 3. splits free pages spanning multiple pageblocks at the beginning and the >> end >>of the range and puts the split pages to the right migratetype free lists >>based on the pageblock migratetypes; >> 4. returns pages not in the range as it did before this patch. >> >> Isolation needs to happen at MAX_ORDER - 1 granularity, because otherwise >> 1) extra code is needed to detect pages (free, PageHuge, THP, or >> PageCompound) >> to make sure all pageblocks belonging to a single page are isolated together >> and later pageblocks outside the range need to have their migratetypes >> restored; >> or 2) extra logic will need to be added during page free time to split a free >> page with multi-migratetype pageblocks. >> >> Two optimizations might come later: >> 1. only check unmovable pages within the range instead of MAX_ORDER - 1 >> aligned >>range during isolation to increase successful rate of >> alloc_contig_range(). > > The issue with virtio-mem is that we'll need that as soon as we change > the granularity to pageblocks, because otherwise, you can heavily > degrade unplug reliably in sane setups: > > Previous: > * Try unplug free 4M range (2 pageblocks): succeeds > > Now: > * Try unplug 2M range (first pageblock): succeeds. > * Try unplug next 2M range (second pageblock): fails because first > contains unmovable allcoations. > OK. Make sense. I will add it in the next version. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 3/7] mm: migrate: allocate the right size of non hugetlb or THP compound pages.
On 10 Dec 2021, at 2:53, Eric Ren wrote:
> Hi,
>
> On 2021/12/10 07:04, Zi Yan wrote:
>> From: Zi Yan
>>
>> alloc_migration_target() is used by alloc_contig_range() and non-LRU
>> movable compound pages can be migrated. Current code does not allocate the
>> right page size for such pages. Check THP precisely using
>> is_transparent_huge() and add allocation support for non-LRU compound
>> pages.
> Could you elaborate on why the current code doesn't get the right size? how
> this patch fixes it.
The current code only check PageHuge() and PageTransHuge(). Non-LRU compound
pages will be regarded as PageTransHuge() and an order-9 page is always
allocated
regardless of the actual page order. This patch makes the exact check for
THPs using is_transparent_huge() instead of PageTransHuge() and checks
PageCompound()
after PageHuge() and is_transparent_huge() for non-LRU compound pages.
>
> The description sounds like it's an existing bug, if so, the patch subject
> should be changed to
> "Fixes ..."?
I have not seen any related bug report.
>
>>
>> Signed-off-by: Zi Yan
>> ---
>> mm/migrate.c | 8 ++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index d487a399253b..2ce3c771b1de 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1563,7 +1563,7 @@ struct page *alloc_migration_target(struct page *page,
>> unsigned long private)
>> return alloc_huge_page_nodemask(h, nid, mtc->nmask, gfp_mask);
>> }
>> - if (PageTransHuge(page)) {
>> +if (is_transparent_hugepage(page)) {
>> /*
>> * clear __GFP_RECLAIM to make the migration callback
>> * consistent with regular THP allocations.
>> @@ -1572,13 +1572,17 @@ struct page *alloc_migration_target(struct page
>> *page, unsigned long private)
> if (PageTransHuge(page)) { // just give more code context
> ...
>> gfp_mask |= GFP_TRANSHUGE;
>> order = HPAGE_PMD_ORDER;
> order assigned here
>> }
>> +if (PageCompound(page)) {
>> +gfp_mask |= __GFP_COMP;
>> +order = compound_order(page);
> re-assinged again as THP is a compound page?
Ah, you are right. Will use else if instead. Thanks.
> Thanks,
> Eric
>> +}
>> zidx = zone_idx(page_zone(page));
>> if (is_highmem_idx(zidx) || zidx == ZONE_MOVABLE)
>> gfp_mask |= __GFP_HIGHMEM;
>> new_page = __alloc_pages(gfp_mask, order, nid, mtc->nmask);
>> - if (new_page && PageTransHuge(new_page))
>> +if (new_page && is_transparent_hugepage(page))
>> prep_transhuge_page(new_page);
>> return new_page;
--
Best Regards,
Yan, Zi
signature.asc
Description: OpenPGP digital signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH v2 1/7] mm: page_alloc: avoid merging non-fallbackable pageblocks with others.
Hi Eric,
Thanks for looking into my patch.
On 10 Dec 2021, at 2:43, Eric Ren wrote:
> Hi,
>
> On 2021/12/10 07:04, Zi Yan wrote:
>> From: Zi Yan
>>
>> This is done in addition to MIGRATE_ISOLATE pageblock merge avoidance.
>> It prepares for the upcoming removal of the MAX_ORDER-1 alignment
>> requirement for CMA and alloc_contig_range().
>>
>> MIGRARTE_HIGHATOMIC should not merge with other migratetypes like
>> MIGRATE_ISOLATE and MIGRARTE_CMA[1], so this commit prevents that too.
>> Also add MIGRARTE_HIGHATOMIC to fallbacks array for completeness.
>>
>> [1]
>> https://lore.kernel.org/linux-mm/20211130100853.gp3...@techsingularity.net/
>>
>> Signed-off-by: Zi Yan
>> ---
>> include/linux/mmzone.h | 6 ++
>> mm/page_alloc.c| 28 ++--
>> 2 files changed, 24 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 58e744b78c2c..b925431b0123 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -83,6 +83,12 @@ static inline bool is_migrate_movable(int mt)
>> return is_migrate_cma(mt) || mt == MIGRATE_MOVABLE;
>> }
>> +/* See fallbacks[MIGRATE_TYPES][3] in page_alloc.c */
>> +static inline bool migratetype_has_fallback(int mt)
>> +{
>> +return mt < MIGRATE_PCPTYPES;
>> +}
>> +
>
> I would suggest spliting the patch into 2 parts. The first part: no
> functioning change, just introduce migratetype_has_fallback()
> and replace where it applys to.
OK. I can do that.
>
>> #define for_each_migratetype_order(order, type) \
>> for (order = 0; order < MAX_ORDER; order++) \
>> for (type = 0; type < MIGRATE_TYPES; type++)
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index edfd6c81af82..107a5f186d3b 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1041,6 +1041,12 @@ buddy_merge_likely(unsigned long pfn, unsigned long
>> buddy_pfn,
>> return page_is_buddy(higher_page, higher_buddy, order + 1);
>> }
>> +static inline bool has_non_fallback_pageblock(struct zone *zone)
>> +{
>> +return has_isolate_pageblock(zone) || zone_cma_pages(zone) != 0 ||
>> +zone->nr_reserved_highatomic != 0;
>
> Make zone->nr_reserved_highatomic != 0 a helper as zone_cma_pages()?
I am not sure. We have zone_cma_pages() because when CMA is not enabled, 0 can
be
simply returned. But MIGRATE_HIGHATOMIC is always present, then an helper
function
is not that useful.
>> +}
>> +
>> /*
>>* Freeing function for a buddy system allocator.
>>*
>> @@ -1116,14 +1122,15 @@ static inline void __free_one_page(struct page *page,
>> }
>> if (order < MAX_ORDER - 1) {
>> /* If we are here, it means order is >= pageblock_order.
>> - * We want to prevent merge between freepages on isolate
>> - * pageblock and normal pageblock. Without this, pageblock
>> - * isolation could cause incorrect freepage or CMA accounting.
>> + * We want to prevent merge between freepages on pageblock
>> + * without fallbacks and normal pageblock. Without this,
>> + * pageblock isolation could cause incorrect freepage or CMA
>> + * accounting or HIGHATOMIC accounting.
>> *
>> * We don't want to hit this code for the more frequent
>> * low-order merging.
>> */
>> -if (unlikely(has_isolate_pageblock(zone))) {
>> +if (unlikely(has_non_fallback_pageblock(zone))) {
> I'm not familiar with the code details, just wondering if this change would
> has side effects on cma
> pageblock merging as it the condition stronger?
No impact on cma pageblock merging, AFAICT.
>
> Thanks,
> Eric
>> int buddy_mt;
>> buddy_pfn = __find_buddy_pfn(pfn, order);
>> @@ -1131,8 +1138,8 @@ static inline void __free_one_page(struct page *page,
>> buddy_mt = get_pageblock_migratetype(buddy);
>> if (migratetype != buddy_mt
>> -&& (is_migrate_isolate(migratetype) ||
>> -is_migrate_isolate(buddy_mt)))
>> +&&
>> (!migratetype_has_fallback(migratetype) ||
>> +
>> !migratetype_has_fallback(buddy_mt)))
>> goto done_merging;
>> }
>> max_order = order + 1;
>> @@ -2483,6 +2490,7 @@ static int fallbacks[MIGRATE_TYPES][3] = {
>> [MIGRATE_UNMOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_MOVABLE] = { MIGRATE_RECLAIMABLE, MIGRATE_UNMOVABLE,
>> MIGRATE_TYPES },
>> [MIGRATE_RECLAIMABLE] = { MIGRATE_UNMOVABLE, MIGRATE_MOVABLE,
>> MIGRATE_TYPES },
>> +[MIGRATE_HIGHATOMIC] = { MIGRATE_TYPES }, /* Never used */
>> #ifdef CONFIG_CMA
>> [MIGRATE_CMA] = { MIGRATE_TYPES }, /*
Re: [RFC PATCH v2 0/7] Use pageblock_order for cma and alloc_contig_range alignment.
On 10 Dec 2021, at 2:30, Eric Ren wrote: > Hi Zi Yan, > > On 2021/12/10 07:04, Zi Yan wrote: >> From: Zi Yan >> >> Hi all, >> >> This patchset tries to remove the MAX_ORDER - 1 alignment requirement for CMA >> and alloc_contig_range(). It prepares for my upcoming changes to make >> MAX_ORDER >> adjustable at boot time[1]. >> >> The MAX_ORDER - 1 alignment requirement comes from that alloc_contig_range() >> isolates pageblocks to remove free memory from buddy allocator but isolating >> only a subset of pageblocks within a page spanning across multiple pageblocks >> causes free page accounting issues. Isolated page might not be put into the >> right free list, since the code assumes the migratetype of the first >> pageblock >> as the whole free page migratetype. This is based on the discussion at [2]. >> >> To remove the requirement, this patchset: >> 1. still isolates pageblocks at MAX_ORDER - 1 granularity; > Then, unplug fails if either pageblock of the MAX_ORDER - 1 page has > unmovable page, right? Right. One of the optimizations mentioned is targeting this by passing the actual range instead of the MAX_ORDER-1 aligned range, so that has_unmovable_pages() will not give false positive, minimizing the isolation failure rates. > > Thanks, > Eric >> 2. but saves the pageblock migratetypes outside the specified range of >> alloc_contig_range() and restores them after all pages within the range >> become free after __alloc_contig_migrate_range(); >> 3. splits free pages spanning multiple pageblocks at the beginning and the >> end >> of the range and puts the split pages to the right migratetype free lists >> based on the pageblock migratetypes; >> 4. returns pages not in the range as it did before this patch. >> >> Isolation needs to happen at MAX_ORDER - 1 granularity, because otherwise >> 1) extra code is needed to detect pages (free, PageHuge, THP, or >> PageCompound) >> to make sure all pageblocks belonging to a single page are isolated together >> and later pageblocks outside the range need to have their migratetypes >> restored; >> or 2) extra logic will need to be added during page free time to split a free >> page with multi-migratetype pageblocks. >> >> Two optimizations might come later: >> 1. only check unmovable pages within the range instead of MAX_ORDER - 1 >> aligned >> range during isolation to increase successful rate of >> alloc_contig_range(). ^^ >> 2. make MIGRATE_ISOLATE a separate bit to avoid saving and restoring existing >> migratetypes before and after isolation respectively. >> >> Feel free to give comments and suggestions. Thanks. >> >> >> [1] >> https://lore.kernel.org/linux-mm/20210805190253.2795604-1-zi@sent.com/ >> [2] >> https://lore.kernel.org/linux-mm/d19fb078-cb9b-f60f-e310-fdeea1b94...@redhat.com/ >> >> >> Zi Yan (7): >>mm: page_alloc: avoid merging non-fallbackable pageblocks with others. >>mm: compaction: handle non-lru compound pages properly in >> isolate_migratepages_block(). >>mm: migrate: allocate the right size of non hugetlb or THP compound >> pages. >>mm: make alloc_contig_range work at pageblock granularity >>mm: cma: use pageblock_order as the single alignment >>drivers: virtio_mem: use pageblock size as the minimum virtio_mem >> size. >>arch: powerpc: adjust fadump alignment to be pageblock aligned. >> >> arch/powerpc/include/asm/fadump-internal.h | 4 +- >> drivers/virtio/virtio_mem.c| 6 +- >> include/linux/mmzone.h | 11 +- >> kernel/dma/contiguous.c| 2 +- >> mm/cma.c | 6 +- >> mm/compaction.c| 10 +- >> mm/migrate.c | 8 +- >> mm/page_alloc.c| 203 + >> 8 files changed, 196 insertions(+), 54 deletions(-) >> -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 23 Nov 2021, at 12:32, Vlastimil Babka wrote: > On 11/23/21 17:35, Zi Yan wrote: >> On 19 Nov 2021, at 10:15, Zi Yan wrote: > From what my understanding, cma required alignment of > max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was > introduced, > __free_one_page() does not prevent merging two different pageblocks, when > MAX_ORDER - 1 > pageblock_order. But current __free_one_page() > implementation > does prevent that. But it does prevent that only for isolated pageblock, not CMA, and yout patchset doesn't seem to expand that to CMA? Or am I missing something. >>> >>> Yeah, you are right. Originally, I thought preventing merging isolated >>> pageblock >>> with other types of pageblocks is sufficient, since MIGRATE_CMA is always >>> converted from MIGRATE_ISOLATE. But that is not true. I will rework the >>> code. >>> Thanks for pointing this out. >>> >> >> I find that two pageblocks with different migratetypes, like >> MIGRATE_RECLAIMABLE >> and MIGRATE_MOVABLE can be merged into a single free page after I checked >> __free_one_page() in detail and printed pageblock information during buddy >> page >> merging. > > Yes, that can happen. > > I am not sure what consequence it will cause. Do you have any idea? > > For MIGRATE_RECLAIMABLE or MIGRATE_MOVABLE or even MIGRATE_UNMOVABLE it's > absolutely fine. As long as these pageblocks are fully free (and they are if > it's a single free page spanning 2 pageblocks), they can be of any of these > type, as they can be reused as needed without causing fragmentation. > > But in case of MIGRATE_CMA and MIGRATE_ISOLATE, uncontrolled merging would > break the specifics of those types. That's why the code is careful for > MIGRATE_ISOLATE, and MIGRATE_CMA was until now done in MAX_ORDER granularity. Thanks for the explanation. Basically migratetypes that can fall back to each other can be merged into a single free page, right? How about MIGRATE_HIGHATOMIC? It should not be merged with other migratetypes from my understanding. -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 19 Nov 2021, at 10:15, Zi Yan wrote: > On 19 Nov 2021, at 7:33, Vlastimil Babka wrote: > >> On 11/15/21 20:37, Zi Yan wrote: >>> From: Zi Yan >>> >>> Hi David, >>> >>> You suggested to make alloc_contig_range() deal with pageblock_order >>> instead of >>> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >>> patchset is my attempt to achieve that. Please take a look and let me know >>> if >>> I am doing it correctly or not. >>> >>> From what my understanding, cma required alignment of >>> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was >>> introduced, >>> __free_one_page() does not prevent merging two different pageblocks, when >>> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() >>> implementation >>> does prevent that. >> >> But it does prevent that only for isolated pageblock, not CMA, and yout >> patchset doesn't seem to expand that to CMA? Or am I missing something. > > Yeah, you are right. Originally, I thought preventing merging isolated > pageblock > with other types of pageblocks is sufficient, since MIGRATE_CMA is always > converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. > Thanks for pointing this out. > I find that two pageblocks with different migratetypes, like MIGRATE_RECLAIMABLE and MIGRATE_MOVABLE can be merged into a single free page after I checked __free_one_page() in detail and printed pageblock information during buddy page merging. I am not sure what consequence it will cause. Do you have any idea? I will fix it in the next version of this patchset. >> >> >>> It should be OK to just align cma to pageblock_order. >>> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >>> pageblock_order as alignment too. >>> >>> In terms of virtio_mem, if I understand correctly, it relies on >>> alloc_contig_range() to obtain contiguous free pages and offlines them to >>> reduce >>> guest memory size. As the result of alloc_contig_range() alignment change, >>> virtio_mem should be able to just align PFNs to pageblock_order. >>> >>> Thanks. >>> >>> >>> [1] >>> https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ >>> >>> Zi Yan (3): >>> mm: cma: alloc_contig_range: use pageblock_order as the single >>> alignment. >>> drivers: virtio_mem: use pageblock size as the minimum virtio_mem >>> size. >>> arch: powerpc: adjust fadump alignment to be pageblock aligned. >>> >>> arch/powerpc/include/asm/fadump-internal.h | 4 +--- >>> drivers/virtio/virtio_mem.c| 6 ++ >>> include/linux/mmzone.h | 5 + >>> kernel/dma/contiguous.c| 2 +- >>> mm/cma.c | 6 ++ >>> mm/page_alloc.c| 12 +--- >>> 6 files changed, 12 insertions(+), 23 deletions(-) >>> > > -- > Best Regards, > Yan, Zi -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 19 Nov 2021, at 7:33, Vlastimil Babka wrote: > On 11/15/21 20:37, Zi Yan wrote: >> From: Zi Yan >> >> Hi David, >> >> You suggested to make alloc_contig_range() deal with pageblock_order instead >> of >> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >> patchset is my attempt to achieve that. Please take a look and let me know if >> I am doing it correctly or not. >> >> From what my understanding, cma required alignment of >> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, >> __free_one_page() does not prevent merging two different pageblocks, when >> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation >> does prevent that. > > But it does prevent that only for isolated pageblock, not CMA, and yout > patchset doesn't seem to expand that to CMA? Or am I missing something. Yeah, you are right. Originally, I thought preventing merging isolated pageblock with other types of pageblocks is sufficient, since MIGRATE_CMA is always converted from MIGRATE_ISOLATE. But that is not true. I will rework the code. Thanks for pointing this out. > > >> It should be OK to just align cma to pageblock_order. >> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >> pageblock_order as alignment too. >> >> In terms of virtio_mem, if I understand correctly, it relies on >> alloc_contig_range() to obtain contiguous free pages and offlines them to >> reduce >> guest memory size. As the result of alloc_contig_range() alignment change, >> virtio_mem should be able to just align PFNs to pageblock_order. >> >> Thanks. >> >> >> [1] >> https://lore.kernel.org/linux-mm/28b57903-fae6-47ac-7e1b-a1dd41421...@redhat.com/ >> >> Zi Yan (3): >> mm: cma: alloc_contig_range: use pageblock_order as the single >> alignment. >> drivers: virtio_mem: use pageblock size as the minimum virtio_mem >> size. >> arch: powerpc: adjust fadump alignment to be pageblock aligned. >> >> arch/powerpc/include/asm/fadump-internal.h | 4 +--- >> drivers/virtio/virtio_mem.c| 6 ++ >> include/linux/mmzone.h | 5 + >> kernel/dma/contiguous.c| 2 +- >> mm/cma.c | 6 ++ >> mm/page_alloc.c| 12 +--- >> 6 files changed, 12 insertions(+), 23 deletions(-) >> -- Best Regards, Yan, Zi signature.asc Description: OpenPGP digital signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 0/3] Use pageblock_order for cma and alloc_contig_range alignment.
On 16 Nov 2021, at 3:58, David Hildenbrand wrote: > On 15.11.21 20:37, Zi Yan wrote: >> From: Zi Yan >> >> Hi David, > > Hi, > > thanks for looking into this. > >> >> You suggested to make alloc_contig_range() deal with pageblock_order instead >> of >> MAX_ORDER - 1 and get rid of MAX_ORDER - 1 dependency in virtio_mem[1]. This >> patchset is my attempt to achieve that. Please take a look and let me know if >> I am doing it correctly or not. >> >> From what my understanding, cma required alignment of >> max(MAX_ORDER - 1, pageblock_order), because when MIGRATE_CMA was introduced, >> __free_one_page() does not prevent merging two different pageblocks, when >> MAX_ORDER - 1 > pageblock_order. But current __free_one_page() implementation >> does prevent that. It should be OK to just align cma to pageblock_order. >> alloc_contig_range() relies on MIGRATE_CMA to get free pages, so it can use >> pageblock_order as alignment too. > > I wonder if that's sufficient. Especially the outer_start logic in > alloc_contig_range() might be problematic. There are some ugly corner > cases with free pages/allocations spanning multiple pageblocks and we > only isolated a single pageblock. Thank you a lot for writing the list of these corner cases. They are very helpful! > > > Regarding CMA, we have to keep the following cases working: > > a) Different pageblock types (MIGRATE_CMA and !MIGRATE_CMA) in MAX_ORDER >- 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume either pageblock 0 is MIGRATE_CMA or pageblock 1 is MIGRATE_CMA, > but not both. We have to make sure alloc_contig_range() keeps working > correctly. This should be the case even with your change, as we won't > merging pages accross differing migratetypes. Yes. > > b) Migrating/freeing a MAX_ ORDER - 1 page while partially isolated: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume both are MIGRATE_CMA. Assume we want to either allocate from > pageblock 0 or pageblock 1. Especially, assume we want to allocate from > pageblock 1. While we would isolate pageblock 1, we wouldn't isolate > pageblock 0. > > What happens if we either have a free page spanning the MAX_ORDER - 1 > range already OR if we have to migrate a MAX_ORDER - 1 page, resulting > in a free MAX_ORDER - 1 page of which only the second pageblock is > isolated? We would end up essentially freeing a page that has mixed > pageblocks, essentially placing it in !MIGRATE_ISOLATE free lists ... I > might be wrong but I have the feeling that this would be problematic. > This could happen when start_isolate_page_range() stumbles upon a compound page with order >= pageblock_order or a free page with order >= pageblock_order, but should not. start_isolate_page_range() should check the actual page size, either compound page size or free page size, and set the migratetype across pageblocks if the page is bigger than pageblock size. More precisely set_migratetype_isolate() should do that. > c) Concurrent allocations: > [ MAX_ ORDER - 1 ] > [ pageblock 0 | pageblock 1] > > Assume b) but we have two concurrent CMA allocations to pageblock 0 and > pageblock 1, which would now be possible as start_isolate_page_range() > isolate would succeed on both. Two isolations will be serialized by the zone lock taken by set_migratetype_isolate(), so the concurrent allocation would not be a problem. If it is a MAX_ORDER-1 free page, the first comer should split it and only isolate one of the pageblock then second one can isolate the other pageblock. If it is a MAX_ORDER-1 compound page, the first comer should isolate both pageblocks, then the second one would fail. WDYT? In sum, it seems to me that the issue is page isolation code only sees pageblock without check the actual page. When there are multiple pageblocks belonging to one page, the problem appears. This should be fixed. > > > Regarding virtio-mem, we care about the following cases: > > a) Allocating parts from completely movable MAX_ ORDER - 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume pageblock 0 and pageblock 1 are either free or contain only > movable pages. Assume we allocated pageblock 0. We have to make sure we > can allocate pageblock 1. The other way around, assume we allocated > pageblock 1, we have to make sure we can allocate pageblock 0. > > Free pages spanning both pageblocks might be problematic. Can you elaborate a bit? If either of pageblock 0 and 1 is used by virtio-mem, why do we care the other? If pageblock 0 and 1 belong to the same page (either free or compound), they should have the same migratetype. If we want to just allocate one of them, we can split the free page or migrate the compound page then split the remaining free page. > > b) Allocate parts of partially movable MAX_ ORDER - 1 page: >[ MAX_ ORDER - 1 ] >[ pageblock 0 | pageblock 1] > > Assume pageblock 0 contains unmovable data but
