linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path
@ 2016-06-16  2:42 Ganesh Mahendran
  2016-06-16 10:17 ` Michal Hocko
  2016-06-21  0:52 ` David Rientjes
  0 siblings, 2 replies; 3+ messages in thread
From: Ganesh Mahendran @ 2016-06-16  2:42 UTC (permalink / raw)
  To: linux-mm, linux-kernel
  Cc: akpm, vbabka, iamjoonsoo.kim, mhocko, mina86, minchan, mgorman,
	rientjes, kirill.shutemov, izumi.taku, hannes, khandual,
	bsingharora, Ganesh Mahendran

In direct compact path, both __alloc_pages_direct_compact and
try_to_compact_pages check (order == 0).

This patch removes the check in __alloc_pages_direct_compact() and
move the modifying of current->flags to the entry point of direct
page compaction where we really do the compaction.

Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
---
v2:
  remove the check in __alloc_pages_direct_compact - Anshuman Khandual
v3:
  remove check in __alloc_pages_direct_compact and move current->flags
  modifying to try_to_compact_pages
---
 mm/compaction.c | 7 ++++++-
 mm/page_alloc.c | 5 -----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index fbb7b38..dcfaf57 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1686,12 +1686,16 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
 
 	*contended = COMPACT_CONTENDED_NONE;
 
-	/* Check if the GFP flags allow compaction */
+	/*
+	 * Check if this is an order-0 request and
+	 * if the GFP flags allow compaction.
+	 */
 	if (!order || !may_enter_fs || !may_perform_io)
 		return COMPACT_SKIPPED;
 
 	trace_mm_compaction_try_to_compact_pages(order, gfp_mask, mode);
 
+	current->flags |= PF_MEMALLOC;
 	/* Compact each zone in the list */
 	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
 								ac->nodemask) {
@@ -1768,6 +1772,7 @@ break_loop:
 		all_zones_contended = 0;
 		break;
 	}
+	current->flags &= ~PF_MEMALLOC;
 
 	/*
 	 * If at least one zone wasn't deferred or skipped, we report if all
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b9ea618..dd3a2b6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3173,13 +3173,8 @@ __alloc_pages_direct_compact(gfp_t gfp_mask, unsigned int order,
 	struct page *page;
 	int contended_compaction;
 
-	if (!order)
-		return NULL;
-
-	current->flags |= PF_MEMALLOC;
 	*compact_result = try_to_compact_pages(gfp_mask, order, alloc_flags, ac,
 						mode, &contended_compaction);
-	current->flags &= ~PF_MEMALLOC;
 
 	if (*compact_result <= COMPACT_INACTIVE)
 		return NULL;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path
  2016-06-16  2:42 [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path Ganesh Mahendran
@ 2016-06-16 10:17 ` Michal Hocko
  2016-06-21  0:52 ` David Rientjes
  1 sibling, 0 replies; 3+ messages in thread
From: Michal Hocko @ 2016-06-16 10:17 UTC (permalink / raw)
  To: Ganesh Mahendran
  Cc: linux-mm, linux-kernel, akpm, vbabka, iamjoonsoo.kim, mina86,
	minchan, mgorman, rientjes, kirill.shutemov, izumi.taku, hannes,
	khandual, bsingharora

On Thu 16-06-16 10:42:36, Ganesh Mahendran wrote:
> In direct compact path, both __alloc_pages_direct_compact and
> try_to_compact_pages check (order == 0).
> 
> This patch removes the check in __alloc_pages_direct_compact() and
> move the modifying of current->flags to the entry point of direct
> page compaction where we really do the compaction.

I do not have strong opinion on whether the order should be checked at
try_to_compact_pages or __alloc_pages_direct_compact. The later one
sounds more suitable because no further steps are really appropriate for
order == 0. try_to_compact_pages is not used anywhere else to be order
aware (maybe it will in future, who knows). But this all is a matter of
taste.

[...]
> diff --git a/mm/compaction.c b/mm/compaction.c
> index fbb7b38..dcfaf57 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1686,12 +1686,16 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
>  
>  	*contended = COMPACT_CONTENDED_NONE;
>  
> -	/* Check if the GFP flags allow compaction */
> +	/*
> +	 * Check if this is an order-0 request and
> +	 * if the GFP flags allow compaction.
> +	 */

If you are touching this comment then it would be appropriate to change
it from "what is checked" into "why it is checked" because that is far
from obvious from this context. Especially !fs/io part.

>  	if (!order || !may_enter_fs || !may_perform_io)
>  		return COMPACT_SKIPPED;
>  

That being said, I am not really sure the patch is an improvement. If
anything I would much rather see the above comment updated.

Thanks!
-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path
  2016-06-16  2:42 [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path Ganesh Mahendran
  2016-06-16 10:17 ` Michal Hocko
@ 2016-06-21  0:52 ` David Rientjes
  1 sibling, 0 replies; 3+ messages in thread
From: David Rientjes @ 2016-06-21  0:52 UTC (permalink / raw)
  To: Ganesh Mahendran
  Cc: linux-mm, linux-kernel, akpm, vbabka, iamjoonsoo.kim, mhocko,
	mina86, minchan, mgorman, kirill.shutemov, izumi.taku, hannes,
	khandual, bsingharora

On Thu, 16 Jun 2016, Ganesh Mahendran wrote:

> diff --git a/mm/compaction.c b/mm/compaction.c
> index fbb7b38..dcfaf57 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1686,12 +1686,16 @@ enum compact_result try_to_compact_pages(gfp_t gfp_mask, unsigned int order,
>  
>  	*contended = COMPACT_CONTENDED_NONE;
>  
> -	/* Check if the GFP flags allow compaction */
> +	/*
> +	 * Check if this is an order-0 request and
> +	 * if the GFP flags allow compaction.
> +	 */

This seems obvious.

>  	if (!order || !may_enter_fs || !may_perform_io)
>  		return COMPACT_SKIPPED;
>  
>  	trace_mm_compaction_try_to_compact_pages(order, gfp_mask, mode);
>  
> +	current->flags |= PF_MEMALLOC;
>  	/* Compact each zone in the list */
>  	for_each_zone_zonelist_nodemask(zone, z, ac->zonelist, ac->high_zoneidx,
>  								ac->nodemask) {
> @@ -1768,6 +1772,7 @@ break_loop:
>  		all_zones_contended = 0;
>  		break;
>  	}
> +	current->flags &= ~PF_MEMALLOC;
>  
>  	/*
>  	 * If at least one zone wasn't deferred or skipped, we report if all

Compaction don't touch task_struct flags and PF_MEMALLOC is flag used 
primarily by the page allocator, moving this to try_to_compact_pages() 
doesn't make sense.

You could remove the !order check in try_to_compact_pages(), but I don't 
think it offers anything substantial.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-06-21  0:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-16  2:42 [PATCH v3] mm/compaction: remove unnecessary order check in direct compact path Ganesh Mahendran
2016-06-16 10:17 ` Michal Hocko
2016-06-21  0:52 ` David Rientjes

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).