From: Mingjun Zhang <[email protected]>
free_contig_range frees cma pages one by one and MIGRATE_CMA pages will be
used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary
migration action when these pages reused by CMA.
Signed-off-by: Mingjun Zhang <[email protected]>
---
mm/page_alloc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0ee638f..84b9d84 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int cold)
* excessively into the page allocator
*/
if (migratetype >= MIGRATE_PCPTYPES) {
- if (unlikely(is_migrate_isolate(migratetype))) {
+ if (unlikely(is_migrate_isolate(migratetype))
+ || is_migrate_cma(migratetype))
free_one_page(zone, page, 0, migratetype);
goto out;
}
--
1.7.9.5
On 10/28/2013 4:42 AM, [email protected] wrote:
> From: Mingjun Zhang <[email protected]>
>
> free_contig_range frees cma pages one by one and MIGRATE_CMA pages will be
> used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary
> migration action when these pages reused by CMA.
>
> Signed-off-by: Mingjun Zhang <[email protected]>
> ---
> mm/page_alloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0ee638f..84b9d84 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int cold)
> * excessively into the page allocator
> */
> if (migratetype >= MIGRATE_PCPTYPES) {
> - if (unlikely(is_migrate_isolate(migratetype))) {
> + if (unlikely(is_migrate_isolate(migratetype))
> + || is_migrate_cma(migratetype))
> free_one_page(zone, page, 0, migratetype);
> goto out;
> }
>
I submitted something very similar a while ago
http://marc.info/?l=linaro-mm-sig&m=137645764208287&w=2 . Has the
opinion on this patch changed?
Thanks,
Laura
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
Hello,
On Mon, Oct 28, 2013 at 07:42:49PM +0800, [email protected] wrote:
> From: Mingjun Zhang <[email protected]>
>
> free_contig_range frees cma pages one by one and MIGRATE_CMA pages will be
> used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary
> migration action when these pages reused by CMA.
You are saying about the overhead but I'm not sure how much it is
because it wouldn't be frequent. Although it's frequent, migration is
already slow path and CMA migration is worse so I really wonder how much
pain is and how much this patch improve.
Having said that, it makes CMA allocation policy consistent which
is that CMA migration type is last fallback to minimize number of migration
and code peice you are adding is already low hit path so that I think
it has no problem.
>
> Signed-off-by: Mingjun Zhang <[email protected]>
> ---
> mm/page_alloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0ee638f..84b9d84 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int cold)
> * excessively into the page allocator
> */
> if (migratetype >= MIGRATE_PCPTYPES) {
> - if (unlikely(is_migrate_isolate(migratetype))) {
> + if (unlikely(is_migrate_isolate(migratetype))
> + || is_migrate_cma(migratetype))
The concern is likely/unlikely usage is proper in this code peice.
If we don't use memory isolation, the code path is used for only
MIGRATE_RESERVE which is very rare allocation in normal workload.
Even, in memory isolation environement, I'm not sure how many
CMA/HOTPLUG is used compared to normal alloc/free.
So, I think below is more proper?
if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
if (is_migrate_isolate(migratetype) || is_migrate_cma(migratetype))
I know it's an another topic but I'd like to disucss it in this time because
we will forget such trivial thing later, again.
}
> free_one_page(zone, page, 0, migratetype);
> goto out;
> }
> --
> 1.7.9.5
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
--
Kind regards,
Minchan Kim
> The concern is likely/unlikely usage is proper in this code peice.
> If we don't use memory isolation, the code path is used for only
> MIGRATE_RESERVE which is very rare allocation in normal workload.
>
> Even, in memory isolation environement, I'm not sure how many
> CMA/HOTPLUG is used compared to normal alloc/free.
> So, I think below is more proper?
>
> if (unlikely(migratetype >= MIGRATE_PCPTYPES)) {
> if (is_migrate_isolate(migratetype) || is_migrate_cma(migratetype))
>
> I know it's an another topic but I'd like to disucss it in this time because
> we will forget such trivial thing later, again.
I completely agree with this.
On Mon, Oct 28, 2013 at 07:42:49PM +0800, [email protected] wrote:
> From: Mingjun Zhang <[email protected]>
>
> free_contig_range frees cma pages one by one and MIGRATE_CMA pages will be
> used as MIGRATE_MOVEABLE pages in the pcp list, it causes unnecessary
> migration action when these pages reused by CMA.
>
> Signed-off-by: Mingjun Zhang <[email protected]>
> ---
> mm/page_alloc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0ee638f..84b9d84 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1362,7 +1362,8 @@ void free_hot_cold_page(struct page *page, int cold)
> * excessively into the page allocator
> */
> if (migratetype >= MIGRATE_PCPTYPES) {
> - if (unlikely(is_migrate_isolate(migratetype))) {
> + if (unlikely(is_migrate_isolate(migratetype))
> + || is_migrate_cma(migratetype))
> free_one_page(zone, page, 0, migratetype);
> goto out;
This slightly impacts the page allocator free path for a marginal gain
on CMA which are relatively rare allocations. There is no obvious
benefit to this patch as I expect CMA allocations to flush the PCP lists
when a range of pages have been isolated and migrated. Is there any
measurable benefit to this patch?
--
Mel Gorman
SUSE Labs