alloc_contig_range() initiates compaction and eventual migration for
the purpose of either CMA or HugeTLB allocation. At present, reason
code remains the same MR_CMA for either of those cases. Lets add a
new reason code which will differentiate the purpose of migration
as HugeTLB allocation instead.
Signed-off-by: Anshuman Khandual <[email protected]>
---
include/linux/migrate.h | 1 +
include/trace/events/migrate.h | 3 ++-
mm/page_alloc.c | 14 ++++++++++----
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index a732598fcf83..44381c33a2bd 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -26,6 +26,7 @@ enum migrate_reason {
MR_MEMPOLICY_MBIND,
MR_NUMA_MISPLACED,
MR_CMA,
+ MR_HUGETLB,
MR_TYPES
};
diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index bcf4daccd6be..61474c93f8f3 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -20,7 +20,8 @@
EM( MR_SYSCALL, "syscall_or_cpuset") \
EM( MR_MEMPOLICY_MBIND, "mempolicy_mbind") \
EM( MR_NUMA_MISPLACED, "numa_misplaced") \
- EMe(MR_CMA, "cma")
+ EM( MR_CMA, "cma") \
+ EMe(MR_HUGETLB, "hugetlb")
/*
* First define the enums in the above macros to be exported to userspace
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 242565855d05..ce8a2f2d4994 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7588,13 +7588,14 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
/* [start, end) must belong to a single zone. */
static int __alloc_contig_migrate_range(struct compact_control *cc,
- unsigned long start, unsigned long end)
+ unsigned long start, unsigned long end,
+ unsigned migratetype)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned long nr_reclaimed;
unsigned long pfn = start;
unsigned int tries = 0;
- int ret = 0;
+ int ret = 0, migrate_reason = 0;
migrate_prep();
@@ -7621,8 +7622,13 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
&cc->migratepages);
cc->nr_migratepages -= nr_reclaimed;
+ if (migratetype == MIGRATE_CMA)
+ migrate_reason = MR_CMA;
+ else
+ migrate_reason = MR_HUGETLB;
+
ret = migrate_pages(&cc->migratepages, new_page_alloc_contig,
- NULL, 0, cc->mode, MR_CMA);
+ NULL, 0, cc->mode, migrate_reason);
}
if (ret < 0) {
putback_movable_pages(&cc->migratepages);
@@ -7710,7 +7716,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
* allocated. So, if we fall through be sure to clear ret so that
* -EBUSY is not accidentally used or returned to caller.
*/
- ret = __alloc_contig_migrate_range(&cc, start, end);
+ ret = __alloc_contig_migrate_range(&cc, start, end, migratetype);
if (ret && ret != -EBUSY)
goto done;
ret =0;
--
2.11.0
On 01/30/2018 08:37 AM, Anshuman Khandual wrote:
> @@ -7621,8 +7622,13 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
> &cc->migratepages);
> cc->nr_migratepages -= nr_reclaimed;
>
> + if (migratetype == MIGRATE_CMA)
> + migrate_reason = MR_CMA;
> + else
> + migrate_reason = MR_HUGETLB;
> +
> ret = migrate_pages(&cc->migratepages, new_page_alloc_contig,
Oops, this is on top of the changes from the other RFC regarding migration
helper functions.
On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
> alloc_contig_range() initiates compaction and eventual migration for
> the purpose of either CMA or HugeTLB allocation. At present, reason
> code remains the same MR_CMA for either of those cases. Lets add a
> new reason code which will differentiate the purpose of migration
> as HugeTLB allocation instead.
Why do we need it?
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> include/linux/migrate.h | 1 +
> include/trace/events/migrate.h | 3 ++-
> mm/page_alloc.c | 14 ++++++++++----
> 3 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index a732598fcf83..44381c33a2bd 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -26,6 +26,7 @@ enum migrate_reason {
> MR_MEMPOLICY_MBIND,
> MR_NUMA_MISPLACED,
> MR_CMA,
> + MR_HUGETLB,
> MR_TYPES
> };
>
> diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
> index bcf4daccd6be..61474c93f8f3 100644
> --- a/include/trace/events/migrate.h
> +++ b/include/trace/events/migrate.h
> @@ -20,7 +20,8 @@
> EM( MR_SYSCALL, "syscall_or_cpuset") \
> EM( MR_MEMPOLICY_MBIND, "mempolicy_mbind") \
> EM( MR_NUMA_MISPLACED, "numa_misplaced") \
> - EMe(MR_CMA, "cma")
> + EM( MR_CMA, "cma") \
> + EMe(MR_HUGETLB, "hugetlb")
>
> /*
> * First define the enums in the above macros to be exported to userspace
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 242565855d05..ce8a2f2d4994 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7588,13 +7588,14 @@ static unsigned long pfn_max_align_up(unsigned long pfn)
>
> /* [start, end) must belong to a single zone. */
> static int __alloc_contig_migrate_range(struct compact_control *cc,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned long end,
> + unsigned migratetype)
> {
> /* This function is based on compact_zone() from compaction.c. */
> unsigned long nr_reclaimed;
> unsigned long pfn = start;
> unsigned int tries = 0;
> - int ret = 0;
> + int ret = 0, migrate_reason = 0;
>
> migrate_prep();
>
> @@ -7621,8 +7622,13 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
> &cc->migratepages);
> cc->nr_migratepages -= nr_reclaimed;
>
> + if (migratetype == MIGRATE_CMA)
> + migrate_reason = MR_CMA;
> + else
> + migrate_reason = MR_HUGETLB;
> +
> ret = migrate_pages(&cc->migratepages, new_page_alloc_contig,
> - NULL, 0, cc->mode, MR_CMA);
> + NULL, 0, cc->mode, migrate_reason);
> }
> if (ret < 0) {
> putback_movable_pages(&cc->migratepages);
> @@ -7710,7 +7716,7 @@ int alloc_contig_range(unsigned long start, unsigned long end,
> * allocated. So, if we fall through be sure to clear ret so that
> * -EBUSY is not accidentally used or returned to caller.
> */
> - ret = __alloc_contig_migrate_range(&cc, start, end);
> + ret = __alloc_contig_migrate_range(&cc, start, end, migratetype);
> if (ret && ret != -EBUSY)
> goto done;
> ret =0;
> --
> 2.11.0
>
--
Michal Hocko
SUSE Labs
On 01/30/2018 01:29 PM, Michal Hocko wrote:
> On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
>> alloc_contig_range() initiates compaction and eventual migration for
>> the purpose of either CMA or HugeTLB allocation. At present, reason
>> code remains the same MR_CMA for either of those cases. Lets add a
>> new reason code which will differentiate the purpose of migration
>> as HugeTLB allocation instead.
> Why do we need it?
The same reason why we have MR_CMA (maybe some other ones as well) at
present, for reporting purpose through traces at the least. It just
seemed like same reason code is being used for two different purpose
of migration.
On 01/30/2018 06:25 PM, Anshuman Khandual wrote:
> On 01/30/2018 01:29 PM, Michal Hocko wrote:
>> On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
>>> alloc_contig_range() initiates compaction and eventual migration for
>>> the purpose of either CMA or HugeTLB allocation. At present, reason
>>> code remains the same MR_CMA for either of those cases. Lets add a
>>> new reason code which will differentiate the purpose of migration
>>> as HugeTLB allocation instead.
>> Why do we need it?
>
> The same reason why we have MR_CMA (maybe some other ones as well) at
> present, for reporting purpose through traces at the least. It just
> seemed like same reason code is being used for two different purpose
> of migration.
>
I was 'thinking' that we could potentially open up alloc_contig_range()
for more general purpose use. Users would not call alloc_contig_range
directly, but it would be wrapped in a more user friendly API. Or,
perhaps it gets modified and becomes something else. Still just thinking
as part of "how do we provide a more general purpose interface for
allocation of more than MAX_ORDER contiguous pages?".
Not sure that we should be adding to the current alloc_contig_range
interface until we decide it is something which will be useful long term.
--
Mike Kravetz
On Wed 31-01-18 07:55:05, Anshuman Khandual wrote:
> On 01/30/2018 01:29 PM, Michal Hocko wrote:
> > On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
> >> alloc_contig_range() initiates compaction and eventual migration for
> >> the purpose of either CMA or HugeTLB allocation. At present, reason
> >> code remains the same MR_CMA for either of those cases. Lets add a
> >> new reason code which will differentiate the purpose of migration
> >> as HugeTLB allocation instead.
> > Why do we need it?
>
> The same reason why we have MR_CMA (maybe some other ones as well) at
> present, for reporting purpose through traces at the least. It just
> seemed like same reason code is being used for two different purpose
> of migration.
But do we have any real user asking for this kind of information?
--
Michal Hocko
SUSE Labs
On Wed, 31 Jan 2018 08:58:52 +0100 Michal Hocko <[email protected]> wrote:
> On Wed 31-01-18 07:55:05, Anshuman Khandual wrote:
> > On 01/30/2018 01:29 PM, Michal Hocko wrote:
> > > On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
> > >> alloc_contig_range() initiates compaction and eventual migration for
> > >> the purpose of either CMA or HugeTLB allocation. At present, reason
> > >> code remains the same MR_CMA for either of those cases. Lets add a
> > >> new reason code which will differentiate the purpose of migration
> > >> as HugeTLB allocation instead.
> > > Why do we need it?
> >
> > The same reason why we have MR_CMA (maybe some other ones as well) at
> > present, for reporting purpose through traces at the least. It just
> > seemed like same reason code is being used for two different purpose
> > of migration.
>
> But do we have any real user asking for this kind of information?
It seems a reasonable cleanup: reusing MR_CMA for hugetlb just because
it happens to do the right thing is a bit hacky - the two things aren't
particularly related and a reader could be excused for feeling
confusion.
But the change seems incomplete:
> + if (migratetype == MIGRATE_CMA)
> + migrate_reason = MR_CMA;
> + else
> + migrate_reason = MR_HUGETLB;
If we're going to do this cleanup then shouldn't we go all the way and
add MIGRATE_HUGETLB?
Alternatively... instead of adding MR_HUGETLB (and perhaps
MIGRATE_HUGETLB), can we identify what characteristics these two things
have in common and invent a new, more generic identifier? So that both
migrate-for-CMA and migrate-for-HUGETLB will use MIGRATE_NEWNAME and
MR_NEWNAME?
On Wed 31-01-18 12:12:17, Andrew Morton wrote:
> On Wed, 31 Jan 2018 08:58:52 +0100 Michal Hocko <[email protected]> wrote:
>
> > On Wed 31-01-18 07:55:05, Anshuman Khandual wrote:
> > > On 01/30/2018 01:29 PM, Michal Hocko wrote:
> > > > On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
> > > >> alloc_contig_range() initiates compaction and eventual migration for
> > > >> the purpose of either CMA or HugeTLB allocation. At present, reason
> > > >> code remains the same MR_CMA for either of those cases. Lets add a
> > > >> new reason code which will differentiate the purpose of migration
> > > >> as HugeTLB allocation instead.
> > > > Why do we need it?
> > >
> > > The same reason why we have MR_CMA (maybe some other ones as well) at
> > > present, for reporting purpose through traces at the least. It just
> > > seemed like same reason code is being used for two different purpose
> > > of migration.
> >
> > But do we have any real user asking for this kind of information?
>
> It seems a reasonable cleanup: reusing MR_CMA for hugetlb just because
> it happens to do the right thing is a bit hacky - the two things aren't
> particularly related and a reader could be excused for feeling
> confusion.
My bad! I thought this is a tracepoint thingy. But it seems to be only
used as a migration reason for page_owner. Now it makes more sense.
> But the change seems incomplete:
>
> > + if (migratetype == MIGRATE_CMA)
> > + migrate_reason = MR_CMA;
> > + else
> > + migrate_reason = MR_HUGETLB;
>
> If we're going to do this cleanup then shouldn't we go all the way and
> add MIGRATE_HUGETLB?
Yes. We can expect more users of alloc_contig_range in future. Maybe we
want to use MR_CONTIG_RANGE instead.
--
Michal Hocko
SUSE Labs
On 02/01/2018 02:02 AM, Michal Hocko wrote:
> On Wed 31-01-18 12:12:17, Andrew Morton wrote:
>> On Wed, 31 Jan 2018 08:58:52 +0100 Michal Hocko <[email protected]> wrote:
>>
>>> On Wed 31-01-18 07:55:05, Anshuman Khandual wrote:
>>>> On 01/30/2018 01:29 PM, Michal Hocko wrote:
>>>>> On Tue 30-01-18 08:37:14, Anshuman Khandual wrote:
>>>>>> alloc_contig_range() initiates compaction and eventual migration for
>>>>>> the purpose of either CMA or HugeTLB allocation. At present, reason
>>>>>> code remains the same MR_CMA for either of those cases. Lets add a
>>>>>> new reason code which will differentiate the purpose of migration
>>>>>> as HugeTLB allocation instead.
>>>>> Why do we need it?
>>>>
>>>> The same reason why we have MR_CMA (maybe some other ones as well) at
>>>> present, for reporting purpose through traces at the least. It just
>>>> seemed like same reason code is being used for two different purpose
>>>> of migration.
>>>
>>> But do we have any real user asking for this kind of information?
>>
>> It seems a reasonable cleanup: reusing MR_CMA for hugetlb just because
>> it happens to do the right thing is a bit hacky - the two things aren't
>> particularly related and a reader could be excused for feeling
>> confusion.
>
> My bad! I thought this is a tracepoint thingy. But it seems to be only
> used as a migration reason for page_owner. Now it makes more sense.
>
>> But the change seems incomplete:
>>
>>> + if (migratetype == MIGRATE_CMA)
>>> + migrate_reason = MR_CMA;
>>> + else
>>> + migrate_reason = MR_HUGETLB;
>>
>> If we're going to do this cleanup then shouldn't we go all the way and
>> add MIGRATE_HUGETLB?
>
> Yes. We can expect more users of alloc_contig_range in future. Maybe we
> want to use MR_CONTIG_RANGE instead.
MR_CONTIG_RANGE can be a replacement for both MR_CMA and MR_HUGETLB.
alloc_contig_range() initiates compaction and eventual migration for
the purpose of either CMA or HugeTLB allocation. At present, reason
code remains the same MR_CMA for either of these cases. Lets make it
MR_CONTIG_RANGE which will appropriately reflect reason code in both
these cases.
Signed-off-by: Anshuman Khandual <[email protected]>
---
arch/powerpc/mm/mmu_context_iommu.c | 2 +-
include/linux/migrate.h | 2 +-
include/trace/events/migrate.h | 2 +-
mm/page_alloc.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
index 91ee2231c527..4c615fcb0cf0 100644
--- a/arch/powerpc/mm/mmu_context_iommu.c
+++ b/arch/powerpc/mm/mmu_context_iommu.c
@@ -111,7 +111,7 @@ static int mm_iommu_move_page_from_cma(struct page *page)
put_page(page); /* Drop the gup reference */
ret = migrate_pages(&cma_migrate_pages, new_iommu_non_cma_page,
- NULL, 0, MIGRATE_SYNC, MR_CMA);
+ NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE);
if (ret) {
if (!list_empty(&cma_migrate_pages))
putback_movable_pages(&cma_migrate_pages);
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index a732598fcf83..7e7e2606bb4c 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -25,7 +25,7 @@ enum migrate_reason {
MR_SYSCALL, /* also applies to cpusets */
MR_MEMPOLICY_MBIND,
MR_NUMA_MISPLACED,
- MR_CMA,
+ MR_CONTIG_RANGE,
MR_TYPES
};
diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index bcf4daccd6be..711372845945 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -20,7 +20,7 @@
EM( MR_SYSCALL, "syscall_or_cpuset") \
EM( MR_MEMPOLICY_MBIND, "mempolicy_mbind") \
EM( MR_NUMA_MISPLACED, "numa_misplaced") \
- EMe(MR_CMA, "cma")
+ EMe(MR_CONTIG_RANGE, "contig_range")
/*
* First define the enums in the above macros to be exported to userspace
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 242565855d05..b9a22e16b4cf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7622,7 +7622,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
cc->nr_migratepages -= nr_reclaimed;
ret = migrate_pages(&cc->migratepages, new_page_alloc_contig,
- NULL, 0, cc->mode, MR_CMA);
+ NULL, 0, cc->mode, MR_CONTIG_RANGE);
}
if (ret < 0) {
putback_movable_pages(&cc->migratepages);
--
2.11.0
On Fri 02-02-18 14:45:18, Anshuman Khandual wrote:
> alloc_contig_range() initiates compaction and eventual migration for
> the purpose of either CMA or HugeTLB allocation. At present, reason
> code remains the same MR_CMA for either of these cases. Lets make it
> MR_CONTIG_RANGE which will appropriately reflect reason code in both
> these cases.
It is not very specific but I guess this is better than inventing a code
for each source. If we ever get to need distinguish all of them then we
should better mark a function which calls the allocator or something
like that.
> Signed-off-by: Anshuman Khandual <[email protected]>
Acked-by: Michal Hocko <[email protected]>
> ---
> arch/powerpc/mm/mmu_context_iommu.c | 2 +-
> include/linux/migrate.h | 2 +-
> include/trace/events/migrate.h | 2 +-
> mm/page_alloc.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/mm/mmu_context_iommu.c b/arch/powerpc/mm/mmu_context_iommu.c
> index 91ee2231c527..4c615fcb0cf0 100644
> --- a/arch/powerpc/mm/mmu_context_iommu.c
> +++ b/arch/powerpc/mm/mmu_context_iommu.c
> @@ -111,7 +111,7 @@ static int mm_iommu_move_page_from_cma(struct page *page)
> put_page(page); /* Drop the gup reference */
>
> ret = migrate_pages(&cma_migrate_pages, new_iommu_non_cma_page,
> - NULL, 0, MIGRATE_SYNC, MR_CMA);
> + NULL, 0, MIGRATE_SYNC, MR_CONTIG_RANGE);
> if (ret) {
> if (!list_empty(&cma_migrate_pages))
> putback_movable_pages(&cma_migrate_pages);
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index a732598fcf83..7e7e2606bb4c 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -25,7 +25,7 @@ enum migrate_reason {
> MR_SYSCALL, /* also applies to cpusets */
> MR_MEMPOLICY_MBIND,
> MR_NUMA_MISPLACED,
> - MR_CMA,
> + MR_CONTIG_RANGE,
> MR_TYPES
> };
>
> diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
> index bcf4daccd6be..711372845945 100644
> --- a/include/trace/events/migrate.h
> +++ b/include/trace/events/migrate.h
> @@ -20,7 +20,7 @@
> EM( MR_SYSCALL, "syscall_or_cpuset") \
> EM( MR_MEMPOLICY_MBIND, "mempolicy_mbind") \
> EM( MR_NUMA_MISPLACED, "numa_misplaced") \
> - EMe(MR_CMA, "cma")
> + EMe(MR_CONTIG_RANGE, "contig_range")
>
> /*
> * First define the enums in the above macros to be exported to userspace
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 242565855d05..b9a22e16b4cf 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7622,7 +7622,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
> cc->nr_migratepages -= nr_reclaimed;
>
> ret = migrate_pages(&cc->migratepages, new_page_alloc_contig,
> - NULL, 0, cc->mode, MR_CMA);
> + NULL, 0, cc->mode, MR_CONTIG_RANGE);
> }
> if (ret < 0) {
> putback_movable_pages(&cc->migratepages);
> --
> 2.11.0
>
--
Michal Hocko
SUSE Labs