* gigantic hugepages vs. movable zones
@ 2017-07-26 10:50 Michal Hocko
2017-07-27 2:22 ` Aneesh Kumar K.V
2017-07-28 20:48 ` Mike Kravetz
0 siblings, 2 replies; 9+ messages in thread
From: Michal Hocko @ 2017-07-26 10:50 UTC (permalink / raw)
To: Luiz Capitulino, Mike Kravetz; +Cc: linux-mm, LKML
Hi,
I've just noticed that alloc_gigantic_page ignores movability of the
gigantic page and it uses any existing zone. Considering that
hugepage_migration_supported only supports 2MB and pgd level hugepages
then 1GB pages are not migratable and as such allocating them from a
movable zone will break the basic expectation of this zone. Standard
hugetlb allocations try to avoid that by using htlb_alloc_mask and I
believe we should do the same for gigantic pages as well.
I suspect this behavior is not intentional. What do you think about the
following untested patch?
---
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-26 10:50 gigantic hugepages vs. movable zones Michal Hocko
@ 2017-07-27 2:22 ` Aneesh Kumar K.V
2017-07-27 7:28 ` Michal Hocko
2017-07-28 20:48 ` Mike Kravetz
1 sibling, 1 reply; 9+ messages in thread
From: Aneesh Kumar K.V @ 2017-07-27 2:22 UTC (permalink / raw)
To: Michal Hocko, Luiz Capitulino, Mike Kravetz; +Cc: linux-mm, LKML
Michal Hocko <mhocko@kernel.org> writes:
> Hi,
> I've just noticed that alloc_gigantic_page ignores movability of the
> gigantic page and it uses any existing zone. Considering that
> hugepage_migration_supported only supports 2MB and pgd level hugepages
> then 1GB pages are not migratable and as such allocating them from a
> movable zone will break the basic expectation of this zone. Standard
> hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> believe we should do the same for gigantic pages as well.
>
> I suspect this behavior is not intentional. What do you think about the
> following untested patch?
I also noticed an unrelated issue with the usage of
start_isolate_page_range. On error we set the migrate type to
MIGRATE_MOVABLE. That may conflict with CMA pages ? Wondering whether
we should check for page's pageblock migrate type in
pfn_range_valid_gigantic() ?
-aneesh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-27 2:22 ` Aneesh Kumar K.V
@ 2017-07-27 7:28 ` Michal Hocko
2017-07-27 8:00 ` Aneesh Kumar K.V
0 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2017-07-27 7:28 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Luiz Capitulino, Mike Kravetz, linux-mm, LKML
On Thu 27-07-17 07:52:08, Aneesh Kumar K.V wrote:
> Michal Hocko <mhocko@kernel.org> writes:
>
> > Hi,
> > I've just noticed that alloc_gigantic_page ignores movability of the
> > gigantic page and it uses any existing zone. Considering that
> > hugepage_migration_supported only supports 2MB and pgd level hugepages
> > then 1GB pages are not migratable and as such allocating them from a
> > movable zone will break the basic expectation of this zone. Standard
> > hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> > believe we should do the same for gigantic pages as well.
> >
> > I suspect this behavior is not intentional. What do you think about the
> > following untested patch?
>
>
> I also noticed an unrelated issue with the usage of
> start_isolate_page_range. On error we set the migrate type to
> MIGRATE_MOVABLE.
Why that should be a problem? I think it is perfectly OK to have
MIGRATE_MOVABLE pageblocks inside kernel zones.
> That may conflict with CMA pages ?
How?
> Wondering whether we should check for page's pageblock migrate type in
> pfn_range_valid_gigantic() ?
I do not think so. Migrate type is just too lowlevel for
pfn_range_valid_gigantic. If something like that is really needed then
it should go down the CMA/alloc_contig_range path.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-27 7:28 ` Michal Hocko
@ 2017-07-27 8:00 ` Aneesh Kumar K.V
2017-07-27 8:12 ` Michal Hocko
0 siblings, 1 reply; 9+ messages in thread
From: Aneesh Kumar K.V @ 2017-07-27 8:00 UTC (permalink / raw)
To: Michal Hocko; +Cc: Luiz Capitulino, Mike Kravetz, linux-mm, LKML
On 07/27/2017 12:58 PM, Michal Hocko wrote:
> On Thu 27-07-17 07:52:08, Aneesh Kumar K.V wrote:
>> Michal Hocko <mhocko@kernel.org> writes:
>>
>>> Hi,
>>> I've just noticed that alloc_gigantic_page ignores movability of the
>>> gigantic page and it uses any existing zone. Considering that
>>> hugepage_migration_supported only supports 2MB and pgd level hugepages
>>> then 1GB pages are not migratable and as such allocating them from a
>>> movable zone will break the basic expectation of this zone. Standard
>>> hugetlb allocations try to avoid that by using htlb_alloc_mask and I
>>> believe we should do the same for gigantic pages as well.
>>>
>>> I suspect this behavior is not intentional. What do you think about the
>>> following untested patch?
>>
>>
>> I also noticed an unrelated issue with the usage of
>> start_isolate_page_range. On error we set the migrate type to
>> MIGRATE_MOVABLE.
>
> Why that should be a problem? I think it is perfectly OK to have
> MIGRATE_MOVABLE pageblocks inside kernel zones.
>
we can pick pages with migrate type movable and if we fail to isolate
won't we set the migrate type of that pages to MOVABLE ?
-aneesh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-27 8:00 ` Aneesh Kumar K.V
@ 2017-07-27 8:12 ` Michal Hocko
2017-07-27 8:22 ` Michal Hocko
0 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2017-07-27 8:12 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: Luiz Capitulino, Mike Kravetz, linux-mm, LKML
On Thu 27-07-17 13:30:31, Aneesh Kumar K.V wrote:
>
>
> On 07/27/2017 12:58 PM, Michal Hocko wrote:
> >On Thu 27-07-17 07:52:08, Aneesh Kumar K.V wrote:
> >>Michal Hocko <mhocko@kernel.org> writes:
> >>
> >>>Hi,
> >>>I've just noticed that alloc_gigantic_page ignores movability of the
> >>>gigantic page and it uses any existing zone. Considering that
> >>>hugepage_migration_supported only supports 2MB and pgd level hugepages
> >>>then 1GB pages are not migratable and as such allocating them from a
> >>>movable zone will break the basic expectation of this zone. Standard
> >>>hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> >>>believe we should do the same for gigantic pages as well.
> >>>
> >>>I suspect this behavior is not intentional. What do you think about the
> >>>following untested patch?
> >>
> >>
> >>I also noticed an unrelated issue with the usage of
> >>start_isolate_page_range. On error we set the migrate type to
> >>MIGRATE_MOVABLE.
> >
> >Why that should be a problem? I think it is perfectly OK to have
> >MIGRATE_MOVABLE pageblocks inside kernel zones.
> >
>
> we can pick pages with migrate type movable and if we fail to isolate won't
> we set the migrate type of that pages to MOVABLE ?
I do not see an immediate problem. GFP_KERNEL allocations can fallback
to movable migrate pageblocks AFAIR. But I am not very much familiar
with migratetypes. Vlastimil, could you have a look please?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-27 8:12 ` Michal Hocko
@ 2017-07-27 8:22 ` Michal Hocko
2017-07-27 11:56 ` Vlastimil Babka
0 siblings, 1 reply; 9+ messages in thread
From: Michal Hocko @ 2017-07-27 8:22 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: Luiz Capitulino, Mike Kravetz, linux-mm, LKML, Vlastimil Babka
[CC for real]
On Thu 27-07-17 10:12:36, Michal Hocko wrote:
> On Thu 27-07-17 13:30:31, Aneesh Kumar K.V wrote:
> >
> >
> > On 07/27/2017 12:58 PM, Michal Hocko wrote:
> > >On Thu 27-07-17 07:52:08, Aneesh Kumar K.V wrote:
> > >>Michal Hocko <mhocko@kernel.org> writes:
> > >>
> > >>>Hi,
> > >>>I've just noticed that alloc_gigantic_page ignores movability of the
> > >>>gigantic page and it uses any existing zone. Considering that
> > >>>hugepage_migration_supported only supports 2MB and pgd level hugepages
> > >>>then 1GB pages are not migratable and as such allocating them from a
> > >>>movable zone will break the basic expectation of this zone. Standard
> > >>>hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> > >>>believe we should do the same for gigantic pages as well.
> > >>>
> > >>>I suspect this behavior is not intentional. What do you think about the
> > >>>following untested patch?
> > >>
> > >>
> > >>I also noticed an unrelated issue with the usage of
> > >>start_isolate_page_range. On error we set the migrate type to
> > >>MIGRATE_MOVABLE.
> > >
> > >Why that should be a problem? I think it is perfectly OK to have
> > >MIGRATE_MOVABLE pageblocks inside kernel zones.
> > >
> >
> > we can pick pages with migrate type movable and if we fail to isolate won't
> > we set the migrate type of that pages to MOVABLE ?
>
> I do not see an immediate problem. GFP_KERNEL allocations can fallback
> to movable migrate pageblocks AFAIR. But I am not very much familiar
> with migratetypes. Vlastimil, could you have a look please?
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-27 8:22 ` Michal Hocko
@ 2017-07-27 11:56 ` Vlastimil Babka
0 siblings, 0 replies; 9+ messages in thread
From: Vlastimil Babka @ 2017-07-27 11:56 UTC (permalink / raw)
To: Michal Hocko, Aneesh Kumar K.V
Cc: Luiz Capitulino, Mike Kravetz, linux-mm, LKML, Joonsoo Kim
On 07/27/2017 10:22 AM, Michal Hocko wrote:
> [CC for real]
>
> On Thu 27-07-17 10:12:36, Michal Hocko wrote:
>> On Thu 27-07-17 13:30:31, Aneesh Kumar K.V wrote:
>>>
>>>
>>> On 07/27/2017 12:58 PM, Michal Hocko wrote:
>>>> On Thu 27-07-17 07:52:08, Aneesh Kumar K.V wrote:
>>>>> Michal Hocko <mhocko@kernel.org> writes:
>>>>>
>>>>>> Hi,
>>>>>> I've just noticed that alloc_gigantic_page ignores movability of the
>>>>>> gigantic page and it uses any existing zone. Considering that
>>>>>> hugepage_migration_supported only supports 2MB and pgd level hugepages
>>>>>> then 1GB pages are not migratable and as such allocating them from a
>>>>>> movable zone will break the basic expectation of this zone. Standard
>>>>>> hugetlb allocations try to avoid that by using htlb_alloc_mask and I
>>>>>> believe we should do the same for gigantic pages as well.
>>>>>>
>>>>>> I suspect this behavior is not intentional. What do you think about the
>>>>>> following untested patch?
>>>>>
>>>>>
>>>>> I also noticed an unrelated issue with the usage of
>>>>> start_isolate_page_range. On error we set the migrate type to
>>>>> MIGRATE_MOVABLE.
>>>>
>>>> Why that should be a problem? I think it is perfectly OK to have
>>>> MIGRATE_MOVABLE pageblocks inside kernel zones.
>>>>
>>>
>>> we can pick pages with migrate type movable and if we fail to isolate won't
^ CMA
>>> we set the migrate type of that pages to MOVABLE ?
Yes, it seems we can silently kill CMA pageblocks in such case. Joonsoo,
can you check?
>>
>> I do not see an immediate problem. GFP_KERNEL allocations can fallback
>> to movable migrate pageblocks AFAIR. But I am not very much familiar
>> with migratetypes. Vlastimil, could you have a look please?
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-26 10:50 gigantic hugepages vs. movable zones Michal Hocko
2017-07-27 2:22 ` Aneesh Kumar K.V
@ 2017-07-28 20:48 ` Mike Kravetz
2017-07-31 6:47 ` Michal Hocko
1 sibling, 1 reply; 9+ messages in thread
From: Mike Kravetz @ 2017-07-28 20:48 UTC (permalink / raw)
To: Michal Hocko, Luiz Capitulino; +Cc: linux-mm, LKML
On 07/26/2017 03:50 AM, Michal Hocko wrote:
> Hi,
> I've just noticed that alloc_gigantic_page ignores movability of the
> gigantic page and it uses any existing zone. Considering that
> hugepage_migration_supported only supports 2MB and pgd level hugepages
> then 1GB pages are not migratable and as such allocating them from a
> movable zone will break the basic expectation of this zone. Standard
> hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> believe we should do the same for gigantic pages as well.
>
> I suspect this behavior is not intentional. What do you think about the
> following untested patch?
> ---
> From 542d32c1eca7dcf38afca1a91bca4a472f6e8651 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.com>
> Date: Wed, 26 Jul 2017 12:43:43 +0200
> Subject: [PATCH] mm, hugetlb: do not allocate non-migrateable gigantic pages
> from movable zones
>
> alloc_gigantic_page doesn't consider movability of the gigantic hugetlb
> when scanning eligible ranges for the allocation. As 1GB hugetlb pages
> are not movable currently this can break the movable zone assumption
> that all allocations are migrateable and as such break memory hotplug.
>
> Reorganize the code and use the standard zonelist allocations scheme
> that we use for standard hugetbl pages. htlb_alloc_mask will ensure that
> only migratable hugetlb pages will ever see a movable zone.
>
> Fixes: 944d9fec8d7a ("hugetlb: add support for gigantic page allocation at runtime")
> Signed-off-by: Michal Hocko <mhocko@suse.com>
This seems reasonable to me, and I like the fact that the code is more
like the default huge page case. I don't see any issues with the code.
I did some simple smoke testing of allocating 1G pages with the new code
and ensuring they ended up as expected.
Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
--
Mike Kravetz
> ---
> mm/hugetlb.c | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index bc48ee783dd9..60530bb3d228 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1066,11 +1066,11 @@ static void free_gigantic_page(struct page *page, unsigned int order)
> }
>
> static int __alloc_gigantic_page(unsigned long start_pfn,
> - unsigned long nr_pages)
> + unsigned long nr_pages, gfp_t gfp_mask)
> {
> unsigned long end_pfn = start_pfn + nr_pages;
> return alloc_contig_range(start_pfn, end_pfn, MIGRATE_MOVABLE,
> - GFP_KERNEL);
> + gfp_mask);
> }
>
> static bool pfn_range_valid_gigantic(struct zone *z,
> @@ -1108,19 +1108,24 @@ static bool zone_spans_last_pfn(const struct zone *zone,
> return zone_spans_pfn(zone, last_pfn);
> }
>
> -static struct page *alloc_gigantic_page(int nid, unsigned int order)
> +static struct page *alloc_gigantic_page(int nid, struct hstate *h)
> {
> + unsigned int order = huge_page_order(h);
> unsigned long nr_pages = 1 << order;
> unsigned long ret, pfn, flags;
> - struct zone *z;
> + struct zonelist *zonelist;
> + struct zone *zone;
> + struct zoneref *z;
> + gfp_t gfp_mask;
>
> - z = NODE_DATA(nid)->node_zones;
> - for (; z - NODE_DATA(nid)->node_zones < MAX_NR_ZONES; z++) {
> - spin_lock_irqsave(&z->lock, flags);
> + gfp_mask = htlb_alloc_mask(h) | __GFP_THISNODE;
> + zonelist = node_zonelist(nid, gfp_mask);
> + for_each_zone_zonelist_nodemask(zone, z, zonelist, gfp_zone(gfp_mask), NULL) {
> + spin_lock_irqsave(&zone->lock, flags);
>
> - pfn = ALIGN(z->zone_start_pfn, nr_pages);
> - while (zone_spans_last_pfn(z, pfn, nr_pages)) {
> - if (pfn_range_valid_gigantic(z, pfn, nr_pages)) {
> + pfn = ALIGN(zone->zone_start_pfn, nr_pages);
> + while (zone_spans_last_pfn(zone, pfn, nr_pages)) {
> + if (pfn_range_valid_gigantic(zone, pfn, nr_pages)) {
> /*
> * We release the zone lock here because
> * alloc_contig_range() will also lock the zone
> @@ -1128,16 +1133,16 @@ static struct page *alloc_gigantic_page(int nid, unsigned int order)
> * spinning on this lock, it may win the race
> * and cause alloc_contig_range() to fail...
> */
> - spin_unlock_irqrestore(&z->lock, flags);
> - ret = __alloc_gigantic_page(pfn, nr_pages);
> + spin_unlock_irqrestore(&zone->lock, flags);
> + ret = __alloc_gigantic_page(pfn, nr_pages, gfp_mask);
> if (!ret)
> return pfn_to_page(pfn);
> - spin_lock_irqsave(&z->lock, flags);
> + spin_lock_irqsave(&zone->lock, flags);
> }
> pfn += nr_pages;
> }
>
> - spin_unlock_irqrestore(&z->lock, flags);
> + spin_unlock_irqrestore(&zone->lock, flags);
> }
>
> return NULL;
> @@ -1150,7 +1155,7 @@ static struct page *alloc_fresh_gigantic_page_node(struct hstate *h, int nid)
> {
> struct page *page;
>
> - page = alloc_gigantic_page(nid, huge_page_order(h));
> + page = alloc_gigantic_page(nid, h);
> if (page) {
> prep_compound_gigantic_page(page, huge_page_order(h));
> prep_new_huge_page(h, page, nid);
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: gigantic hugepages vs. movable zones
2017-07-28 20:48 ` Mike Kravetz
@ 2017-07-31 6:47 ` Michal Hocko
0 siblings, 0 replies; 9+ messages in thread
From: Michal Hocko @ 2017-07-31 6:47 UTC (permalink / raw)
To: Mike Kravetz; +Cc: Luiz Capitulino, linux-mm, LKML
On Fri 28-07-17 13:48:28, Mike Kravetz wrote:
> On 07/26/2017 03:50 AM, Michal Hocko wrote:
> > Hi,
> > I've just noticed that alloc_gigantic_page ignores movability of the
> > gigantic page and it uses any existing zone. Considering that
> > hugepage_migration_supported only supports 2MB and pgd level hugepages
> > then 1GB pages are not migratable and as such allocating them from a
> > movable zone will break the basic expectation of this zone. Standard
> > hugetlb allocations try to avoid that by using htlb_alloc_mask and I
> > believe we should do the same for gigantic pages as well.
> >
> > I suspect this behavior is not intentional. What do you think about the
> > following untested patch?
> > ---
> > From 542d32c1eca7dcf38afca1a91bca4a472f6e8651 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.com>
> > Date: Wed, 26 Jul 2017 12:43:43 +0200
> > Subject: [PATCH] mm, hugetlb: do not allocate non-migrateable gigantic pages
> > from movable zones
> >
> > alloc_gigantic_page doesn't consider movability of the gigantic hugetlb
> > when scanning eligible ranges for the allocation. As 1GB hugetlb pages
> > are not movable currently this can break the movable zone assumption
> > that all allocations are migrateable and as such break memory hotplug.
> >
> > Reorganize the code and use the standard zonelist allocations scheme
> > that we use for standard hugetbl pages. htlb_alloc_mask will ensure that
> > only migratable hugetlb pages will ever see a movable zone.
> >
> > Fixes: 944d9fec8d7a ("hugetlb: add support for gigantic page allocation at runtime")
> > Signed-off-by: Michal Hocko <mhocko@suse.com>
>
> This seems reasonable to me, and I like the fact that the code is more
> like the default huge page case. I don't see any issues with the code.
> I did some simple smoke testing of allocating 1G pages with the new code
> and ensuring they ended up as expected.
>
> Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
Thanks a lot Mike! I will play with this some more today and tomorrow
and send the final patch later this week.
--
Michal Hocko
SUSE Labs
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-07-31 6:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 10:50 gigantic hugepages vs. movable zones Michal Hocko
2017-07-27 2:22 ` Aneesh Kumar K.V
2017-07-27 7:28 ` Michal Hocko
2017-07-27 8:00 ` Aneesh Kumar K.V
2017-07-27 8:12 ` Michal Hocko
2017-07-27 8:22 ` Michal Hocko
2017-07-27 11:56 ` Vlastimil Babka
2017-07-28 20:48 ` Mike Kravetz
2017-07-31 6:47 ` Michal Hocko
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).