[PATCH] mm: compact: remove watermark check at compact suitable

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] mm: compact: remove watermark check at compact suitable

Chen Feng
There are two paths calling this function.
For direct compact, there is no need to check the zone watermark here.
For kswapd wakeup kcompactd, since there is a reclaim before this.
It makes sense to do compact even the watermark is ok at this time.

Signed-off-by: Chen Feng <[hidden email]>
---
 mm/compaction.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/mm/compaction.c b/mm/compaction.c
index 8fa2540..cb322df 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1260,13 +1260,6 @@ static unsigned long __compaction_suitable(struct zone *zone, int order,
  return COMPACT_CONTINUE;
 
  watermark = low_wmark_pages(zone);
- /*
- * If watermarks for high-order allocation are already met, there
- * should be no need for compaction at all.
- */
- if (zone_watermark_ok(zone, order, watermark, classzone_idx,
- alloc_flags))
- return COMPACT_PARTIAL;
 
  /*
  * Watermarks for order-0 must be met for compaction. Note the 2UL.
--
1.9.1

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] mm: compact: remove watermark check at compact suitable

Vlastimil Babka
On 05/23/2016 05:20 AM, Chen Feng wrote:
> There are two paths calling this function.
> For direct compact, there is no need to check the zone watermark here.
> For kswapd wakeup kcompactd, since there is a reclaim before this.
> It makes sense to do compact even the watermark is ok at this time.

Hi,

I'm just working on v2 of the series [1] and some patches planned for v2 are
trying to simplify the watermark checks around compaction. The check you are
removing looked like simple and obvious one, so I didn't change it. But I'll
think more about your patch, e.g. if there are some corner cases. See for
example the fragindex check:

          * index of -1000 would imply allocations might succeed depending on
          * watermarks, but we already failed the high-order watermark check

After your patch, there is no more high-order watermark check, so the assumption
here is gone.
Also the comment above __compaction_suitable() should be updated too.

[1] http://lkml.kernel.org/r/<[hidden email]>

> Signed-off-by: Chen Feng <[hidden email]>
> ---
>   mm/compaction.c | 7 -------
>   1 file changed, 7 deletions(-)
>
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 8fa2540..cb322df 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -1260,13 +1260,6 @@ static unsigned long __compaction_suitable(struct zone *zone, int order,
>   return COMPACT_CONTINUE;
>
>   watermark = low_wmark_pages(zone);
> - /*
> - * If watermarks for high-order allocation are already met, there
> - * should be no need for compaction at all.
> - */
> - if (zone_watermark_ok(zone, order, watermark, classzone_idx,
> - alloc_flags))
> - return COMPACT_PARTIAL;
>
>   /*
>   * Watermarks for order-0 must be met for compaction. Note the 2UL.
>